Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPUB/snapshot image annotations #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

AbeJellinek
Copy link
Member

@AbeJellinek AbeJellinek commented Sep 19, 2023

This isn't really the same tool as in the PDF view - it only allows you to select images, and only entire images. It acts a lot more like the EPUB/snapshot note tool than the PDF area tool. Pretty much the only difference is that the image tool can only target images and includes the targeted image in the annotations it creates. So should we just combine the two? We could make the note tool produce image annotations instead of note annotations when an image is targeted, and then we wouldn't need a separate image annotation tool with confusingly inconsistent behavior.

(I looked into supporting all DOM elements/multiple DOM elements instead of only single images, but that would add a huge amount of complexity for marginal gain. People are usually just going to want to pull out a single image at a time. Annotating text is already covered by other tools, and unlike in PDFs, unselectable text is rare - when it occurs, it's usually short segments like equations.)

Fixes zotero/zotero#4032

@AbeJellinek
Copy link
Member Author

Rebased

@dstillman
Copy link
Member

I'm not able to build this with Node 21:

Error [ERR_REQUIRE_ESM]: require() of ES Module […]/Client/reader/epubjs/epub.js/node_modules/pkg-dir/index.js from […]/Client/reader/epubjs/epub.js/node_modules/import-local/index.js not supported.

As for the original question, I get what you're saying about it working like the note annotation tool, but I think that's just because the element selector is an appropriate interaction model for this mode. The intention is still different: creating a note annotation vs. an image annotation. We don't handle those annotation types the same way — in the content view, sidebar, or notes — so I think we want to keep them distinct. (I also think most of the time people are just selecting single images from PDFs, so image annotations will generally look the same in all three modes.)

@AbeJellinek
Copy link
Member Author

I'm not able to build this with Node 21

I just undid a config change that made it in by mistake. Otherwise there's nothing in here that should affect the build, or EPUB.js at all. Try npm ci before running the build?

@AbeJellinek AbeJellinek changed the title WIP: EPUB/snapshot image annotations EPUB/snapshot image annotations May 7, 2024
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.

Support image extraction for image annotations for EPUBs/snapshots
2 participants