? ? Pending

User tests: Successful: Unsuccessful:

avatar Bakual
Bakual
15 Mar 2020

Pull Request for Issue #27760 .

Summary of Changes

Adds the disabled attribute to the switcher fields if in the XML either readonly or disabled is set.
Since readonly isn't a valid attribute for radios (which technically a switcher still is), I just add disabled in that case.

I haven't touched CSS as I leave that to people who have a clue about design.
Imho it should be made visible somehow that the field is disabled.

Testing Instructions

See Issue #27760

Expected result

"Locked" Switcher

Actual result

Switcher can be toggled

Documentation Changes Required

None

avatar Bakual Bakual - open - 15 Mar 2020
avatar Bakual Bakual - change - 15 Mar 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 15 Mar 2020
Category Layout
avatar Bakual
Bakual - comment - 15 Mar 2020

For the CSS side, the div containing the switcher has now an additional class when disabled. So it should be possible to target the disabled switchers with div.switcher.disabled. Hope that helps.

avatar SharkyKZ
SharkyKZ - comment - 15 Mar 2020

Readonly and disabled attributes are not the same. Values of disabled inputs are not submitted to the server. When readonly is used, selected radio should not be disabled.

avatar Bakual
Bakual - comment - 15 Mar 2020

The thing is, there is no readonly attribute for radio elements. See https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly.
I've used the same behavior we already use in J3 and in the radio buttons.

avatar SharkyKZ
SharkyKZ - comment - 18 Mar 2020

We can emulate readonly behavior by disabling all inputs except currently selected. Not sure how accessible that is though. @brianteeman any thoughts?

avatar Bakual
Bakual - comment - 18 Mar 2020

@SharkyKZ Why do you want to emulate readonly? The readonly attribute doesn't make sense for radios, that's why it doesn't exist. There is no point in emulating it.

avatar SharkyKZ
SharkyKZ - comment - 18 Mar 2020

Treating readonly and disabled the same is no good. We need to have the selected value submitted when readonly is used. It's not a matter of whether HTML element supports it or not, it's about how we implement the functionality in our fields.

avatar brianteeman
brianteeman - comment - 18 Mar 2020

can someone tell me the use case for this

avatar SharkyKZ
SharkyKZ - comment - 18 Mar 2020

Not allowing to change the value after the form was submitted for the first time. Or having a field disabled until another field has been filled in.

avatar Bakual
Bakual - comment - 18 Mar 2020

As said, readonly doesn't exist for that type of input.
I just added it for convenience and since it was already in J3 and in the button layout. The readonly and disabled attribute set in the XML will both result in a disabled radio/switcher/button input.

The dev using a readonly/disabled element has to take care of this on PHP side. You couldn't trust the submitted value anyway as the user could remove the disabled/readonly attribute in the browser.

avatar astridx
astridx - comment - 19 Mar 2020

I have tested this item successfully on 87f2daa


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28356.

avatar astridx astridx - test_item - 19 Mar 2020 - Tested successfully
avatar astridx
astridx - comment - 19 Mar 2020

I think this implementation is user-friendly. The normal user does not know that there is no readonly for the radio type. He doesn't even know that the switcher is a radio input. At readonly, he expects exactly what bakual has programmed technically correctly here.

@brianteeman Just as for the button, there are also use cases for the switch. You want to show that it is an option, but that it is deactivated with the current settings.

It only bothered me that the appearance does not change. It looks like a mistake. That's why I added a PR here.
@Bakual If you like, please merge or add to your code.

For my test I added

disabled = "1"

to a switch in the file
joomla-cms4/administrator/components/com_templates/forms/source.xml

I tested also with

readonly = "true"
disabled = "true"
readonly = "1"

I tested also with the from me applied change. It looks like this:
Templates  Customise  Cassiopeia    test   Administration(1)

avatar Bakual Bakual - change - 19 Mar 2020
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 19 Mar 2020
Category Layout Administration Templates (admin) Layout
avatar Bakual
Bakual - comment - 19 Mar 2020

If you like, please merge or add to your code.

Merged. Thanks for the help!

avatar Quy
Quy - comment - 19 Mar 2020

I have tested this item successfully on d39e43e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28356.

avatar Quy Quy - change - 19 Mar 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 19 Mar 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28356.

avatar Quy Quy - test_item - 19 Mar 2020 - Tested successfully
avatar jwaisner jwaisner - change - 22 Mar 2020
Status Ready to Commit Pending
avatar jwaisner
jwaisner - comment - 22 Mar 2020

Setting back to Pending as there has only been one successful test.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28356.

avatar Bakual
Bakual - comment - 22 Mar 2020

Setting back to Pending as there has only been one successful test.

@jwaisner It's two tests. One from Quy and one from Astridx

avatar Quy Quy - change - 22 Mar 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 22 Mar 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28356.

avatar zero-24 zero-24 - change - 22 Mar 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-03-22 21:02:54
Closed_By zero-24
Labels Added: ?
avatar zero-24 zero-24 - close - 22 Mar 2020
avatar zero-24 zero-24 - merge - 22 Mar 2020
avatar zero-24
zero-24 - comment - 22 Mar 2020

Merged thanks @Bakual & testers ?

Add a Comment

Login with GitHub to post a comment