Skip to content

a11y: announce when link popup appears#65

Open
abaevbog wants to merge 6 commits into
zotero:masterfrom
abaevbog:announce_link_popup
Open

a11y: announce when link popup appears#65
abaevbog wants to merge 6 commits into
zotero:masterfrom
abaevbog:announce_link_popup

Conversation

@abaevbog
Copy link
Copy Markdown
Contributor

Per https://forums.zotero.org/discussion/122292/feature-request-allow-alt-enter-to-open-links-in-notes-for-accessibility, screen reader users have no way of knowing that the popup is there, and that shift-tab would allow them to edit/open/remove the link.

Addresses: zotero/zotero#5199

This adds an announcement that screen readers read out instructing the user to Shift-Tab to get to the link popup. This is what the announcement sounds like. The actual phrasing may need to be tweaked a bit to be more informative?

note_editor_link_popup_announce.mov

Per https://forums.zotero.org/discussion/122292/feature-request-allow-alt-enter-to-open-links-in-notes-for-accessibility,
screen reader users have no way of knowing that the
popup is there, and that shift-tab would allow them to
edit/open/remove the link.

Addresses: zotero/zotero#5199
Comment thread src/ui/editor.js Outdated
</div>
<div id='a11y-alert' aria-live='polite'>
{editorState.link.popup.active ? intl.formatMessage({ id: 'noteEditor.a11yLinkPopupAppearedAlert' }) : ""}
</div>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any reason this is using single quotes instead of double?

Should we make the div itself conditional on the popup, and not just its contents?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Any reason this is using single quotes instead of double?

It seemed to me that everything in note-editor had single-quotes, which is why I went with them. But I see that single quotes are everywhere, except for JSX, where we have double quotes. That is the overall convention, right? In any case, I changed it for double quotes.

Should we make the div itself conditional on the popup, and not just its contents?

It is more reliable to have an aria-live node and set/clear its content conditionally. The docs are not super detailed on this but the general instruction is that dynamic changes in content is what triggers announcements. So if we make the div itself conditional, there would be no technical change of content inside of the aria-live node, so screen readers may not announce anything. I've definitely ran into aria-live regions not getting read out during VPAT work for the reader when I tried to render the nodes conditionally.

After I made this PR, there was a followup comment in the forums that I think adds some more context here: https://forums.zotero.org/discussion/comment/489820/#Comment_489820. They are saying that the cursor is lost after using the popup - it's not really lost but it definitely can seem like it is. If you shift-tab into the link popup, then in order to go back to where the cursor was, you have to tab back into the editor. If you try to close the popup with Escape, the focus goes back into the toolbar, from where you need to tab back into the editor.

I thought we can also improve this by closing the link popup and returning focus to the editor if Escape is pressed in it?

While we are at it, maybe adding a shortcut to open the link as it's initially requested in the forums thread actually doesn't hurt? We already have Cmd-K to open the link popup in editing mode after all, and opening the link is possibly an even more frequent action than editing it?

I pushed the Escape handling in link popup and cmd/ctrl/shift-Enter opening the link in the editor as separate commits here, in case we don't want one or the other.

- add fluent support in zotero desktop
- set a11y alerts when link or citation popups appear
using document.l10n if it is defined, or fallback to
strings.js otherwise (e.g. in web library)
- move logic of setting the aria-live component
into a separate A11yAlert component
@abaevbog
Copy link
Copy Markdown
Contributor Author

With the last update, the aria-live announcement will be set using document.l10n available in zotero desktop, or from strings.js otherwise.

While working on this, I realized that we probably want to add similar handling to the citation popup as well. The idea is very similar - if one relies on screen reader, there is almost no way to know that you can edit a citation. So I added announcements for citation popup as well. Escape will now refocus the editor from citation popup too.

To keep the main editor.js file clean, I moved the handling of this aria-live logic and selecting where to get the string into its own UI component. This feels like the cleanest approach to me. I expect it will also be the easiest to extend later, if we need to add more announcements there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants