feat: to-be-checked and to-be-partially-checked implementation#175
feat: to-be-checked and to-be-partially-checked implementation#175KeylaMunnoz wants to merge 2 commits into
Conversation
JDOM10
left a comment
There was a problem hiding this comment.
Hey @KeylaMunnoz! This is looking great, just left a few comments. Let me know what do you think! ![]()
| public toBeChecked(): this { | ||
| const isNativeCheckable = isCheckableInput(this.actual); | ||
| const isAriaCheckable = isValidAriaChecked(this.actual) | ||
| && ["true", "false"].includes(this.actual.getAttribute("aria-checked") ?? ""); |
There was a problem hiding this comment.
isValidAriaChecked already restricts to "true" | "false" | "mixed", so the ["true", "false"].includes(...) clause makes the helper call redundant — the second check fully covers it. Let's drop one of them (or fold the true/false-only logic into the helper) so the intent is clear.
| public toBeChecked(): this { | ||
| const isNativeCheckable = isCheckableInput(this.actual); | ||
| const isAriaCheckable = isValidAriaChecked(this.actual) | ||
| && ["true", "false"].includes(this.actual.getAttribute("aria-checked") ?? ""); |
There was a problem hiding this comment.
Also this accepts any element with aria-checked, so a non-widget like would pass. Similar to how toBePressed gates on isButtonElement, we should require a checkable role (checkbox, radio, switch, …) here. Related: the aria-radio-* fixtures are currently unused, so radio-role coverage is missing too.
Implementation of both matchers: