Skip to content

Commit caf76e3

Browse files
authored
fix: more improvements to suggestions (#599)
* fix: don't suggest query that would result in different element * fix: escape regexp in suggestions
1 parent bec190b commit caf76e3

File tree

2 files changed

+38
-8
lines changed

2 files changed

+38
-8
lines changed

src/__tests__/suggestions.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,18 @@ test('respects ignores', () => {
3535
).not.toThrowError()
3636
})
3737

38+
test('does not suggest query that would give a different element', () => {
39+
renderIntoDocument(`
40+
<div data-testid="foo"><img src="foo" /></div>
41+
<div data-testid="bar"><a href="/foo"><div role="figure"><img src="foo" /></div></a></div>
42+
<a data-testid="baz"><h1>link text</h1></a>
43+
`)
44+
45+
expect(() => screen.getByTestId('foo')).not.toThrowError()
46+
expect(() => screen.getByTestId('bar')).not.toThrowError()
47+
expect(() => screen.getByTestId('baz')).not.toThrowError()
48+
})
49+
3850
test('does not suggest when using getByRole', () => {
3951
renderIntoDocument(`<button data-testid="foo">submit</button>`)
4052

@@ -57,6 +69,12 @@ test(`should not suggest if the suggestion would give different results`, () =>
5769
).not.toThrowError()
5870
})
5971

72+
test('should suggest by label over title', () => {
73+
renderIntoDocument(`<label><span>bar</span><input title="foo" /></label>`)
74+
75+
expect(() => screen.getByTitle('foo')).toThrowError(/getByLabelText\("bar"\)/)
76+
})
77+
6078
test('should not suggest if there would be mixed suggestions', () => {
6179
renderIntoDocument(`
6280
<button data-testid="foo">submit</button>
@@ -145,6 +163,16 @@ test('should suggest img role w/ alt text', () => {
145163
)
146164
})
147165

166+
test('escapes regular expressions in suggestion', () => {
167+
renderIntoDocument(
168+
`<img src="foo.png" alt="The Problem (picture of a question mark)" data-testid="foo" />`,
169+
)
170+
171+
expect(() => screen.getByTestId('foo')).toThrowError(
172+
/getByRole\("img", \{name: \/the problem \\\(picture of a question mark\\\)\/i\}\)/,
173+
)
174+
})
175+
148176
test('should suggest getByLabelText when no role available', () => {
149177
renderIntoDocument(
150178
`<label for="foo">Username</label><input data-testid="foo" id="foo" />`,

src/suggestions.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import {computeAccessibleName} from 'dom-accessibility-api'
2-
import {getRoles} from './role-helpers'
32
import {getDefaultNormalizer} from './matches'
43
import {getNodeText} from './get-node-text'
54
import {DEFAULT_IGNORE_TAGS} from './config'
5+
import {getImplicitAriaRoles} from './role-helpers'
66

77
const normalize = getDefaultNormalizer()
88

@@ -26,23 +26,25 @@ function getLabelTextFor(element) {
2626
}
2727
return undefined
2828
}
29-
29+
function escapeRegExp(string) {
30+
return string.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&') // $& means the whole matched string
31+
}
3032
function makeSuggestion(queryName, content, {variant, name}) {
3133
return {
3234
queryName,
3335
toString() {
34-
const options = name ? `, {name: /${name.toLowerCase()}/i}` : ''
36+
const options = name
37+
? `, {name: /${escapeRegExp(name.toLowerCase())}/i}`
38+
: ''
3539
return `${variant}By${queryName}("${content}"${options})`
3640
},
3741
}
3842
}
3943

4044
export function getSuggestedQuery(element, variant) {
41-
const roles = getRoles(element)
42-
43-
const roleNames = Object.keys(roles)
44-
if (roleNames.length) {
45-
const [role] = roleNames
45+
const role =
46+
element.getAttribute('role') ?? getImplicitAriaRoles(element)?.[0]
47+
if (role) {
4648
return makeSuggestion('Role', role, {
4749
variant,
4850
name: computeAccessibleName(element),

0 commit comments

Comments
 (0)