Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure button plugin sets/removes active class correctly on page load #28952

Conversation

patrickhlauke
Copy link
Member

@patrickhlauke patrickhlauke commented Jun 24, 2019

More fundamentally fixes a problem with Bootstrap's "faked" toggles, checkboxes, radio buttons ... the fact that the visual presentation can get out of sync with the real value/state of controls if, for instance, autocomplete happened (which would change the underlying value, but not set the correct active class). This does a sanity check on page load and makes sure the active class is only set if the underlying control is really pressed/checked, and otherwise removes it. This guarantees that they're in sync from the moment the page is loaded, regardless of whether or not a browser did an autocomplete, or even the author forgot to set BOTH the class and the relevant attribute.

Also, removes the now unnecessary autocomplete="off" attributes used to essentially prevent out-of-sync situations from happening (in Firefox, primarily). Now, if Firefox does an autocomplete, this new logic ensures that the active class will be correct, no problem.

Closes #28623

Supersedes #28649

Could potentially be ported to v5, but once #28463 lands the part relating to checkboxes/radio buttons will be irrelevant ... will create a separate v5-specific patch to keep just the aria-pressed toggles in sync on page load (probably as part of that PR itself)

…s on page load

Sanity check, ensures that the UI visually matches the actual values/states of controls. Also ensures that if any autocomplete/autofill happened, this is visually accounted for
by having the correct class set.

Includes unit tests (and `autocomplete` has been removed from these as it's no longer necessary)
As the attribute was there to force/ensure that the visual presentation matched the state, and this is now taken care of programmatically, there's no need to unnecessarily suppress autocomplete...let them autocomplete if they want to...
js/src/button.js Outdated Show resolved Hide resolved
js/src/button.js Outdated Show resolved Hide resolved
js/src/button.js Outdated Show resolved Hide resolved
@XhmikosR XhmikosR changed the base branch from v4-dev to v4-dev-xmr June 24, 2019 14:33
@patrickhlauke patrickhlauke merged commit e38d9fa into v4-dev-xmr Jun 24, 2019
@patrickhlauke patrickhlauke deleted the patrickhlauke-v4-button-plugin-onload-styles-match-reality branch June 24, 2019 20:01
XhmikosR pushed a commit that referenced this pull request Jun 26, 2019
…#28952)

* Ensure correct active class is set on button toggles/checkboxes/radios on page load

Sanity check, ensures that the UI visually matches the actual values/states of controls. Also ensures that if any autocomplete/autofill happened, this is visually accounted for
by having the correct class set.

Includes unit tests (and `autocomplete` has been removed from these as it's no longer necessary)

* Remove now unnecessary autocomplete attribute

As the attribute was there to force/ensure that the visual presentation matched the state, and this is now taken care of programmatically, there's no need to unnecessarily suppress autocomplete...let them autocomplete if they want to...
XhmikosR pushed a commit that referenced this pull request Jul 10, 2019
…#28952)

* Ensure correct active class is set on button toggles/checkboxes/radios on page load

Sanity check, ensures that the UI visually matches the actual values/states of controls. Also ensures that if any autocomplete/autofill happened, this is visually accounted for
by having the correct class set.

Includes unit tests (and `autocomplete` has been removed from these as it's no longer necessary)

* Remove now unnecessary autocomplete attribute

As the attribute was there to force/ensure that the visual presentation matched the state, and this is now taken care of programmatically, there's no need to unnecessarily suppress autocomplete...let them autocomplete if they want to...
@mdo mdo mentioned this pull request Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants