Skip to content

[B] Fix Annotation a11y issues#3932

Merged
jen-castiron merged 18 commits intoa11yfrom
jw/a11y
Nov 5, 2025
Merged

[B] Fix Annotation a11y issues#3932
jen-castiron merged 18 commits intoa11yfrom
jw/a11y

Conversation

@jen-castiron
Copy link
Contributor

@jen-castiron jen-castiron commented Oct 8, 2025

Changes

All changes below were to the annotations drawer or annotation popup

[B] Add uuid to "Annotate this passage…" textarea (Row 30)

[B] Set annotation textarea id & label to uuid (Row 28)

[B] Fix collapsible content wrapper in annotation dialog (Row 32 & 48)
-- Separates button from content
-- Adds focus to content when opened
-- Adds show/hide label
-- Wrap annotations in blockquotes, include cite tag

[B] Change underlined annotation tag from span to mark (Row 48)

[B] Use bright hover and focus color on the annotation menu, add underline (Row 47)
-- Resolves issue where the active and hover states were too similar

[B] Switch report flag modal to native dialog; handle esc (lauren commit)
[B] Split into new global modal, fix up new dialog modal styles

[B] Fix list markup for annotation comments (Row 44)
-- If there's only one entry in the list, the list should have role=presentation

[B] Fix restore focus after removing a highlight
-- Cherry-picked from Lauren's work

[B] Fix a11y and return focus on edit annotation (Row 35)
-- Uses dialog popup (non-modal dialog) a11y

[B] Add role=group around options on annotation submenu
-- Row 23, Audit 1

[E] Add 'go to passage' to reader annotation drawer
-- Fixes issue where focus on drawer close might not be where users expect it
-- Some users may expect the focus to return to the Notes button, while some may want the focus to return to the annotation
-- This adds a 'go to passage' button that will close the drawer and focus on the annotation

@jen-castiron jen-castiron changed the title [B] Fix a11y issues in login form [B] Fix a11y issues Oct 8, 2025
@jen-castiron jen-castiron force-pushed the jw/a11y branch 2 times, most recently from eef3c2b to 0226fc2 Compare October 16, 2025 22:35
@jen-castiron jen-castiron force-pushed the jw/a11y branch 2 times, most recently from 7e7d963 to 222d409 Compare October 18, 2025 00:51
@jen-castiron

This comment was marked as resolved.

@jen-castiron

This comment was marked as resolved.

@jen-castiron jen-castiron force-pushed the jw/a11y branch 2 times, most recently from 7ad700f to 78050e1 Compare October 20, 2025 19:06
@jen-castiron jen-castiron marked this pull request as draft October 20, 2025 19:09
@jen-castiron jen-castiron force-pushed the jw/a11y branch 4 times, most recently from cba23d9 to 1c16aae Compare October 20, 2025 20:45
@dananjohnson

This comment was marked as resolved.

@dananjohnson

This comment was marked as resolved.

@jen-castiron jen-castiron force-pushed the jw/a11y branch 6 times, most recently from 8d7a495 to 34d00b7 Compare October 22, 2025 00:57
@jen-castiron jen-castiron changed the title [B] Fix a11y issues [B] Fix Annotation a11y issues Oct 22, 2025
@jen-castiron jen-castiron marked this pull request as ready for review October 22, 2025 01:05
@jen-castiron jen-castiron marked this pull request as ready for review October 30, 2025 17:24
@jen-castiron
Copy link
Contributor Author

@dananjohnson This PR is ready for review. There's one focus issue left, but the rest of the issues in this PR are ready

Copy link
Member

@dananjohnson dananjohnson left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all these things, @jen-castiron! I noted a few things in my comments. There's also an issue with the flag modal that @1aurend should take a look at.

Copy link
Member

Choose a reason for hiding this comment

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

@1aurend I'm seeing some issues with the focus trap for the flag modal.

  • If you tab through to the Cancel button, tabbing again doesn't move focus any further. It just stays on Cancel.
  • If you tab in reverse, the trap does correctly cycle through the dialog elements, but it also focuses on the body element as part of the loop.

Copy link
Member

Choose a reason for hiding this comment

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

There are also some warnings in the console about missing required props, id and dialogId.

Copy link
Contributor

@1aurend 1aurend Nov 5, 2025

Choose a reason for hiding this comment

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

I pushed a fix for the ids. It looked like the prop just got missed when the dialog was switched to the new global component. However, for the focus, the behavior I'm seeing matches what's in these discussions:
whatwg/html#8339
(https://github.com/w3c/wcag/issues/4185
It seems like you're expecting something different?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I don't think this is quite right, at least in Chrome and Safari. Firefox cycles focus between the interactive elements in the modal and the browser chrome. That seems consistent with modal dialogs on other sites that I checked. In Chrome and Safari, when focus moves out of the modal, it gets stuck on the body element and won't come back to the modal. Oddly, that's a bit different than what I saw when I first reported the issue. 🤔 Neither matches other implementations, though, or what I'd expect, which makes me wonder whether the underlying drawer's focus trap is interfering. Does it need to be paused? I can show you what I'm seeing at confab if it'd be helpful.

@jen-castiron jen-castiron force-pushed the jw/a11y branch 3 times, most recently from 0798d80 to 05ca9f5 Compare October 31, 2025 19:55
Copy link
Member

@dananjohnson dananjohnson left a comment

Choose a reason for hiding this comment

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

Jen, everything is looking good. Once @1aurend addresses the flag modal issue, this PR should be good to go.

@1aurend
Copy link
Contributor

1aurend commented Nov 5, 2025

@jen-castiron @dananjohnson fixed my goof with the helper export. Sorry about that! I think reverting to not using a named export will be smoothest for rebasing resources.

@jen-castiron jen-castiron changed the base branch from master to a11y November 5, 2025 20:29
jen-castiron and others added 18 commits November 5, 2025 12:30
- Separates button from content
- Adds focus to content when opened
- Adds show/hide label
- Wrap annotations in blockquotes, include cite tag
- Resolves issue where the active and hover states were too similar
- If there's only one entry in the list, the list should have role=presentation
-- Uses dialog popup (non-modal dialog) a11y
- Fixes issue where focus on drawer close might not be where users expect it
- Some users may expect the focus to return to the Notes button, while some may want the focus to return to the annotation
- This adds a 'go to passage' button that will close the drawer and focus on the annotation
@jen-castiron jen-castiron merged commit d4cced3 into a11y Nov 5, 2025
3 checks passed
@jen-castiron jen-castiron deleted the jw/a11y branch November 5, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants