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
feat: add toBePartiallyChecked matcher #249
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
import {render} from './helpers/test-utils' | ||
|
||
describe('.toBePartiallyChecked', () => { | ||
test('handles input checkbox with aria-checked', () => { | ||
const {queryByTestId} = render(` | ||
<input type="checkbox" aria-checked="mixed" data-testid="checkbox-mixed" /> | ||
<input type="checkbox" checked data-testid="checkbox-checked" /> | ||
<input type="checkbox" data-testid="checkbox-unchecked" /> | ||
`) | ||
|
||
expect(queryByTestId('checkbox-mixed')).toBePartiallyChecked() | ||
expect(queryByTestId('checkbox-checked')).not.toBePartiallyChecked() | ||
expect(queryByTestId('checkbox-unchecked')).not.toBePartiallyChecked() | ||
}) | ||
|
||
test('handles input checkbox set as indeterminate', () => { | ||
const {queryByTestId} = render(` | ||
<input type="checkbox" data-testid="checkbox-mixed" /> | ||
<input type="checkbox" checked data-testid="checkbox-checked" /> | ||
<input type="checkbox" data-testid="checkbox-unchecked" /> | ||
`) | ||
|
||
queryByTestId('checkbox-mixed').indeterminate = true | ||
|
||
expect(queryByTestId('checkbox-mixed')).toBePartiallyChecked() | ||
expect(queryByTestId('checkbox-checked')).not.toBePartiallyChecked() | ||
expect(queryByTestId('checkbox-unchecked')).not.toBePartiallyChecked() | ||
}) | ||
|
||
test('handles element with role="checkbox"', () => { | ||
const {queryByTestId} = render(` | ||
<div role="checkbox" aria-checked="mixed" data-testid="aria-checkbox-mixed" /> | ||
<div role="checkbox" aria-checked="true" data-testid="aria-checkbox-checked" /> | ||
<div role="checkbox" aria-checked="false" data-testid="aria-checkbox-unchecked" /> | ||
`) | ||
|
||
expect(queryByTestId('aria-checkbox-mixed')).toBePartiallyChecked() | ||
expect(queryByTestId('aria-checkbox-checked')).not.toBePartiallyChecked() | ||
expect(queryByTestId('aria-checkbox-unchecked')).not.toBePartiallyChecked() | ||
}) | ||
|
||
test('throws when input checkbox is mixed but expected not to be', () => { | ||
const {queryByTestId} = render( | ||
`<input type="checkbox" aria-checked="mixed" data-testid="checkbox-mixed" />`, | ||
) | ||
|
||
expect(() => | ||
expect(queryByTestId('checkbox-mixed')).not.toBePartiallyChecked(), | ||
).toThrowError() | ||
}) | ||
|
||
test('throws when input checkbox is indeterminate but expected not to be', () => { | ||
const {queryByTestId} = render( | ||
`<input type="checkbox" data-testid="checkbox-mixed" />`, | ||
) | ||
|
||
queryByTestId('checkbox-mixed').indeterminate = true | ||
|
||
expect(() => | ||
expect(queryByTestId('input-mixed')).not.toBePartiallyChecked(), | ||
).toThrowError() | ||
}) | ||
|
||
test('throws when input checkbox is not checked but expected to be', () => { | ||
const {queryByTestId} = render( | ||
`<input type="checkbox" data-testid="checkbox-empty" />`, | ||
) | ||
|
||
expect(() => | ||
expect(queryByTestId('checkbox-empty')).toBePartiallyChecked(), | ||
).toThrowError() | ||
}) | ||
|
||
test('throws when element with role="checkbox" is partially checked but expected not to be', () => { | ||
const {queryByTestId} = render( | ||
`<div role="checkbox" aria-checked="mixed" data-testid="aria-checkbox-mixed" />`, | ||
) | ||
|
||
expect(() => | ||
expect(queryByTestId('aria-checkbox-mixed')).not.toBePartiallyChecked(), | ||
).toThrowError() | ||
}) | ||
|
||
test('throws when element with role="checkbox" is checked but expected to be partially checked', () => { | ||
const {queryByTestId} = render( | ||
`<div role="checkbox" aria-checked="true" data-testid="aria-checkbox-checked" />`, | ||
) | ||
|
||
expect(() => | ||
expect(queryByTestId('aria-checkbox-checked')).toBePartiallyChecked(), | ||
).toThrowError() | ||
}) | ||
|
||
test('throws when element with role="checkbox" is not checked but expected to be', () => { | ||
const {queryByTestId} = render( | ||
`<div role="checkbox" aria-checked="false" data-testid="aria-checkbox" />`, | ||
) | ||
|
||
expect(() => | ||
expect(queryByTestId('aria-checkbox')).toBePartiallyChecked(), | ||
).toThrowError() | ||
}) | ||
|
||
test('throws when element with role="checkbox" has an invalid aria-checked attribute', () => { | ||
const {queryByTestId} = render( | ||
`<div role="checkbox" aria-checked="something" data-testid="aria-checkbox-invalid" />`, | ||
) | ||
|
||
expect(() => | ||
expect(queryByTestId('aria-checkbox-invalid')).toBePartiallyChecked(), | ||
).toThrowError( | ||
'only inputs with type="checkbox" or elements with role="checkbox" and a valid aria-checked attribute can be used with .toBePartiallyChecked(). Use .toHaveValue() instead', | ||
) | ||
}) | ||
|
||
test('throws when the element is not an input', () => { | ||
const {queryByTestId} = render(`<select data-testid="select"></select>`) | ||
expect(() => | ||
expect(queryByTestId('select')).toBePartiallyChecked(), | ||
).toThrowError( | ||
'only inputs with type="checkbox" or elements with role="checkbox" and a valid aria-checked attribute can be used with .toBePartiallyChecked(). Use .toHaveValue() instead', | ||
) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import {matcherHint, printReceived} from 'jest-matcher-utils' | ||
import {checkHtmlElement} from './utils' | ||
|
||
export function toBePartiallyChecked(element) { | ||
checkHtmlElement(element, toBePartiallyChecked, this) | ||
|
||
const isValidInput = () => { | ||
return ( | ||
element.tagName.toLowerCase() === 'input' && element.type === 'checkbox' | ||
) | ||
} | ||
|
||
const isValidAriaElement = () => { | ||
return ( | ||
element.getAttribute('role') === 'checkbox' && | ||
element.getAttribute('aria-checked') === 'mixed' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check is already done in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, I don't think this should be part of this function. This function should only check if the element has a valid role for which this check makes sense. Then the actual check with the boolean value you set the |
||
) | ||
} | ||
|
||
if (!isValidInput() && !isValidAriaElement()) { | ||
return { | ||
pass: false, | ||
message: () => | ||
'only inputs with type="checkbox" or elements with role="checkbox" and a valid aria-checked attribute can be used with .toBePartiallyChecked(). Use .toHaveValue() instead', | ||
} | ||
} | ||
|
||
const isPartiallyChecked = () => { | ||
const isAriaMixed = element.getAttribute('aria-checked') === 'mixed' | ||
|
||
if (isValidInput()) { | ||
return element.indeterminate || isAriaMixed | ||
} | ||
|
||
return isAriaMixed | ||
} | ||
|
||
return { | ||
pass: isPartiallyChecked(), | ||
message: () => { | ||
const is = isPartiallyChecked() ? 'is' : 'is not' | ||
return [ | ||
matcherHint( | ||
`${this.isNot ? '.not' : ''}.toBePartiallyChecked`, | ||
'element', | ||
'', | ||
), | ||
'', | ||
`Received element ${is} partially checked:`, | ||
` ${printReceived(element.cloneNode(false))}`, | ||
].join('\n') | ||
}, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I don't think we should support
aria-checked
in a<input type="checkbox" />
. It should be supported only on non-native checkbox elements withrole="checkbox"
.The reason being that a
<input type="checkbox" />
won't convey to the user visually that it is partially checked. The only way the native checkbox can be partially checked is when itsindeterminate
property is set totrue
.What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same when I tested it in the broswer, as you say
indeterminate
set totrue
has a visual effect on the checkbox:The only thing that made me change my mind is that in the accessibility consoleboth of them had role checkbox and checked
mixed
, but yeah, I think it should be better to check for input+indeterminate and role checkbox + aria mixed, let me change itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you care to share more about this detail?
I'm also particularly interested to know about the
input
checkbox withindeterminate = true
. How is it conveyed in the accessibility console.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both inputs show the same data in the Accessibility tab in the Chrome Devtools:
Input with
indeterminate
:Input with
aria-checked="mixed"
:You can check the Codesandbox demo I've been using.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, my bad then. If the
<input type="checkbox" aria-checked="mixed" />
shows aschecked: mixed
even ifindeterminate
was not set true, then I think it was ok the first time around, and I assumed without checking what you correctly went out to check.What do you think?
If you agree, can you easily revert the changes after my earlier review? Sorry for having made you work extra for nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was also asking for your opinion here. But I'm conflicted. The
<input type="checkbox" aria-checked="mixed" />
looks visually as if unchecked, but semantically (from the aria perspective) it is still partially checked.I'm leaning towards trusting the accessibility info. If someone uses an input checkbox with
aria-checked="mixed"
they would presumably take over the rendering to reflect that using icons and stuff.So yes, in the end I'm leaning to that (after having thought about it out loud here 😄)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought the same at first, since it looks unchecked, but then I thought it's just a matter of how chrome styles the checkbox.
Let's support both of them then :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so the last commit needs to be reverted, right? It was good the first time around, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'll revert it and change only the check inside
isValidAriaElement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good now :)