zotero / reader

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

EPUB/Snapshot: Keyboard accessibility improvements for annotations #150

Open AbeJellinek opened 1 week ago

AbeJellinek commented 1 week ago

Implements relevant #138 for EPUB/snapshot. Supports creating/resizing highlight/underline/note annotations.

mrtcode commented 1 week ago

Looks pretty good, but there are a few things to think about:

  1. onSetFindPopup doesn't seem to receive result.annotation, so creating annotations from the find input doesn't work.
  2. Note annotations can only be created with keyboard when text is selected, and there is no way to move them afterward, right?
  3. Also, navigating visible annotations and links with Tab and arrow keys seems to be necessary thing for accessibility, isn't it?
AbeJellinek commented 1 week ago

onSetFindPopup doesn't seem to receive result.annotation, so creating annotations from the find input doesn't work.

It was working in snapshots already; fixed for EPUBs.

Note annotations can only be created with keyboard when text is selected, and there is no way to move them afterward, right?

Currently, yes. We could support shortcuts that move them between block elements. I'm a little skeptical about how well it'll work, but at least it'll be easier than deleting and recreating.

Also, navigating visible annotations and links with Tab and arrow keys seems to be necessary thing for accessibility, isn't it?

Yes, definitely. I forgot that those changes were part of #138; I'll get on porting those over to EPUB/snapshot.

AbeJellinek commented 1 week ago

Do we want 2D navigation in EPUB/snapshot? I'm sort of inclined toward no, since it feels a little weird for textual web content (which has a linear order built in, unlike some PDFs), but no strong opinion.

mrtcode commented 1 week ago

Yeah, I think 1D navigation is sufficient to start.

AbeJellinek commented 1 week ago

OK, could you give it another try? Arrow key navigation and note annotation moving should work now.

mrtcode commented 1 week ago

Sure. Looks great, but there are still a few things:

Shift-Option-ArrowLeft results in this issue for me:

https://github.com/user-attachments/assets/33aee452-11db-46ee-9f5b-87e8c7e02a98

Tab -> Enter -> Escape scrolls the view to the top instead of defocusing the currently focused annotation:

https://github.com/user-attachments/assets/d352df0a-280c-48c9-97d7-b0364da6421d

I'm not sure if this is an issue, but when Caps Lock is on, I see that blue icon whenever I click on text:

https://github.com/user-attachments/assets/bb093aa1-cd99-44f5-b940-beba36d4ac28

So is there a way to create a note annotation with just keyboard?

AbeJellinek commented 1 week ago

OK, those issues should be fixed.

So is there a way to create a note annotation with just keyboard?

Ctrl-Alt-3 will now create a note annotation in the middle of the screen if there isn't any text selected.

I'm not sure if this is an issue, but when Caps Lock is on, I see that blue icon whenever I click on text

macOS bug. I don't know why it's showing in this particular iframe, since nothing there should be sent to contenteditable. I just fully disabled it on my non-work machine: https://gist.github.com/stephancasas/236f543b0f9f6509f5fe5878de01e38a

AbeJellinek commented 3 days ago

@mrtcode: Could you give this another try when you have a chance and let me know if anything seems to be missing?

mrtcode commented 3 days ago

Sorry for the delay.

A few little observations:

Should the annotation popup hide when resizing a highlight?

An extra tab is needed to focus on the annotation popup:

https://github.com/user-attachments/assets/c346c76b-3652-439f-b845-783b396bd9f7

Annotation movement with Shift-ArrowKeys is intentionally different from PDF view to prevent accidental movement, right?

After adding a note annotation with Ctrl-Option-3, the annotation is focused, but the popup doesn't appear.

AbeJellinek commented 3 days ago

Annotation movement with Shift-ArrowKeys is intentionally different from PDF view to prevent accidental movement, right?

How's that?

I'll fix the popup bugs. Thanks - don't test with the sidebar closed enough.

mrtcode commented 3 days ago

Annotation movement with Shift-ArrowKeys is intentionally different from PDF view to prevent accidental movement, right?

How's that?

Sorry. I mean Note annotation movement to another paragraph.

AbeJellinek commented 3 days ago

Oh, that wasn't exactly intentional (although maybe I'm just forgetting my reasoning), but I think it does make sense. EPUB/snapshot note annotations are annotations on text, not freeform annotations on the page, so I think it makes sense for them to move like highlights.

AbeJellinek commented 2 days ago

Should all be fixed.

mrtcode commented 1 day ago

Almost perfect. Just one more thing.

After moving (with Shift-ArrowKeys) the note annotation across paragraphs, it disappeared, and I now get this error:

Unable to get range for CFI epubcfi(/6/14!/,/1:1,/5:0) TypeError: step is undefined
    stepsToXpath resource://zotero/reader/reader.js:28614
    stepsToXpath resource://zotero/reader/reader.js:28613
    findNode resource://zotero/reader/reader.js:28713
    toRange resource://zotero/reader/reader.js:28782
    getRange resource://zotero/reader/reader.js:39162
    navigate resource://zotero/reader/reader.js:39845
    _navigateToSelector resource://zotero/reader/reader.js:39239
    navigate resource://zotero/reader/reader.js:37507
    navigate resource://zotero/reader/reader.js:39888
    setSelectedAnnotations resource://zotero/reader/reader.js:51681
    handleSidebarAnnotationSectionClick resource://zotero/reader/reader.js:24081
    handleSectionClick resource://zotero/reader/reader.js:23521
    onClick resource://zotero/reader/reader.js:23671
    React 15
AbeJellinek commented 1 day ago

Uh oh. Which EPUB/snapshot? And where was the note? I'll see if I can reproduce.

mrtcode commented 1 day ago

This EPUB file. Just add a note annotation and hold Shift-ArrowDown until it crashes at some paragraph.