User tests: Successful: Unsuccessful:
Remove duplicate class="custom-select"
.
Go to Menus > Main Menu
View page source
Search for <select id="menutype"
class="custom-select"
listed twice.
<select id="menutype" name="menutype" class="custom-select" onchange="this.form.submit();">
<select id="menutype" name="menutype" class="custom-select" onchange="this.form.submit();" class="custom-select">
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
i just wonder if it is correct to remove the one in the library
If w don't do that then we're forcing the bootstrap class everywhere which is totally wrong
Technically the library helpers shouldn't be framework coupled (even if it is a very generic utility class) unless the helper is a framework oriented one (i.e. JHtmlBootstrap
). So it's one of those practical things, if it's in the library there's some Bootstrap leakage there, whereas if it's not then anywhere in the code using this needs to ensure the class is included.
if it's not then anywhere in the code using this needs to ensure the class is included.
True, and also this gets a bit messy if we consider that these fields get their data from an XML file which is hard to override
the reason for me asking was so because i saw other instances of custom-select-* in the libraries and it seems odd to remove one and not the other and smells of an issue elsewhere in the codebase if it is duplicating for this instance but not for others
Maybe the middle ground will be a joomla-select class (for the core templates will just extend the bootstrap custom-select).
I'm not really sure that will help. You go from the field been coupled with bootstrap to been coupled with a block of css. At a basic level we would be asking people to load both bootstrap and this Joomla css (previously 'chosen'). Not that that is a problem but it creates another middle of the road approach. This time between bootstrap and a joomla css framework.
You go from the field been coupled with bootstrap to been coupled with a block of css.
Well my idea here was about consistency, eg all select elements have this class. What you do with that class is down to the t mplate developer
I have tested this item
x-link comment from duplicate PR: #21147 (comment)
I have tested this item
While testing our extension with J4, i came across this one,
specifying a custom class for select.groupedlist is problematic as you have duplicate class attribute added
The hard coded class attribute is not added for select.list, but only for select.groupedlist, such hard coding does not exist in J3
and in the end if it is really needed, then only add it only as default if one was not provided by the method caller
Status | Pending | ⇒ | Ready to Commit |
Ready to Commit after two successful tests.
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-07-16 18:59:23 |
Closed_By | ⇒ | laoneo | |
Labels |
Added:
?
|
Good catch, thanks for the fix and the tests.
your pr works - i just wonder if it is correct to remove the one in the library