VPAT: aria edits

[abaevbog]

Address https://github.com/zotero/zotero/issues/4220 and easier parts of https://github.com/zotero/zotero/issues/4222

• [X] vpat 60: added role=button and aria-label to X clear icon of search annotations so it's clear what that button does
• [X] vpat 61: hide magnifying glass of search annotations field from screen readers, since it should never be focusable
• [X] vpat 62: explicitly refocus search annotations input field on X click to not loose focus (possible with click through voiceover shortcut)
• [X] vpat 64: added aria labels to annotation preview inputs in the sidebar and popup
• [X] vpat 65: added title to context menu button so screen readers announce what that button does
• [X] vpat 69: more descriptive aria-label for "tags" button in annotations preview.
• [X] vpat 71: added aria-labelledby and aria-describedby to the actual annotation. The first thing that will be announced is the header ("Page ...") followed by the editable area (comment or annotation text)
• [X] vpat 73: added tabs semantics to the sidebar component. vpat 72 states that we should have an audible announcement after a new tab was selected. This does not seem exactly correct per https://www.w3.org/WAI/WCAG21/Understanding/status-messages.html#excepted-examples, however the sidebar should still have proper tabbar semantics to help screen readers handle focus. That way, when a different tab is selected, the screen readers tend to repeat the title of the tab with a note that it is selected, so this should address that concern.
• [X] vpat 74: added aria-description to tags in selector to make it clear that clicking on a tag will filter the annotations by it. Also, added role=checkbox with aria-selected to indicate which state each tag is in.
• [X] vpat 79: added aria-label to "Edit page number" input

[abaevbog]

@mrtcode these are my fixes to simpler accessibility issues within the reader brought up by VPAT reviewers. Let me know what you think!

A few notes:

• I kept these as a single PR so that it's possible to squash all commits into one before merging to keep the commit history somewhat clean. But correct me if I should break these into individual PRs so that it's easier to review?
• There is a glitch in react 17 that logs a warning as if aria-description is not a valid aria property: https://github.com/facebook/react/issues/21035. It should be gone in react 18.
• To add these accessibility fixes, I had to add some new localized strings. I don't fully understand how the localization workflow happens with the reader but I added each of the new strings that we would want into en-us.strings.js for reference. But to get them to be actually announced, I had to add those strings into zotero.properties of the main repo. Is there somewhere else I should place the new strings?

[mrtcode]

I kept these as a single PR so that it's possible to squash all commits into one before merging to keep the commit history somewhat clean. But correct me if I should break these into individual PRs so that it's easier to review?

I think it's fine as it is. I don't have a strong opinion on whether we want to squash the commits or not.

There is a glitch in react 17 that logs a warning as if aria-description is not a valid aria property: https://github.com/facebook/react/issues/21035. It should be gone in react 18.

That warning is a bit annoying, but I'll suppress it, so it's fine.

To add these accessibility fixes, I had to add some new localized strings. I don't fully understand how the localization workflow happens with the reader but I added each of the new strings that we would want into en-us.strings.js for reference. But to get them to be actually announced, I had to add those strings into zotero.properties of the main repo. Is there somewhere else I should place the new strings?

The reader gets its localized strings from Zotero with the pdfReader and general prefixes. Even though we are now mostly using zotero.ftl, the reader can only use strings from zotero.properties. When using the reader in the browser, it falls back to using en-us.strings.js.

Anyway, it looks good, and I think you can merge it.

[abaevbog]

I added here a little tweak to make the annotation text focusable without it being editable. I think it achieves what we want: the screen readers being able to read the text without having to enable editing first through the menu.

Even though we are now mostly using zotero.ftl, the reader can only use strings from zotero.properties

Ok, gotcha! So before updating the reader submodule in the main repo, we should make sure to add the new lines to zotero.properties:

pdfReader.annotationComment = Annotation comment
pdfReader.annotationText = Annotation text
pdfReader.manageTags = Click to manage tags
pdfReader.openMenu = Open menu
pdfReader.thumbnails = Thumbnails
pdfReader.tagSelectorMessage = Filter annotations by this tag

Anyway, it looks good, and I think you can merge it.

That's great! It looks like I don't have write permissions to the reader repo though. So there's no "Merge" button on my end here.