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

feat(byRole): Exclude inaccessible elements #352

Merged
merged 8 commits into from
Sep 23, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/__tests__/element-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ test('get throws a useful error message', () => {
</div>"
`)
expect(() => getByRole('LucyRicardo')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an element with the role "LucyRicardo"
"Unable to find an accessible element with the role "LucyRicardo"

There are no available roles.
There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<div>
<div />
Expand Down
154 changes: 150 additions & 4 deletions src/__tests__/role.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {render} from './helpers/test-utils'

test('logs available roles when it fails', () => {
test('by default logs accessible roles when it fails', () => {
const {getByRole} = render(`<h1>Hi</h1>`)
expect(() => getByRole('article')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an element with the role "article"
"Unable to find an accessible element with the role "article"

Here are the available roles:
Here are the accessible roles:

heading:

Expand All @@ -21,9 +21,49 @@ Here are the available roles:
`)
})

test('logs error when there are no available roles', () => {
test('when hidden: true logs available roles when it fails', () => {
const {getByRole} = render(`<div hidden><h1>Hi</h1></div>`)
expect(() => getByRole('article', {hidden: true}))
.toThrowErrorMatchingInlineSnapshot(`
"Unable to find an element with the role "article"

Here are the available roles:

heading:

<h1 />

--------------------------------------------------

<div>
<div
hidden=""
>
<h1>
Hi
</h1>
</div>
</div>"
`)
})

test('logs error when there are no accessible roles', () => {
const {getByRole} = render('<div />')
expect(() => getByRole('article')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an accessible element with the role "article"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<div>
<div />
</div>"
`)
})

test('logs a different error if inaccessible roles should be included', () => {
const {getByRole} = render('<div />')
expect(() => getByRole('article', {hidden: true}))
.toThrowErrorMatchingInlineSnapshot(`
"Unable to find an element with the role "article"

There are no available roles.
Expand All @@ -33,3 +73,109 @@ There are no available roles.
</div>"
`)
})

test('by default excludes elements that have the html hidden attribute or any of their parents', () => {
const {getByRole} = render('<div hidden><ul /></div>')

expect(() => getByRole('list')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an accessible element with the role "list"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<div>
<div
hidden=""
>
<ul />
</div>
</div>"
`)
})

test('by default excludes elements which have display: none or any of their parents', () => {
const {getByRole} = render(
'<div style="display: none;"><ul style="display: block;" /></div>',
)

expect(() => getByRole('list')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an accessible element with the role "list"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<div>
<div
style="display: none;"
>
<ul
style="display: block;"
/>
</div>
</div>"
`)
})

test('by default excludes elements which have visibility hidden', () => {
const {getByRole} = render('<div style="visibility: hidden;"><ul /></div>')

expect(() => getByRole('list')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an accessible element with the role "list"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<div>
<div
style="visibility: hidden;"
>
<ul />
</div>
</div>"
`)
})

test('by default excludes elements which have aria-hidden="true" or any of their parents', () => {
// > if it, or any of its ancestors [...] have their aria-hidden attribute value set to true.
// -- https://www.w3.org/TR/wai-aria/#aria-hidden
// > In other words, aria-hidden="true" on a parent overrides aria-hidden="false" on descendants.
// -- https://www.w3.org/TR/core-aam-1.1/#exclude_elements2
const {getByRole} = render(
'<div aria-hidden="true"><ul aria-hidden="false" /></div>',
)

expect(() => getByRole('list')).toThrowErrorMatchingInlineSnapshot(`
"Unable to find an accessible element with the role "list"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the \`hidden\` option to \`true\`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole

<div>
<div
aria-hidden="true"
>
<ul
aria-hidden="false"
/>
</div>
</div>"
`)
})

test('considers the computed visibility style not the parent', () => {
// this behavior deviates from the spec which includes "any descendant"
// if visibility is hidden. However, chrome a11y tree and nvda will include
// the following markup. This behavior might change depending on how
// https://github.com/w3c/aria/issues/1055 is resolved.
const {getByRole} = render(
'<div style="visibility: hidden;"><main style="visibility: visible;"><ul /></main></div>',
)

expect(getByRole('list')).not.toBeNull()
})

test('can include inaccessible roles', () => {
// this behavior deviates from the spec which includes "any descendant"
// if visibility is hidden. However, chrome a11y tree and nvda will include
// the following markup. This behavior might change depending on how
// https://github.com/w3c/aria/issues/1055 is resolved.
const {getByRole} = render('<div hidden><ul /></div>')

expect(getByRole('list', {hidden: true})).not.toBeNull()
})
53 changes: 36 additions & 17 deletions src/queries/role.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,67 @@
import {getImplicitAriaRoles, prettyRoles} from '../role-helpers'
import {
getImplicitAriaRoles,
prettyRoles,
shouldExcludeFromA11yTree,
} from '../role-helpers'
import {buildQueries, fuzzyMatches, makeNormalizer, matches} from './all-utils'

function queryAllByRole(
container,
role,
{exact = true, collapseWhitespace, trim, normalizer} = {},
{exact = true, collapseWhitespace, hidden = false, trim, normalizer} = {},
) {
const matcher = exact ? matches : fuzzyMatches
const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer})

return Array.from(container.querySelectorAll('*')).filter(node => {
const isRoleSpecifiedExplicitly = node.hasAttribute('role')
return Array.from(container.querySelectorAll('*'))
.filter(element => {
return hidden === false
? shouldExcludeFromA11yTree(element) === false
: true
})
.filter(node => {
const isRoleSpecifiedExplicitly = node.hasAttribute('role')

if (isRoleSpecifiedExplicitly) {
return matcher(node.getAttribute('role'), node, role, matchNormalizer)
}
if (isRoleSpecifiedExplicitly) {
return matcher(node.getAttribute('role'), node, role, matchNormalizer)
}

const implicitRoles = getImplicitAriaRoles(node)
const implicitRoles = getImplicitAriaRoles(node)

return implicitRoles.some(implicitRole =>
matcher(implicitRole, node, role, matchNormalizer),
)
})
return implicitRoles.some(implicitRole =>
matcher(implicitRole, node, role, matchNormalizer),
)
})
}

const getMultipleError = (c, role) =>
`Found multiple elements with the role "${role}"`

const getMissingError = (container, role) => {
const roles = prettyRoles(container)
const getMissingError = (container, role, {hidden = false} = {}) => {
const roles = prettyRoles(container, {hidden})
let roleMessage

if (roles.length === 0) {
roleMessage = 'There are no available roles.'
if (hidden === false) {
roleMessage =
'There are no accessible roles. But there might be some inaccessible roles. ' +
'If you wish to access them, then set the `hidden` option to `true`. ' +
'Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole'
} else {
roleMessage = 'There are no available roles.'
}
} else {
roleMessage = `
Here are the available roles:
Here are the ${hidden === false ? 'accessible' : 'available'} roles:

${roles.replace(/\n/g, '\n ').replace(/\n\s\s\n/g, '\n\n')}
`.trim()
}

return `
Unable to find an element with the role "${role}"
Unable to find an ${
hidden === false ? 'accessible ' : ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra space.

Suggested change
hidden === false ? 'accessible ' : ''
hidden === false ? 'accessible' : ''

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need to keep this and remove the unconditional space later on. Otherwise we get

Unable to find an  element with the role "${role}
                 ^^ double space

for hidden: false

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space is still there (since I'm pretty sure it's required) but I fixed the double space issue.

}element with the role "${role}"

${roleMessage}`.trim()
}
Expand Down