Skip to content

[4.0] Add "disabled" attribute to switcher#28356

Merged
zero-24 merged 3 commits intojoomla:4.0-devfrom
Bakual:4.0_DisabledSwitcher
Mar 22, 2020
Merged

[4.0] Add "disabled" attribute to switcher#28356
zero-24 merged 3 commits intojoomla:4.0-devfrom
Bakual:4.0_DisabledSwitcher

Conversation

@Bakual
Copy link
Copy Markdown
Contributor

@Bakual Bakual commented Mar 15, 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

@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Mar 15, 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.

@SharkyKZ
Copy link
Copy Markdown
Contributor

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.

@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Mar 15, 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.

@SharkyKZ
Copy link
Copy Markdown
Contributor

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

@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Mar 18, 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.

@SharkyKZ
Copy link
Copy Markdown
Contributor

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.

@brianteeman
Copy link
Copy Markdown
Contributor

can someone tell me the use case for this

@SharkyKZ
Copy link
Copy Markdown
Contributor

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.

@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Mar 18, 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.

@astridx
Copy link
Copy Markdown
Contributor

astridx commented Mar 19, 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.

@astridx
Copy link
Copy Markdown
Contributor

astridx commented Mar 19, 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)

Switcher - add opacity if disabled
@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Mar 19, 2020

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

Merged. Thanks for the help!

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Mar 19, 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.

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Mar 19, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 19, 2020
@jwaisner
Copy link
Copy Markdown
Member

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.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 22, 2020
@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Mar 22, 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

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Mar 22, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 22, 2020
@zero-24 zero-24 added this to the Joomla 4.0 milestone Mar 22, 2020
@zero-24 zero-24 merged commit fcd6332 into joomla:4.0-dev Mar 22, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 22, 2020
@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Mar 22, 2020

Merged thanks @Bakual & testers 👍

@Bakual Bakual deleted the 4.0_DisabledSwitcher branch March 22, 2020 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants