User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | UI/UX |
Rel_Number | 0 | ⇒ | 9639 |
Relation Type | ⇒ | Pull Request for | |
Easy | No | ⇒ | Yes |
I have tested this item unsuccessfully on 29945ed
After applying the patch, as you can see in the image the "caption class" drop down menu still differ from "Image float menu"
<?php echo JText::_('COM_MEDIA_NOT_SET') ?> also not showing "Not set" in the "caption class" drop down menu.
I have tested it in both front-end and back-end article edit menu. Still same issue.
@shubhamnba2009
You applied the patch on 3.5.1?
@MATsxm I have applied it in 3.5.0 let me test this on 3.5.1. After that i'll submit my test result.
I have updated my 3.5.0 version to 3.5.1 and after applying the patch still same problem
How are you applying the patch - your screenshot shows the original code
Caches?
Tested again on Chrome, FF & edge and the patch fix it here
I have updated my Joomla using the update notification available 0n 3.5.0. As you can see in my screeenshot on right side below the version is 3.5.1.
And i had applied the code using Joomla! patch tester!!>
I think there must be some problem while upgrading. Maybe i should download the latest staging then test it!!
I can't get the new dropdown and for a good reason:
We have overrides in isis html/media/ that were deleted
https://github.com/joomla/joomla-cms/tree/staging/administrator/templates/isis/html/layouts/joomla
and protostar
https://github.com/joomla/joomla-cms/tree/staging/templates/protostar/html/layouts/joomla
But these files were NOT added to script.php, therefore not deleted when updating Joomla.
Will make an urgent PR.
In the meanwhile, @brianteeman , there is a line in your patch that has extraneous spaces instead of a tab.
+ <option value="" selected="selected"><?php echo JText::_('COM_MEDIA_NOT_SET') ?></option>
can you correct?
Once done I can test this OK
This PR has received new commits.
CC: @MATsxm, @shubhamnba2009
Ah that explains why it worked perfectly on a clean install but didnt show on an upgraded site
This PR has received new commits.
CC: @MATsxm, @shubhamnba2009
I have done those two lines but really almost every string needs updating
with it as well. Doesnt fail travis so is it needed?
On 7 April 2016 at 09:38, Joomla! CMS Bot notifications@github.com wrote:
This PR has received new commits.
CC: @MATsxm https://github.com/MATsxm, @shubhamnba2009
https://github.com/shubhamnba2009
This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/9754
https://issues.joomla.org/tracker/joomla-cms/9754.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9754 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@brianteeman no. Travis don't check that. I can prepare a seperate PR for that changes at the other places later. As it is out of scope of this PR. Thanks.
I can add them to this PR if you want and if you say that it is needed
On 7 April 2016 at 09:47, zero-24 notifications@github.com wrote:
@brianteeman https://github.com/brianteeman no. Travis don't check
that. I can prepare a seperate PR for that changes at the other places
later. As it is out of scope of this PR. Thanks.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9754 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
I have tested this item successfully on 69f0f8f
TEST OK
concerning the script PR, I closed it:
My mistake.
These files were NOT present in 3.4.8, 3.5.0 or 3.5.1.
They were remainings of test with the beta/rc
I have tested this item successfully on 69f0f8f
Works fine here.
Status | Pending | ⇒ | Ready to Commit |
RTC - thanks for testing
Labels |
Added:
?
|
Milestone |
Added: |
The difference between a datalist and a select is that with a datalist you can add an own class there. That may be usefull if your template doesn't support the "text-*" classes or you want to add a custom class there. Originally it was a plain text field if I remember right.
So I don't think it's a good idea to change a text field which allowed any class input to a predefined select.
How would a user add or remove a class?
On 9 April 2016 at 12:54, Thomas Hunziker notifications@github.com wrote:
The difference between a datalist and a select is that with a datalist you
can add an own class there. That may be usefull if your template doesn't
support the "text-*" classes or you want to add a custom class there.
Originally it was a plain text field if I remember right.So I don't think it's a good idea to change a text field which allowed any
class input to a predefined select.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9754 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
How would a user add or remove a class?
It's a regular textfield, the datalist options only act as suggestions. You can write in anything into that field.
It's a regular textfield, the datalist options only act as suggestions. You can write in anything into that field.
Clearly several people didnt realise that. Its not a UI convention we use anywhere else so I will stick by my changes in this PR
It may not be a common UI convention but IMO this is right as is (unless we duplicate the logic for the module position selector which also allows custom input). I can't say I have the slightest clue where the captions are used (I don't use them, sue me), but there's a part of me that thinks limiting the options to essentially alignment classes might not be a fully wanted behavior.
in that case I would remove all the options as it certainly isnt clear
that this is not a select but just a suggestion
On 9 April 2016 at 13:25, Michael Babker notifications@github.com wrote:
It may not be a common UI convention but IMO this is right as is (unless
we duplicate the logic for the module position selector which also allows
custom input). I can't say I have the slightest clue where the captions are
used (I don't use them, sue me), but there's a part of me that thinks
limiting the options to essentially alignment classes might not be a fully
wanted behavior.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9754 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Clearly several people didnt realise that. Its not a UI convention we use anywhere else so I will stick by my changes in this PR
It's a regular HTML5 element we use there. Nothing special. People will get used to it with time of course.
Either stick with the datalist (select is wrong for sure as it limits the uses of that field) or remove them. But removing will not fix anything imho.
If people are confused about a regular HTML5 feature, then the fix is probably to educate the users
Labels |
Removed:
?
|
Labels |
Added:
?
|
In this case I agree with Thomas - we probably could do more to improve the UX - but it is important that users can optionally enter their own classes in that box (especially for other templates).
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-04-09 13:50:21 |
Closed_By | ⇒ | wilsonge |
Labels |
Removed:
?
|
Milestone |
Removed: |
I have tested this item successfully on 29945ed
able to reproduce then #9754 works as expected, able to choose caption class, then change the caption to another, able to save, edit again... everything looks fine here.
Thanks
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9754.