-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat(PdfReader): add OnPrintingAsync parameter #731
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 all commits
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -127,7 +127,7 @@ const addEventListener = (el, pdfViewer, eventBus, invoke, options) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.triggerPagesInit === true) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await invoke.invokeMethodAsync("pagesInit", numPages); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await invoke.invokeMethodAsync("PagesInit", numPages); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -139,6 +139,12 @@ const addEventListener = (el, pdfViewer, eventBus, invoke, options) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.triggerPagesLoaded === true) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await invoke.invokeMethodAsync("PagesLoaded", e.pagesCount); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const controls = el.querySelector(".bb-view-controls"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EventHandler.on(controls, "click", ".bb-view-print", e => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| printPdf(options.url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+144
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): The event handler uses In the |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await invoke.invokeMethodAsync("Printing"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+144
to
+147
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EventHandler.on(controls, "click", ".bb-view-print", e => { | |
| printPdf(options.url); | |
| await invoke.invokeMethodAsync("Printing"); | |
| }); | |
| if (options.triggerPrinting === true) { | |
| EventHandler.on(controls, "click", ".bb-view-print", async e => { | |
| printPdf(options.url); | |
| await invoke.invokeMethodAsync("Printing"); | |
| }); | |
| } |
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.
suggestion: Null handling for .bb-view-controls should be added before wiring event listeners.
If .bb-view-controls is missing, el.querySelector(".bb-view-controls") returns null, and passing that into EventHandler.on will likely throw. Please guard this with a null check (e.g. if (controls) { EventHandler.on(...); }) so layouts without controls don’t cause runtime errors.
| if (options.triggerPagesLoaded === true) { | |
| await invoke.invokeMethodAsync("PagesLoaded", e.pagesCount); | |
| } | |
| const controls = el.querySelector(".bb-view-controls"); | |
| EventHandler.on(controls, "click", ".bb-view-print", e => { | |
| printPdf(options.url); | |
| await invoke.invokeMethodAsync("Printing"); | |
| }); | |
| }) | |
| if (options.triggerPagesLoaded === true) { | |
| await invoke.invokeMethodAsync("PagesLoaded", e.pagesCount); | |
| } | |
| const controls = el.querySelector(".bb-view-controls"); | |
| if (controls) { | |
| EventHandler.on(controls, "click", ".bb-view-print", async e => { | |
| printPdf(options.url); | |
| await invoke.invokeMethodAsync("Printing"); | |
| }); | |
| } | |
| }) |
Copilot
AI
Nov 27, 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 print event handler is registered inside the 'pagesloaded' event callback. This means the handler will be registered multiple times if the PDF is reloaded, causing the print function and callback to be invoked multiple times on each print click. Consider registering this handler once in the main addEventListener function body, similar to how the minus/plus button handlers are registered on lines 197-198.
| const controls = el.querySelector(".bb-view-controls"); | |
| EventHandler.on(controls, "click", ".bb-view-print", e => { | |
| printPdf(options.url); | |
| await invoke.invokeMethodAsync("Printing"); | |
| }); | |
| }) | |
| }); | |
| // Register print button click handler once | |
| const controls = el.querySelector(".bb-view-controls"); | |
| if (controls) { | |
| EventHandler.on(controls, "click", ".bb-view-print", async e => { | |
| printPdf(options.url); | |
| await invoke.invokeMethodAsync("Printing"); | |
| }); | |
| } |
Copilot
AI
Nov 27, 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.
Setting classList directly to a string is incorrect. The classList property is a read-only DOMTokenList. Use iframe.classList.add("bb-view-print-iframe") instead, or use iframe.className = "bb-view-print-iframe" to set the class attribute.
| iframe.classList = "bb-view-print-iframe"; | |
| iframe.className = "bb-view-print-iframe"; |
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 event handler callback function is missing the
asynckeyword but usesawaiton line 146. This will cause a syntax error. The arrow function on line 144 should be declared asasync (e) => {instead ofe => {.