zotero / reader

PDF/EPUB/HTML reader for Zotero
Other
129 stars 30 forks source link

Keyboard accessibility improvements #138

Closed mrtcode closed 1 month ago

mrtcode commented 2 months ago

This PR implements the changes discussed in zotero/zotero#4224, but while doing that it also triggered other keyboard accessibility changes throughout the reader code. Once we agree on the changes and merge this PR, similar changes can be implemented in EPUB and snapshot views.

As discussed in zotero/zotero#4224:

I used Cmd/Ctrl instead of Option/Alt for highlight/underline left side resizing because the Option key is used for word selection on macOS, and resizing highlight/underline is essentially adjusting text selection (Update: this word selection mode currently doesn't work in PDF view text selection layer, but it will in future). However, should we consider doing the opposite and using Cmd/Ctrl for resizing other annotations as well?

Additional changes worth mentioning:

AbeJellinek commented 2 months ago

Some initial thoughts after trying it out a bit:

AbeJellinek commented 2 months ago

Overall, though, everything is working pretty well and this is a huge improvement in keyboard usability!

(I'll be away tomorrow through the 20th, but I can get on implementing the same for EPUB/snapshot once I'm back.)

mrtcode commented 2 months ago
  • The inconsistency between keyboard shortcuts for highlights/underlines and other annotation types is a little confusing. It feels weird that I need to use Shift + Option/Alt + Arrow Keys to resize an image annotation, but Shift + Option/Alt + Arrow Keys on a highlight does nothing at all.

I agree. However, in future versions, Option/Alt will be used for word selection during regular text selection (and it already is in EPUB and snapshot views), and it would be odd if, for highlights/underlines, it moves the left resizer. That said, I’m not happy with this inconsistency either. But what to do?

  • When I select some text, scroll away from it, and press Ctrl-Alt-1 to create a highlight, the highlight is created offscreen and isn't scrolled into view. Maybe it should be?
  • Ctrl-Alt-3 -> Delete leaves behind a ghost focus ring in the view. Can't create a new annotation via keyboard until I dismiss it by pressing Escape a couple times.

Good catches!

  • Lots of jank in the annotations pane while moving around an image annotation with the arrow keys. (Not that important.)

Yes, that's a good observation. Maybe in future versions it will debounce image annotation rerendering or pause it if Shift is pressed.

  • When I have a link focused in the PDF, I think the spacebar should activate it.

Sure.

Overall, though, everything is working pretty well and this is a huge improvement in keyboard usability!

(I'll be away tomorrow through the 20th, but I can get on implementing the same for EPUB/snapshot once I'm back.)

No need to rush this. It’s still quite buggy and requires thorough testing, as we don’t want to disrupt Zotero at this stage.

abaevbog commented 2 months ago

This is great! I think the keyboard shortcuts approach we took is quite intuitive and in general everything does work quite well. A few minor things I paid attention to after a bit of playing around:


I do have one question. Currently, once I have the new focus ring on the annotation, I have to press Enter for it to get selected for the resizing keyboard shortcuts to take effect. Isn't this somewhat close to the resizing mode that we decided against? Why is the selection of the annotation via Enter necessary in the first place? We tried to pick the shortcuts with Shift, Ctrl and Alt/Option modifiers so they don't interfere with unmodifier arrow keypresses for navigation between annotations. So would it be better to skip the Enter step and have Shift-Arrow (and all other keypresses) move/resize the annotation right away (when the focus outline is on it but it's not explicitly selected)? Alternatively, if we do want Enter to be needed to activate an annotation for resizing shortcuts to be available, maybe we could then use unmodified arrows keys to move annotations instead of scrolling the page? Currently, once the annotation is selected, Shift-Arrow will move it but unmodified up/down arrows will scroll the page and right/left will go to the next/previous page. I think if you are working on an annotation, you are more likely to expect unmodified arrow keypresses to do something to that annotation as opposed to scrolling the page.

Final observation here is that Enter on a focused underline or highlight annotation without a comment will also focus an empty comment field. Then, the first resizing shortcut keypress (e.g Shift-RightArrow) will just blur the comment field and the next keypress will do the resizing. It would be a bit hard to explain via a concise verbal announcement to a screen reader user this setup and I, personally, find it a bit confusing as well. If the resizing shortcut keypress were to select the annotation and begin moving it, we could know that the comment field does not need to be focused.

Sorry if that last part got a bit too verbose in the end but hopefully it made sense!

mrtcode commented 2 months ago

Thanks for the great insights!

  • One more instance of a focus outline appearing where the annotation no longer exists: focus a note annotation, press Enter to be able to move it, hold Shift-ArrowDown to move it, press Escape to be done -> an outline will appear where the note was before we moved it.

Ok, I'll fix.

  • When an annotation is added via keyboard shortcut, maybe we should immediately focus it? At least, when done via "Find in document". Currently, if you have enough annotations to fill all the space in the sidebar, you search for a text in the end of the document and then press Ctrl + Option+ 2 to make it an annotation, there is almost no indication that something changed because you won't see the annotation appear in the sidebar (if it went to the bottom). It's quite hard to notice the annotation appearing in the document too because that text is also being highlighted as a search result. So I accidentally made 5 annotations because I thought that I was pressing the wrong keys. If you are making an annotation, I suppose it's reasonable that you would want to engage with it right after to make a comment or leave a tag or something?

Good point, I'll fix.

  • Maybe Enter after resizing could do the same thing as Escape by leaving the resizing mode and returning to the focus mode with the new outline? I found myself selecting an annotation, moving it, and then thinking "Ok, this is good, I'm done, submit, save, etc.", so my impulse was to press Enter as opposed to Escape.

I think Enter might be a bit too much here and we could keep it for more important tasks. Also:

  • As mentioned above, as an image annotation (especially a long one) is being moved across the page, it seems to disappear and re-appear in sidebar, which looks like continuous blinking

Ok, I'll try to fix that.

I do have one question. Currently, once I have the new focus ring on the annotation, I have to press Enter for it to get selected for the resizing keyboard shortcuts to take effect. Isn't this somewhat close to the resizing mode that we decided against? Why is the selection of the annotation via Enter necessary in the first place? We tried to pick the shortcuts with Shift, Ctrl and Alt/Option modifiers so they don't interfere with unmodifier arrow keypresses for navigation between annotations. So would it be better to skip the Enter step and have Shift-Arrow (and all other keypresses) move/resize the annotation right away (when the focus outline is on it but it's not explicitly selected)?

However, if the annotation is selected with the mouse, you still need to be able to move it. Allowing movement in both modes (focus and selection) would be weird.

Alternatively, if we do want Enter to be needed to activate an annotation for resizing shortcuts to be available, maybe we could then use unmodified arrows keys to move annotations instead of scrolling the page? Currently, once the annotation is selected, Shift-Arrow will move it but unmodified up/down arrows will scroll the page and right/left will go to the next/previous page. I think if you are working on an annotation, you are more likely to expect unmodified arrow keypresses to do something to that annotation as opposed to scrolling the page.

Now that annotation navigation with arrow keys is only possible from the annotation view, we can try using the arrow keys without a modifier, as you suggested. However, I'm a bit concerned it might make it too easy to move annotations, especially if the arrow keys will resize the right side of highlight/underline annotations.

Final observation here is that Enter on a focused underline or highlight annotation without a comment will also focus an empty comment field. Then, the first resizing shortcut keypress (e.g Shift-RightArrow) will just blur the comment field and the next keypress will do the resizing. It would be a bit hard to explain via a concise verbal announcement to a screen reader user this setup and I, personally, find it a bit confusing as well. If the resizing shortcut keypress were to select the annotation and begin moving it, we could know that the comment field does not need to be focused.

I agree that it's confusing. However, if we use the arrow keys for moving, it won't work for selecting and moving the annotation. One option could be to do automatic focusing on the empty comment field only when the annotation is clicked with the mouse. But in that case, if you create an annotation with the keyboard and select it, you won't be able to quickly start typing into the comment field.

abaevbog commented 2 months ago

However, if the annotation is selected with the mouse, you still need to be able to move it. Allowing movement in both modes (focus and selection) would be weird.

That would totally be weird.

Now that annotation navigation with arrow keys is only possible from the annotation view, we can try using the arrow keys without a modifier, as you suggested. However, I'm a bit concerned it might make it too easy to move annotations, especially if the arrow keys will resize the right side of highlight/underline annotations.

That is a fair point as well. We do want to make sure that these actions are very intentional...

Would it make sense to select the focused annotation when either of those shortcuts is used? That may offer the benefits of not requiring an extra Enter keypress without making it likely that someone would change an annotation unintentionally and or having this functionality in both focused and selected modes?

One option could be to do automatic focusing on the empty comment field only when the annotation is clicked with the mouse. But in that case, if you create an annotation with the keyboard and select it, you won't be able to quickly start typing into the comment field.

I think this is actually a good idea, regardless which way we go with the keys used for annotation movements. While focusing the comment right away does offer the benefit of making adding a comment faster, I do think that the drawback of a less intuitive interface for screen readers may outweigh them. Predictability is an important factor, and the fact that sometimes selecting an annotation will focus a textfield and sometimes it won't is not necessarily predictable. If you don't know that this is how it works and rely on screen reader announcements, you wouldn't know which comment field you're in and why, so we would want to add more announcements to explain it, and I think it can get messy. I can't find a specific guidance that would be relevant here because this interface is quite complex and most WCAG examples are quite basic but change of context not explicitly caused by the user is generally a bad thing (e.g. in Tools > Developer > Translator Editor, switching between tabs will also focus the first element with each tab, and it was marked as a problem by the VPAT review). And in this case, we always select and focus the annotation and, in addition to that, move focus into the textfield, so I think this example is somewhat relevant here.

mrtcode commented 2 months ago

Would it make sense to select the focused annotation when either of those shortcuts is used? That may offer the benefits of not requiring an extra Enter keypress without making it likely that someone would change an annotation unintentionally and or having this functionality in both focused and selected modes?

It depends on whether we decide to use the arrow keys without a modifier. If we do, the arrow keys will simply move the focus to another object, instead of selecting the annotation.

mrtcode commented 2 months ago

After further thought, I made the following changes:

Please let me know your thoughts.

abaevbog commented 2 months ago

This is great! This collection of shortcuts feels very good to me - arrows feel right to move objects around and Shift(+Cmd)+Arrows does fit well underline and highlight, since it's almost like text selection. Ghost outline and image flickering is also gone for me.

So far, the only additional thought I have is if we should close the find popup after an annotation is created via shortcut as well? I imaging once you add the annotation, you're done with the search process and are ready to move on to the next step - add some comments/tags, or just keep reading. And, if you do want to edit the annotation after it's made, when you close the popup, the selected annotation gets un-selected, so you would have to navigate to find it again.

I'll play around to see if I have any more thoughts!

mrtcode commented 2 months ago

So, if there are no objections from anyone, I'll thoroughly test and merge the code.

We can always make small adjustments in the future.

The DOM view could also have this implemented later, possibly once we are certain users don’t object to the changes (@AbeJellinek).

AbeJellinek commented 2 months ago

Works for me!

abaevbog commented 2 months ago

Sounds great!

AbeJellinek commented 1 month ago

I'll get on implementing this for EPUB/snapshot if OK with you @mrtcode

abaevbog commented 1 month ago

Awesome! There are some tiny conflicts in https://github.com/zotero/reader/pull/137 now (nothing major). I assume it's best for me wait for the EPUB before resolving, in case we have some additional conflicts there.

AbeJellinek commented 1 month ago

Oh, forgot about

possibly once we are certain users don’t object to the changes

I'll still do a little initial work, but I think that's reasonable.

AbeJellinek commented 2 weeks ago

Working on this for EPUB/snapshot, since it doesn't seem like we've gotten any complaints.

Should Shift + Option/Alt + arrow keys resize highlights by word boundaries? That would track with the behavior on macOS (Option + arrow keys selects by word) and would be pretty convenient. I don't think it's too confusing that it "conflicts" with the text/image/ink shortcuts.

mrtcode commented 2 weeks ago

Should Shift + Option/Alt + arrow keys resize highlights by word boundaries? That would track with the behavior on macOS (Option + arrow keys selects by word) and would be pretty convenient. I don't think it's too confusing that it "conflicts" with the text/image/ink shortcuts.

Yes, because highlight resizing is conceptually similar to selection changes, it should support everything that a regular selection supports. In the future, this keyboard shortcut will also be supported in the PDF view.