search

zotero / reader

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

Add thumbnails header with page rotation options for web target #110

Closed tnajdek closed 1 year ago

tnajdek commented 1 year ago

This PR enables three-dot "more" menu that appears on thumbnails on hover. This works for mouse users, but touch and keyboard will need additional solution(s) but I'm yet to come up with something sensible. Once I figure it out, I'll send another PR.

New implementation described below: https://github.com/zotero/reader/pull/110#issuecomment-1703423093

tnajdek commented 1 year ago

On a second though I might need to come up with a different UI for this, let's put this on hold for the time being.

tnajdek commented 1 year ago

Different approach: I've introduced a header which says n pages selected and contains a button to access to rotate selected pages. This solution is keyboard-accessible and mostly touch-compatible (though there is room for improvement here). There should be no visible change for non-web targets.

Please let me know if this works or if you have any suggestions.

https://github.com/zotero/reader/assets/214628/0d9946de-25ab-47bc-aae7-921dee5030de

mrtcode commented 1 year ago

I think the header would be great, but it's not ideal that "1 page selected" is always visible, because rotation and deletion are relatively rare operations.

Also, I guess, multiple thumbnails can't be selected on touch devices, right?

Maybe each thumbnail should have a partially transparent selection button like, for example, in Google Photos web app. Although this would be complicated to implement because at least one thumbnail is always selected.

Anyway, I think for now you can merge this, and we'll improve it in future.

tnajdek commented 1 year ago

Similar cases are handled by a dedicated "select" mode on touch devices in web library, i.e. you need to enable "select" mode, at which point each "tap" is adding/removing from selection, instead of navigating to that page - maybe we could do that? And on desktop, leave it as it is in this PR, but maybe remove "1 page selected" unless it's at least 2 pages?

Anyway, I think for now you can merge this, and we'll improve it in future.

I don't have write access to this repo 😅

dstillman commented 1 year ago

I don't have write access to this repo

Fixed