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

Don't set read-only class if readonly is null #276

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cincodenada
Copy link

@cincodenada cincodenada commented May 27, 2021

toggleClass() requires a boolean value, not just a truthy/falsy one.

Because the current code just passes in the value of readonly directly, if it is null, the current code results in the read-only class toggled rather than set to any determinate value. While setting readonly to null may be an edge case, it's easy to make this behavior less surprising and unintuitive by simply coercing the value passed to toggleClass to a boolean, so that readonly=null always results in no read-only class.

I've created an Ember Twiddle demonstrating the current behavior here (it may take some time to build, I'm not sure where that build is cached): https://ember-twiddle.com/b9967c2e3604add699c58a2f97399915?openFiles=templates.application%5C.hbs%2C

You can see in the demonstration that true and false act as expected, but null behaves strangely - if you select true and then null, the checkbox will be toggleable, but if you select false and then null, it will be read-only. This is because setting readonly to null has the effect of toggling it, instead of setting it to any given value.

toggleClass() requires a boolean value, not just a truthy/falsy one.
When readonly is null, the current code results in the read-only class
being toggled (since null is treated the same as the argument not being
provided), resutling in indeterminate results depending on what the
previous value of readonly was.

To avoid this surprising behavior, just coerce the value of readonly to
a boolean before passing it to toggleClass(), so that null will always
result in no read-only class, same as if the readonly attribute was not
provided.
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.

1 participant