8000 Define "snapshot" the same way everywhere by AbeJellinek · Pull Request #5086 · zotero/zotero · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Define "snapshot" the same way everywhere #5086

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.

Sign up for GitHub

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? 8000 Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Define "snapshot" the same way everywhere #5086

wants to merge 2 commits into from

Conversation

AbeJellinek
Copy link
Member

Before this PR:

  • All HTML attachments, whether imported via the Connector or from disk, could be opened as snapshots in the reader
  • All HTML attachments' attachmentReaderType was 'snapshot'

But:

  • HTML attachments imported from disk wouldn't satisfy isSnapshotAttachment(), so some snapshot functionality (like Add Note from Annotations) wouldn't work
  • HTML attachments imported from disk had a generic document icon, not a snapshot icon

This PR makes isSnapshotAttachment() use the same conditions as attachmentReaderType and rewrites attachmentReaderType to use the is*Attachment() methods instead of duplicating their logic.

Caveat: We don't currently have an icon for a "linked snapshot," so this uses the web link icon.

@AbeJellinek AbeJellinek requested a review from dstillman March 10, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dstillman dstillman Awaiting requested review from dstillman

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@AbeJellinek
0