[#27202] - [4.0] Move Switcher field to separate class
- Closed
- 1 Apr 2020
- Medium
- Build: 4.0-dev
- # 27202
- Diff
- SharkyKZ:j4/switcher-field
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
Summary of Changes
This is an alternative to #26214 that aims to eliminate weird use of class attribute to set field layouts. Furthermore, Switcher has some specific logic - it should not contain more than 2 options. For this the switcher should have its own class.
Testing Instructions
View forms containing switcher.
Expected result
Works like before.
Documentation Changes Required
Forms should be updated to use type="switcher" instead of type="radio" class="switcher".
Radio field layout moved back to layouts/joomla/form/radio.php.
joomla-cms-bot
- | Category | ⇒ | Administration com_banners com_checkin com_config com_contact com_content com_csp com_fields com_finder com_installer com_joomlaupdate com_media com_menus |
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
I have the same opinion about it breaking b/c for no real benefit
Breaking B/C with what?
Sorry it is three years since that thread. What I was requesting there was that it would be the class that was changed and not the type. joomla/40-backend-template#171 (comment)
Please fix conflicts.
Enable Content - Vote plugin
Go to Configuration > Articles > List Layouts
Update the following:
Show Votes in List
Show Ratings in List
Create custom URL field
Update Show URL
Do new installation.
Under Database Configuration > Connection Encryption > Two-way authentication
Update Verify Server Certificate
it works, but have a conflict again, and liltle copy/paste mistake
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27202.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27202.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27202.
I don't think this is the right thing to do.
The switcher is a radio field, just with a different design. It doesn't make sense to add field types for specific designs.
The current solution with multiple JLayouts is the correct solution. The technically most correct solution would be to add a "layout" attribute to the radio field declaration, like you proposed in #26214. But writing layout="joomla.form.field.radio.switcher" into each field is tedious and not very intuitiv. Thus using a class (which is design and JS related) is a nice compromise which worked very well for the whole J3 series.
Using class was fine in J3. But in J4 it's used to select a layout and that's totally wrong.
The switcher is a radio field, just with a different design. It doesn't make sense to add field types for specific designs.
Switcher does have specific logic. It must not have more than 2 options.
Using class was fine in J3. But in J4 it's used to select a layout and that's totally wrong.
It's not that much different. In J3, the layout was changed with JS, in J4 it's done in PHP.
Switcher does have specific logic. It must not have more than 2 options.
It's not a specific logic, it's just a limitation of the same logic. Same as btn-group-yesno. In fact "switcher" is a direct replacement for "btn-group btn-group-yesno".
Is it the most correct thing to do? No. But neither is this PR.
The most correct one would be the #26214, but that's horrible for developers.
I think all ways are correct.
I am for a separated field.
It gives more control, and makes thing more obvious than use of "class".
It does not hurt in the end :)
It gives more control
How that?
It does not hurt in the end
It's more work for extension devs to adjust their extensions to the new UI style. They can't just replace the class "btn-group-yesno" with "switcher", that part could be done with a search/replace.
They need to change the type, but not all radios - only those with 2 options (or btn-group-yesno).
So it DOES in fact hurt much.
I stick by my opinion 3 years ago as linked to further above
How that?
joomla-cms/libraries/src/Form/Field/SwitcherField.php
Lines 91 to 97 in f414372
and #27672
It's more work for extension devs to adjust their extensions to the new UI style
No one forced to use it ASAP, it will stay a regular radio button for those for lazy.
No one forced to use it ASAP, it will stay a regular radio button for those for lazy.
The work is exactly the same, if he does it right now or someday later.
Eventually, users will expect a component to follow the core UI.
This PR is just changing for the sake of changing. There is no advantage, but only disadvantage.
The point is dropping a bad practice.
Okay, then it should be that PR #26214 I do not mind,
but then every XML defination should have:
hiddenLabel="true" layout="joomla.form.field.radio.switcher"
Is that much easier to edit than changing type to type="switcher"?
But we should not use a class to switch the layout, it bad example for other.
I think this PR is good.
The point is dropping a bad practice.
It's not bad practice. It's a good compromise because the technically correct solutions are worse.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Needs Review |
needs review by @wilsonge
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27202.
| Labels |
Removed:
?
|
||
No, this is still the best option.
It's not, the other one is the technical correct solution, allthought ugly. This one would be worse.
Personally I like the layout attribute - I think it's a good middle ground - this is still radio buttons at the end of the day (in html too) - we're just adding some crazy css styling to them. Yes it's specific styling for when you have only two options - but how many options and the layout are both decided in the XML already anyhow
Choosing to reject this. I still prefer the layouts option as the middle group after a bit of time to see it in action
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-04-01 18:17:03 |
| Closed_By | ⇒ | wilsonge |
This used to be defined by
typebut was changed to aclassas requested: joomla/40-backend-template#171Not sure if @brianteeman and @mbabker have the same opinion 3 years on.