vpat 81: instructions for keyboard interface #142

abaevbog · 2024-10-08T07:28:03Z

added aria-descritions for buttons in the toolbar that tell what shortcuts exist and how to use them for different annotation types
added aria-description to the input in 'Find in document' popup to tell that one can create annotations from search results
when annotation is selected, announce a message saying that the annotation is selected, indicating that a popup appeared (if no sidebar), and telling how one can manipulate the annotation (more/resize/etc.)

A few minor functional tweaks:

when annotation is added from "Find in document", close the popup and focus the view. Otherwise, one is likely to close popup with Escape and then they would have to tab to the annotation again.
when a note is added via keyboard shortcut, do focus the comment, since the instruction has to be "Use ctrl-option-3 to add a note annotation", and
if comment is not focused, one would create an empty note and then have to tab all the way to the sidebar and find it before being able to edit it.

I'll be doing some more testing with different screen readers but @mrtcode I wanted to check with you if this approach makes sense

abaevbog · 2024-10-08T19:07:22Z

After a few additional tweaks, this is the current behavior and messages: link. I think this gives a good enough of info without getting in the way too much. Let me know what you think!

abaevbog · 2024-10-08T19:41:28Z

One thing worth noting is that adding underline and highlight annotations from "Find in document" popup for just screen reader users is tricky, since the search results announcements are not that informative. So far, we just read out the index of the search result, which means that if there are multiple matches, it's hard to tell which one is the right one. It should be addressed by parts of #137, where we announce the snippet and other info to distinguish between search results better.

abaevbog · 2024-10-09T18:37:21Z

This last commit should fix the regression where keyboard shortcuts would not add highlight and underline annotations from "Find in document" popup.

I think this is behaving quite well now with screen readers that I tested. It's the last tweak we would want to add before going for another round of VPAT reviews, since some issues explicitly ask for instructions, and it's probably best if they get to test the instructions part together with the new keyboard interface.

mrtcode · 2024-10-10T20:43:45Z

src/common/reader.js

 					if (!triggeringEvent || triggeringEvent.button !== 2) {
 						if (triggeredFromView) {
 							if (['note', 'highlight', 'underline'].includes(annotation.type)
-								&& !annotation.comment && (!triggeringEvent || !('key' in triggeringEvent))) {
+								&& !annotation.comment && (!triggeringEvent || !('key' in triggeringEvent) || (triggeringEvent.code.includes("Digit") && annotation.type === "note"))) {


We can probably just remove !('key' in triggeringEvent) instead of checking for digit, as it was added to prevent focusing on note annotation comment when it was created from keyboard events, as we discussed in another thread.

Right, so I then just left the annotation.type === "note" part, as we always want to let the user type once they created a note via shortcut. It does do exactly what I advocated against with other annotation type, when selecting it with Enter moves focus away into the sidebar. But I feel like in this case, it is not unreasonable, since the only thing you would want to do with an empty note is edit it.

mrtcode · 2024-10-10T20:46:00Z

src/common/reader.js

+					// After a small delay for focus to settle, announce to screen readers that annotation
+					// is selected and how one can manipulate it
+					setTimeout(() => {
+						let a11yAnnouncement = this._getString("pdfReader.a11yAnnotationSelected");


We typically use single quotes unless double quotes are necessary.

Gotcha, I'll keep it in mind! I re-pushed everything as it was easier to find the double quotes that way

mrtcode · 2024-10-10T20:48:35Z

src/common/reader.js

@@ -1197,8 +1199,37 @@ class Reader {
 						}
 						else {
 							this._lastView.navigate({ annotationID: annotation.id });
+							// When annotation is added from "Find in" popup, close it and re-focus the view
+							this.toggleFindPopup({ primary: this._lastViewPrimary, open: false });


This can probably go into find popup component as it already has a way to close itself.

mrtcode · 2024-10-10T20:56:26Z

src/common/reader.js

@@ -1197,8 +1199,37 @@ class Reader {
 						}
 						else {
 							this._lastView.navigate({ annotationID: annotation.id });
+							// When annotation is added from "Find in" popup, close it and re-focus the view
+							this.toggleFindPopup({ primary: this._lastViewPrimary, open: false });
+							this.focusView(this._lastViewPrimary);


This is a broader issue, and even closing the find popup by clicking the close button doesn't restore focus to the view. However, we might want to keep it as is for now until I figure out the best way to resolve it.

Understood, I removed that part. To give at least some indication that creating the annotation did work, I added one more announcement that will fire whenever an annotation is created (e.g. "Underline annotation has been created"). So, in this case, the popup will close and this message will be heard.

To fix regression where the Control-Option/Alt-1/2 would not work from the Find in popup, since the color was not being set.

dstillman · 2024-10-11T04:17:40Z

src/en-us.strings.js

+    'pdfReader.a11yEditTextAnnotation': 'To move the end of the text annotation, use the left/right arrow keys while holding Shift. To move the start of the annotation, use the arrow keys while holding Shift -',
+    'pdfReader.a11yResizeAnnotation': 'To resize the annotation, use the arrow keys while holding Shift.',
+    'pdfReader.a11yAnnotationSelected': 'Annotation selected',
+    'pdfReader.a11yAnnotationPopupAppeared': 'Use Tab to navigate the annotation popup.'


Is this last one necessary? Is the concern that someone wouldn't know that there's a popup, and so we have to tell them how to navigate it? Because Tab is obviously how you navigate everything.

Is the concern that someone wouldn't know that there's a popup, and so we have to tell them how to navigate it?

That's exactly it. I remember we discussed earlier that currently there is no way to know that something happened when the popup opens. So this is just a way of saying that a popup to edit the annotations appeared and it is the next focusable element. There very much might be a better way to phrase it!

No, that makes sense.

src/en-us.strings.js

- added aria-descritions for buttons in the toolbar that tell what shortcuts exist and how to use them for different annotation types - added aria-description to the input in 'Find in document' popup to tell that one can create annotations from search results - when annotation is selected, announce a message saying that the annotation is selected, indicating that a popup appeared (if no sidebar), and telling how one can manipulate the annotation (more/resize/etc.) - when annotation is added, announce that a new annotation has been created. It helps in cases like when a text annotation is added and text is immediately focused, or when underline annotation is created from Find popup and focus does not come back to the view to at least indicate that it did work. - when annotation is added from "Find in document", close the popup. - when a note is added via keyboard shortcut, focus the comment, since the instruction has to be "Use ctrl-option-3 to add a note annotation", and if comment is not focused, one would create an empty note and then have to tab to the sidebar to edit it - fix glitchy announcements of external link annotations

dstillman · 2024-10-11T05:31:18Z

src/en-us.strings.js

+    'pdfReader.a11yMoveAnnotation': 'Use the arrow keys to move the annotation.',
+    'pdfReader.a11yEditTextAnnotation': 'To move the end of the text annotation, use the left/right arrow keys while holding Shift. To move the start of the annotation, use the arrow keys while holding Shift -',
+    'pdfReader.a11yResizeAnnotation': 'To resize the annotation, use the arrow keys while holding Shift.',
+    'pdfReader.a11yAnnotationSelected': 'selected.',


These partial fragments don't work. We can do it if we really need to for keyboard shortcuts, where we're just building up a string, but we can't assume a given sentence structure for localized strings.

So we'd have to use placeholders here or just have separate strings with hard-coded annotation types.

Assuming we really need to say the annotation type, separate strings are probably the safest.

The thing that goes before "selected" or "has been created" is the annotation type. In english, it becomes "Note annotation selected". Would that still cause issues? Would it be better to do something like "Selected: " and "Created: ", followed by annotation type? Like "Selected: Note annotation"?

Just use separate strings. A fragment like that could easily not work in a given language, there could be gender differences, and at the very least it could totally confuse localizers — e.g., what is "selected."? I know it's antithetical to the programmer DRY ethos, but attempting to reuse strings is usually a bad idea when it comes to localization. (The main exception would be something like a button name — "and click {$button}".)

Gotcha. Thanks for the explanation!

The last compromise I have in mind (without repetition that really doesn't sit well with me) is to keep initial sentences and just append the annotation type? It would say e.g. "Annotation has been selected. Note annotation.". A bit clunky but just as functional.

I mean I guess the only exception here would be if screen reader users would rather hear "Annotation selected: highlight" or "Annotation selected. Highlight annotation." instead of "Highlight annotation selected" or "A highlight annotation was selected". You hear more of these strings, so you know better what the norm is. But "Highlight annotation selected" seems the cleanest to me, and that can just be done as its own string.

(So I'll maybe retract "Just use regular sentences" in this specific context, but the larger point stands — repetition is often the better option for localization.)

Regular sentences work well! Screen readers do have their own ways to announce elements and their positioning, but as far as the announcements on the application level go, I believe full sentences are as clear as it can get. So I added pdfReader.a11yAnnotationSelected.* and pdfReader.a11yAnnotationCreated.* strings for each type. I will admit that it does feel a bit cleaned than combining strings on the fly.

It occurs to me that telegraphic style avoids the problem of tense: "has been selected" or "is selected"? Present tense is maybe more normal for a screen reader? (I've mostly tested with VoiceOver: "You are in a…") So if we want to use full sentences, I'd probably go with "A highlight annotation is selected". For "created", not sure about "has been created" vs. "was created".

But while I don't actually know how other languages handle telegraphic style, we obviously use it elsewhere, so we totally can say "Highlight annotation selected" and "Highlight annotation created" here and avoid this problem (at least in English!) as long as we put them all in separate strings.

Hm, yeah. Having gotten some sleep, I see the perfect tense is adding unnecessary complexity here, and "${annotation_type} annotation selected"/"${annotation_type} annotation created" would be better for localization and also a bit more concise.

I think as far as how exactly we phrase the announcement (e.g. past vs perfect tense) doesn't really matter too much, as long as it properly informs the user what happened and where they are at. My impression is that screen readers approach these things in their own ways: e.g. Voiceover does announce "You are in ...", while NVDA just says "Grouping ..., button ...". But past tense with telegraphic style indeed seems most prevalent. For example, if you open firefox and type something (e.g. "zotero") in the url bar until autocomplete is suggested, you'll then hear "zotero.org selected instead", which is an alert announcement similar in nature to what we are doing here.

a11yAnnotationCreated* and a11yAnnotationSelected*

mrtcode · 2024-10-11T19:07:14Z

Ok, it looks good to me.

dstillman · 2024-10-12T04:26:02Z

Actually, sorry, I'm pretty sure we need placeholders here. Even something like To turn a search result into a highlight or underline annotation, press probably doesn't work in certain languages, like Turkish. We've gotten this feedback in the forums, and just asking Google Translate now, it translates "To turn a search result into a highlight or underline annotation, press Cmd-Shift-1." as Bir arama sonucunu vurgulu veya altı çizili açıklamaya dönüştürmek için Cmd-Shift-1 tuşlarına basın. For Japanese, it says 検索結果をハイライトまたは下線の注釈にするには、Cmd-Shift-1 を押します。.

@mrtcode, do you know if there's a way to do this? I don't know anything about the reader's localization system.

mrtcode · 2024-10-12T06:54:15Z

@mrtcode, do you know if there's a way to do this? I don't know anything about the reader's localization system.

No, we currently use only plain strings in the reader. Some are rendered through react-intl, while others are used directly. For now, the best approach is to manually replace them using something like this._getString('pdfReader.a11ySomething').replace('%S', 'Cmd-Shift-1').

To achieve formatting like in Zotero.getString, we would need to duplicate its code into the reader...

mrtcode · 2024-10-12T06:59:21Z

Or we could start using Fluent in the Zotero reader and switch all the strings.

dstillman · 2024-10-12T09:07:48Z

If switching to Fluent is realistic, we should do that — it makes localization both more powerful and safer.

A stopgap option, in order to get this out sooner, would be to add these as Fluent strings with Fluent placeholders and feed those into the reader, and then manually replace those in the code for now.

abaevbog · 2024-10-13T06:52:09Z

A stopgap option, in order to get this out sooner, would be to add these as Fluent strings with Fluent placeholders and feed those into the reader, and then manually replace those in the code for now.

I didn't assume that fluent would just work in the reader - but it does? I added <link rel="localization" href="zotero.ftl"/> into root index.reader.html, and with data-l10n-id="pdfReader-a11yAnnotationInstruction" data-l10n-args='{"number": "1"}' added to one of the annotation tool buttons, I got a valid aria-label on it with (with zotero.ftl also containing pdfReader-a11yAnnotationInstruction = .aria-label = To add this annotation into the document, focus the document and press Control - { pdfReader-a11y-modifier } - { $number }). So is there a reason to not use that just for all of the recent a11y strings?

dstillman · 2024-10-13T07:10:39Z

Oh! OK then!

But I guess the problem is that the reader is also used elsewhere (e.g., in the web library) where we won't have automatic Fluent support. Presumably we can just use Fluent.js, but it will have to be added to the non-client build process. What we have now with the duplicated en-US-only strings is dumb anyway, though, and we want to be able to localize non-client contexts, so we should have a reader.ftl in the client, use the automatic support for that in client builds, and plan to bundle those localized files and fluent.js as part of the non-client build process. We won't want to create a circular dependency by making zotero/zotero a submodule of zotero/reader, though, so we'll need to do something else there — maybe just pulling files from a fixed zotero/zotero commit hash on GitHub? I'll defer to @mrtcode there.

mrtcode · 2024-10-14T08:01:01Z

I think we can start by using Fluent for Zotero build to handle a11y strings for now, and introduce Fluent.js in the future. While the a11y strings won’t be functional in the web library, this shouldn't cause any degradation in existing functionality.

dstillman · 2024-10-14T09:27:20Z

OK, sure. So we'll remove these new strings from en-us.strings.js and add to a reader.ftl in the client.

@abaevbog: Note that after creating reader.ftl, it needs to be added to ftlFileBaseNames in js-build/config.js.

Per: zotero/reader#142 (comment)

abaevbog · 2024-10-14T21:36:23Z

This makes sense to me. Since web-library in general has only a subset of the desktop functionality, I think it is reasonable to focus on the desktop app for now. Besides, I don't think web library is a part of the VPAT review.

In these PRs, I added the reader.ftl to the client and changed all announcements/labels we just added to use document.l10n or data-l10n-id. I gotta say, it is way nicer to handle this with fluent, so a lot of logic putting announcements together could be cleaned up too.

In the reader, nodes with data-l10n-id should just be not affected, and aria-live announcements are explicitly skipped if document.l10n does not exist.

Per: zotero/reader#142 (comment)

abaevbog requested a review from mrtcode October 9, 2024 18:37

mrtcode reviewed Oct 10, 2024

View reviewed changes

set annotation colors in Find in popup

38b9453

To fix regression where the Control-Option/Alt-1/2 would not work from the Find in popup, since the color was not being set.

dstillman force-pushed the vpat_81_add_announcements branch from 4658c8c to be614e2 Compare October 11, 2024 04:16

dstillman reviewed Oct 11, 2024

View reviewed changes

src/en-us.strings.js Show resolved Hide resolved

abaevbog and others added 3 commits October 10, 2024 22:16

Tweak accessibility instruction strings

5d0ad13

do not repeat "Annotation" in messages

c7f2e51

abaevbog force-pushed the vpat_81_add_announcements branch from be614e2 to c7f2e51 Compare October 11, 2024 05:26

a couple of missed double quotes

e5f1456

dstillman reviewed Oct 11, 2024

View reviewed changes

abaevbog added 2 commits October 10, 2024 23:15

Separate l10n string for each annotation state

c1c8065

a11yAnnotationCreated* and a11yAnnotationSelected*

telegraphic style for messages

c3ba633

Remove trailing periods from strings

d0528f9

dstillman merged commit ffc5593 into zotero:master Oct 12, 2024
1 check passed

dstillman mentioned this pull request Oct 12, 2024

Use Fluent.js and convert all existing reader strings #143

Open

abaevbog added a commit to abaevbog/zotero that referenced this pull request Oct 14, 2024

reader.ftl file for a11y strings in the reader

9cbe85c

Per: zotero/reader#142 (comment)

This was referenced Oct 14, 2024

Use fluent to format a11y strings in reader #144

Merged

reader.ftl file for a11y strings in the reader zotero/zotero#4752

Merged

dstillman pushed a commit to zotero/zotero that referenced this pull request Oct 15, 2024

reader.ftl file for a11y strings in the reader (#4752)

cde21ac

Per: zotero/reader#142 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vpat 81: instructions for keyboard interface #142

vpat 81: instructions for keyboard interface #142

abaevbog commented Oct 8, 2024 •

edited

Loading

abaevbog commented Oct 8, 2024

abaevbog commented Oct 8, 2024

abaevbog commented Oct 9, 2024

mrtcode Oct 10, 2024

abaevbog Oct 11, 2024

mrtcode Oct 10, 2024

abaevbog Oct 11, 2024

mrtcode Oct 10, 2024

mrtcode Oct 10, 2024

abaevbog Oct 11, 2024

dstillman Oct 11, 2024

abaevbog Oct 11, 2024

dstillman Oct 11, 2024

dstillman Oct 11, 2024

dstillman Oct 11, 2024

abaevbog Oct 11, 2024

dstillman Oct 11, 2024

abaevbog Oct 11, 2024

dstillman Oct 11, 2024 •

edited

Loading

dstillman Oct 11, 2024

abaevbog Oct 11, 2024

dstillman Oct 11, 2024

abaevbog Oct 11, 2024 •

edited

Loading

mrtcode commented Oct 11, 2024

dstillman commented Oct 12, 2024

mrtcode commented Oct 12, 2024

mrtcode commented Oct 12, 2024

dstillman commented Oct 12, 2024

abaevbog commented Oct 13, 2024

dstillman commented Oct 13, 2024

mrtcode commented Oct 14, 2024 •

edited

Loading

dstillman commented Oct 14, 2024

abaevbog commented Oct 14, 2024

vpat 81: instructions for keyboard interface #142

vpat 81: instructions for keyboard interface #142

Conversation

abaevbog commented Oct 8, 2024 • edited Loading

abaevbog commented Oct 8, 2024

abaevbog commented Oct 8, 2024

abaevbog commented Oct 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstillman Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abaevbog Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

mrtcode commented Oct 11, 2024

dstillman commented Oct 12, 2024

mrtcode commented Oct 12, 2024

mrtcode commented Oct 12, 2024

dstillman commented Oct 12, 2024

abaevbog commented Oct 13, 2024

dstillman commented Oct 13, 2024

mrtcode commented Oct 14, 2024 • edited Loading

dstillman commented Oct 14, 2024

abaevbog commented Oct 14, 2024

abaevbog commented Oct 8, 2024 •

edited

Loading

dstillman Oct 11, 2024 •

edited

Loading

abaevbog Oct 11, 2024 •

edited

Loading

mrtcode commented Oct 14, 2024 •

edited

Loading