Interpret undefined value as false #38
Conversation
|
Undefined should not be allowed by validator. Using undefined on purpose is an anti-pattern in javascript and a huge source of bugs. Two options here are:
|
src/Button.vue
Outdated
| @@ -1,7 +1,7 @@ | |||
| <template> | |||
| <label role="checkbox" | |||
| :class="className" | |||
| :aria-checked="ariaChecked"> | |||
| :aria-checked="!!toggled"> | |||
There was a problem hiding this comment.
aria-checked requires an actual word "true" or "false". This introduces a bug.
There was a problem hiding this comment.
No it doesn't, I tested it. This coerces the value to a boolean, and the boolean then gets rendered as a string by Vue.
There was a problem hiding this comment.
Example:
https://jsfiddle.net/63hjbL5p/1/ - look in inspector.
Requirement:
aria-checked="true": The checkbox is checked
aria-checked="false": The checkbox is not checked
aria-checked="mixed": The checkbox is partially checked
Example:
<span role="checkbox" aria-checked="false" tabindex="0" id="chk1"></span>
What i am saying is that if :aria-checked="false", argument will be removed from the Element. By requirement it should be present and should be aria-checked="false". Thats just how aria-checked works.
Dont get me wrong, your code also works, it just does not do what browser will expect form aria-checked argument.
There was a problem hiding this comment.
Yes, I'm sorry, it does get removed you're right. I'll adjust it.
|
Yes I agree, undefined shouldn't be passed, but if it happens to be passed, I don't think it should throw an exception from a random place in the code, which it currently does. This PR is to address the exception. I'm fine with adding more strict checks to the validator as well. |
|
The problem with |
1f0d085 to
642680c
Compare
|
I think this is the better approach then: 642680c |
|
Yep, looks good to me 👍 |
|
Thanks @euvl ! |
This is a follow up PR to euvl#38
Currently, if I pass
undefinedas value to the toggle button, it internally throws an error due to the call oftoString()on undefined for the value ofariaChecked. This fix converts it to a boolean instead and renders that as a string automatically.