feat(dom): toHaveAccessibleDescription - #174
Conversation
…json and yarn.lock
…n and update related tests
…ion and normalizeText helper
| const expectation = expectedDescription instanceof RegExp | ||
| ? `matching ${expectedDescription}` | ||
| : `"${expectedDescription}"`; |
There was a problem hiding this comment.
A minor suggestion here, to consider only constructing expectation when hasExpectedValue is true. Otherwise, it evaluates to the string "undefined" when no argument is passed, which isn't currently exposed but could be misleading or cause issues if this code changes later.
const expectation = hasExpectedValue
? expectedDescription instanceof RegExp
? `matching ${expectedDescription}`
: `"${expectedDescription}"`
: "";
| @@ -327,25 +328,24 @@ export class ElementAssertion<T extends Element> extends Assertion<T> { | |||
| : desc === expectedDescription; | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
Non-blocking suggestion: this helper is clear as is, but it might be worth extracting it into helpers/accessibility.ts as we would end up adding other matchers in the future (e.g. toHaveAccessibleName) that could reuse the same matching logic 😸 let me know what you think !
There was a problem hiding this comment.
Thanks, sounds good!
…proved accessibility checks
…mproved expectation descriptions
| .join(" "); | ||
|
|
||
| return normalizeText(combinedText); | ||
| return expected instanceof RegExp |
There was a problem hiding this comment.
why are we returning this?
There was a problem hiding this comment.
Because expected is RegExp | string — the check picks the right comparison at runtime and narrows the type so .test() type-checks.
GonuDvc
left a comment
GonuDvc
left a comment
There was a problem hiding this comment.
Approach is solid. Minor nit: this would benefit from a brief comment explaining why we skip validation for internal callers.
| @@ -1,35 +1,24 @@ | |||
| function normalizeText(text: string): string { | |||
| return text.replace(/\s+/g, " ").trim(); | |||
| export function isValidAriaPressed(element: Element): boolean { | |||
There was a problem hiding this comment.
good catch on cleanning up this helper 🚀
|
|
||
| public toHaveDescription(expectedDescription?: RegExp | string): this { | ||
| const description = getAccessibleDescription(this.actual); | ||
| public toHaveAccessibleDescription(expectedDescription?: RegExp | string): this { |
There was a problem hiding this comment.
what do you think about adding an alias to toHaveDescription
There was a problem hiding this comment.
I'll leave it as is. toHaveDescription is ambiguous: it reads like it's checking a description attribute or raw text content rather than the ARIA-computed description. This ticket is also moving toward covering the ARIA-related assertions, so the explicit name fits better.
4 participants
Added to Have Accessible Description.