-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat(PdfReader): add EnableThumbnails parameter #739
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -79,6 +79,13 @@ export function scale(id, scale) { | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| export function resetThumbnails(id) { | ||||||||||
| const { el, pdfViewer } = Data.get(id); | ||||||||||
| if (pdfViewer) { | ||||||||||
|
||||||||||
| if (pdfViewer) { | |
| if (el && pdfViewer) { |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression options.enableThumbnails || false is redundant. Since options.enableThumbnails is already a boolean, the || false doesn't change the behavior. If enableThumbnails is true, it returns true; if it's false, the expression evaluates to false. This should be simplified to just options.enableThumbnails.
| if (options.enableThumbnails || false) { | |
| if (options.enableThumbnails) { |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check for thumbnailsEl. The code queries for .bb-view-thumbnails but doesn't verify it exists before calling classList.toggle(). While the event delegation pattern ensures this handler only fires when .bb-view-bar is clicked, there's a timing issue: during dynamic state changes (e.g., when EnableThumbnails transitions from true to false), the .bb-view-bar element might still exist briefly while .bb-view-thumbnails is already removed, leading to a null reference error. Add a null check: if (thumbnailsEl) { thumbnailsEl.classList.toggle("show"); }.
| thumbnailsEl.classList.toggle("show"); | |
| if (thumbnailsEl) { | |
| thumbnailsEl.classList.toggle("show"); | |
| } |
Uh oh!
There was an error while loading. Please reload this page.