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.

Sign up for GitHub

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

Jump to bottom

Add aria-expanded to getAccessibilityTree and <details>/<summary> #451

Merged
merged 1 commit into from Mar 31, 2022
Merged

Add aria-expanded to getAccessibilityTree and <details>/<summary> #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions .changeset/silver-actors-jam.md
View file
Open in desktop
@@ -0,0 +1,9 @@
---
'pleasantest': major
---

Added `aria-expanded` support to `getAccessibilityTree` and fix handling for `<details>`/`<summary>`

Now, elements which have the `aria-expanded` attribute will represent the state of that attribute in accessibility tree snapshots. `<details>`/`<summary>` elements will represent their expanded state in the tree as well.

Also, for collapsed `<details>`/`<summary>` elements, the hidden content is now hidden in the accessibility tree, to match screen reader behavior.
20 changes: 19 additions & 1 deletion src/accessibility/browser.ts
View file
Open in desktop
Expand Up @@ -44,7 +44,13 @@ const getElementAccessibilityState = (element: Element): AccessibilityState => {
if (
(element as HTMLElement).hidden ||
element.getAttribute('aria-hidden') === 'true' ||
computedStyle.display === 'none'
computedStyle.display === 'none' ||
(element.parentElement?.tagName === 'DETAILS' &&
!(element.parentElement as HTMLDetailsElement).open &&
!(
element.tagName === 'SUMMARY' &&
element.parentElement.firstElementChild === element
))
)
return AccessibilityState.SelfAndDescendentsExcludedFromTree;

Expand Down Expand Up @@ -97,6 +103,18 @@ export const getAccessibilityTree = (
if (selfIsInAccessibilityTree) {
const name = computeAccessibleName(element);
if (name) text += ` "${name}"`;
if (
element.ariaExpanded === 'true' ||
(element.tagName === 'SUMMARY' &&
(element.parentElement as HTMLDetailsElement).open)
)
text += ` (expanded=true)`;
if (
element.ariaExpanded === 'false' ||
(element.tagName === 'SUMMARY' &&
!(element.parentElement as HTMLDetailsElement).open)
)
text += ` (expanded=false)`;
if (document.activeElement === element) text += ` (focused)`;
if (includeDescriptions) {
const description = computeAccessibleDescription(element);
Expand Down
65 changes: 65 additions & 0 deletions tests/accessibility/getAccessibilityTree.test.ts
View file
Open in desktop
Expand Up @@ -281,3 +281,68 @@ test(
`);
}),
);

test(
'<details>/<summary>',
withBrowser(async ({ utils, page, screen, user }) => {
await utils.injectHTML(`
<details>
<summary>
Click me!
<h1>Tags in summary do not preserve their semantic meaning</h1>
</summary>
<p>Some content</p>
<h2>Tags in details do preserve their semantic meaning</h2>
</details>
`);

// Starts collapsed
expect(await getAccessibilityTree(page)).toMatchInlineSnapshot(`
document
group
button "Click me! Tags in summary do not preserve their semantic meaning" (expanded=false)
`);

const toggle = await screen.getByText(/click me!/i);
await user.click(toggle);

// After toggling it should be expanded
expect(await getAccessibilityTree(page)).toMatchInlineSnapshot(`
document
group
button "Click me! Tags in summary do not preserve their semantic meaning" (expanded=true) (focused)
Copy link
Member Author

Choose a reason for hiding this comment

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

This behavior is according to spec (the <h1> inside of <summary> loses its semantic meaning). But, the behavior does not match voiceover in safari, or chrome or firefox's accessibility tree.

According to the spec:

It would be more difficult for us to diverge from the spec and implement it the way browsers do. Chrome accessibility tree, firefox accessibility tree, and safari voiceover all chose not to use the button role for <summary> elements. We could add a special override here but I would rather not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here are what browsers use for <summary> elements:

  • Chrome devtools accessibility tree: DisclosureTriangle
  • Firefox devtools accessibility tree: summary
  • Safari VO: summary, group

Copy link
Member

Choose a reason for hiding this comment

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

I think it's reasonable to leave this matching the spec. Might be good to add a "known issues" section in the README, though, outlining things like this that might otherwise trigger bug reports.

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 would be open to people opening issues for their use cases where they run into this difference, where we can continue the conversation of whether it makes sense to match the spec here

text "Some content"
heading "Tags in details do preserve their semantic meaning"
text "Tags in details do preserve their semantic meaning"
`);
}),
);

test(
'aria-expanded and aria-collapsed',
withBrowser(async ({ utils, page }) => {
await utils.injectHTML(`
<button aria-expanded="false">Click me!</button>
`);
expect(await getAccessibilityTree(page)).toMatchInlineSnapshot(`
document
button "Click me!" (expanded=false)
`);

await utils.injectHTML(`
<button aria-expanded="true">Click me!</button>
`);
expect(await getAccessibilityTree(page)).toMatchInlineSnapshot(`
document
button "Click me!" (expanded=true)
`);

await utils.injectHTML(`
<button>Click me!</button>
`);
expect(await getAccessibilityTree(page)).toMatchInlineSnapshot(`
document
button "Click me!"
`);
}),
);