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
Merged
gnapse
merged 2 commits into
testing-library:master
from
rubenmoya:pr/partially-checked-matcher
May 19, 2020
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
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() | ||
}) | ||
|
||
test('throws when the element is not a checkbox', () => { | ||
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', | ||
) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
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' | ||
} | ||
|
||
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') | ||
}, | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 :)