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

fix(ByRole): Constrain API #1211

Merged
merged 3 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 16 additions & 23 deletions src/__tests__/element-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -788,35 +788,28 @@ test('queryAllByRole returns semantic html elements', () => {
</form>
`)

expect(queryAllByRole(/table/i)).toHaveLength(1)
expect(queryAllByRole(/tabl/i, {exact: false})).toHaveLength(1)
expect(queryAllByRole(/columnheader/i)).toHaveLength(1)
expect(queryAllByRole(/rowheader/i)).toHaveLength(1)
expect(queryAllByRole(/grid/i)).toHaveLength(1)
expect(queryAllByRole(/form/i)).toHaveLength(0)
expect(queryAllByRole(/button/i)).toHaveLength(1)
expect(queryAllByRole(/heading/i)).toHaveLength(6)
expect(queryAllByRole('table')).toHaveLength(1)
expect(queryAllByRole('columnheader')).toHaveLength(1)
expect(queryAllByRole('rowheader')).toHaveLength(1)
expect(queryAllByRole('grid')).toHaveLength(1)
expect(queryAllByRole('form')).toHaveLength(0)
expect(queryAllByRole('button')).toHaveLength(1)
expect(queryAllByRole('heading')).toHaveLength(6)
expect(queryAllByRole('list')).toHaveLength(2)
expect(queryAllByRole(/listitem/i)).toHaveLength(3)
expect(queryAllByRole(/textbox/i)).toHaveLength(2)
expect(queryAllByRole(/checkbox/i)).toHaveLength(1)
expect(queryAllByRole(/radio/i)).toHaveLength(1)
expect(queryAllByRole('listitem')).toHaveLength(3)
expect(queryAllByRole('textbox')).toHaveLength(2)
expect(queryAllByRole('checkbox')).toHaveLength(1)
expect(queryAllByRole('radio')).toHaveLength(1)
expect(queryAllByRole('row')).toHaveLength(3)
expect(queryAllByRole(/rowgroup/i)).toHaveLength(2)
expect(queryAllByRole(/(table)|(textbox)/i)).toHaveLength(3)
expect(queryAllByRole(/img/i)).toHaveLength(1)
expect(queryAllByRole('rowgroup')).toHaveLength(2)
expect(queryAllByRole('img')).toHaveLength(1)
expect(queryAllByRole('meter')).toHaveLength(1)
expect(queryAllByRole('progressbar')).toHaveLength(0)
expect(queryAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
expect(queryAllByRole('combobox')).toHaveLength(1)
expect(queryAllByRole('listbox')).toHaveLength(1)
})

test('queryByRole matches case with non-string matcher', () => {
const {queryByRole} = render(`<span role="1" />`)
expect(queryByRole(1)).toBeTruthy()
})

test('getAll* matchers return an array', () => {
const {
getAllByAltText,
Expand All @@ -827,7 +820,7 @@ test('getAll* matchers return an array', () => {
getAllByText,
getAllByRole,
} = render(`
<div role="container">
<div role="section">
<img
data-testid="poster"
alt="finding nemo poster"
Expand Down Expand Up @@ -864,7 +857,7 @@ test('getAll* matchers return an array', () => {
expect(getAllByDisplayValue('Japanese cars')).toHaveLength(1)
expect(getAllByDisplayValue(/cars$/)).toHaveLength(2)
expect(getAllByText(/^where/i)).toHaveLength(1)
expect(getAllByRole(/container/i)).toHaveLength(1)
expect(getAllByRole('section')).toHaveLength(1)
expect(getAllByRole('meter')).toHaveLength(1)
expect(getAllByRole('progressbar', {queryFallbacks: true})).toHaveLength(1)
})
Expand All @@ -879,7 +872,7 @@ test('getAll* matchers throw for 0 matches', () => {
getAllByText,
getAllByRole,
} = render(`
<div role="container">
<div role="section">
<label>No Matches Please</label>
</div>,
`)
Expand Down
8 changes: 4 additions & 4 deletions src/__tests__/get-by-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ cases(
html: `<div title="his"></div><div title="history"></div>`,
},
getByRole: {
query: /his/,
html: `<div role="his"></div><div role="history"></div>`,
query: 'button',
html: `<button>one</button><div role="button">two</button>`,
},
getByTestId: {
query: /his/,
Expand Down Expand Up @@ -87,8 +87,8 @@ cases(
html: `<div title="his"></div><div title="history"></div>`,
},
queryByRole: {
query: /his/,
html: `<div role="his"></div><div role="history"></div>`,
query: 'button',
html: `<button>one</button><div role="button">two</button>`,
},
queryByTestId: {
query: /his/,
Expand Down
4 changes: 0 additions & 4 deletions src/__tests__/text-matchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,6 @@ cases(
dom: `<input value="User ${LRM}name" />`,
queryFn: 'queryAllByDisplayValue',
},
queryAllByRole: {
dom: `<input role="User ${LRM}name" />`,
queryFn: 'queryAllByRole',
},
},
)

Expand Down
49 changes: 11 additions & 38 deletions src/queries/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,17 @@ import {
Matcher,
MatcherFunction,
MatcherOptions,
NormalizerFn,
} from '../../types'

import {
buildQueries,
fuzzyMatches,
getConfig,
makeNormalizer,
matches,
} from './all-utils'
import {buildQueries, getConfig, matches} from './all-utils'

const queryAllByRole: AllByRole = (
container,
role,
{
exact = true,
collapseWhitespace,
hidden = getConfig().defaultHidden,
name,
description,
trim,
normalizer,
queryFallbacks = false,
selected,
checked,
Expand All @@ -61,8 +50,6 @@ const queryAllByRole: AllByRole = (
} = {},
) => {
checkContainerType(container)
const matcher = exact ? matches : fuzzyMatches
const matchNormalizer = makeNormalizer({collapseWhitespace, trim, normalizer})

if (selected !== undefined) {
// guard against unknown roles
Expand Down Expand Up @@ -136,7 +123,7 @@ const queryAllByRole: AllByRole = (
return Array.from(
container.querySelectorAll<HTMLElement>(
// Only query elements that can be matched by the following filters
makeRoleSelector(role, exact, normalizer ? matchNormalizer : undefined),
makeRoleSelector(role),
),
)
.filter(node => {
Expand All @@ -148,22 +135,18 @@ const queryAllByRole: AllByRole = (
return roleValue
.split(' ')
.filter(Boolean)
.some(text => matcher(text, node, role as Matcher, matchNormalizer))
}
// if a custom normalizer is passed then let normalizer handle the role value
if (normalizer) {
return matcher(roleValue, node, role as Matcher, matchNormalizer)
.some(roleAttributeToken => roleAttributeToken === role)
}
// other wise only send the first word to match
const [firstWord] = roleValue.split(' ')
return matcher(firstWord, node, role as Matcher, matchNormalizer)
// other wise only send the first token to match
const [firstRoleAttributeToken] = roleValue.split(' ')
return firstRoleAttributeToken === role
}

const implicitRoles = getImplicitAriaRoles(node) as string[]

return implicitRoles.some(implicitRole =>
matcher(implicitRole, node, role as Matcher, matchNormalizer),
)
return implicitRoles.some(implicitRole => {
return implicitRole === role
})
})
.filter(element => {
if (selected !== undefined) {
Expand Down Expand Up @@ -228,18 +211,8 @@ const queryAllByRole: AllByRole = (
})
}

function makeRoleSelector(
role: ByRoleMatcher,
exact: boolean,
customNormalizer?: NormalizerFn,
) {
if (typeof role !== 'string') {
// For non-string role parameters we can not determine the implicitRoleSelectors.
return '*'
}

const explicitRoleSelector =
exact && !customNormalizer ? `*[role~="${role}"]` : '*[role]'
function makeRoleSelector(role: ByRoleMatcher) {
const explicitRoleSelector = `*[role~="${role}"]`

const roleRelations =
roleElements.get(role as ARIARoleDefinitionKey) ?? new Set()
Expand Down
2 changes: 0 additions & 2 deletions types/__tests__/type-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ export async function testByRole() {
)

console.assert(queryByRole(element, 'foo') === null)
console.assert(queryByRole(element, /foo/) === null)
console.assert(screen.queryByRole('foo') === null)
console.assert(screen.queryByRole(/foo/) === null)
}

export function testA11yHelper() {
Expand Down
4 changes: 2 additions & 2 deletions types/matches.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ export type MatcherFunction = (
export type Matcher = MatcherFunction | RegExp | number | string

// Get autocomplete for ARIARole union types, while still supporting another string
// Ref: https://github.com/microsoft/TypeScript/issues/29729#issuecomment-505826972
export type ByRoleMatcher = ARIARole | MatcherFunction | {}
// Ref: https://github.com/microsoft/TypeScript/issues/29729#issuecomment-567871939
export type ByRoleMatcher = ARIARole | (string & {})

export type NormalizerFn = (text: string) => string

Expand Down
4 changes: 3 additions & 1 deletion types/queries.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ export type FindByText<T extends HTMLElement = HTMLElement> = (
waitForElementOptions?: waitForOptions,
) => Promise<T>

export interface ByRoleOptions extends MatcherOptions {
export interface ByRoleOptions {
/** suppress suggestions for a specific query */
suggest?: boolean
/**
* If true includes elements in the query set that are usually excluded from
* the accessibility tree. `role="none"` or `role="presentation"` are included
Expand Down