From b1ca27016217b641f3287c33b31292fb67a2dffc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 26 Dec 2023 10:56:48 +0100 Subject: [PATCH] Remove the internal "secondarytoolbarreset" event and slightly re-factor the code With modern JavaScript class features we can move the relevant event handling into private methods, and thus invoke it directly when resetting the toolbar UI-state. *Please note:* This patch slightly reduces the size of the `web/secondary_toolbar.js` file. --- web/secondary_toolbar.js | 156 +++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 82 deletions(-) diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js index f05ee3f08..d4c3c9541 100644 --- a/web/secondary_toolbar.js +++ b/web/secondary_toolbar.js @@ -52,14 +52,15 @@ import { PagesCountLimit } from "./pdf_viewer.js"; */ class SecondaryToolbar { + #opts; + /** * @param {SecondaryToolbarOptions} options * @param {EventBus} eventBus */ constructor(options, eventBus) { - this.toolbar = options.toolbar; - this.toggleButton = options.toggleButton; - this.buttons = [ + this.#opts = options; + const buttons = [ { element: options.presentationModeButton, eventName: "presentationmode", @@ -141,28 +142,19 @@ class SecondaryToolbar { }, ]; if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { - this.buttons.push({ + buttons.push({ element: options.openFileButton, eventName: "openfile", close: true, }); } - this.items = { - firstPage: options.firstPageButton, - lastPage: options.lastPageButton, - pageRotateCw: options.pageRotateCwButton, - pageRotateCcw: options.pageRotateCcwButton, - }; this.eventBus = eventBus; this.opened = false; // Bind the event listeners for click, cursor tool, and scroll/spread mode // actions. - this.#bindClickListeners(); - this.#bindCursorToolsListener(options); - this.#bindScrollModeListener(options); - this.#bindSpreadModeListener(options); + this.#bindListeners(buttons); this.reset(); } @@ -190,30 +182,40 @@ class SecondaryToolbar { this.#updateUIState(); // Reset the Scroll/Spread buttons too, since they're document specific. - this.eventBus.dispatch("secondarytoolbarreset", { source: this }); + this.#scrollModeChanged({ mode: ScrollMode.VERTICAL }); + this.#spreadModeChanged({ mode: SpreadMode.NONE }); } #updateUIState() { - this.items.firstPage.disabled = this.pageNumber <= 1; - this.items.lastPage.disabled = this.pageNumber >= this.pagesCount; - this.items.pageRotateCw.disabled = this.pagesCount === 0; - this.items.pageRotateCcw.disabled = this.pagesCount === 0; + const { + firstPageButton, + lastPageButton, + pageRotateCwButton, + pageRotateCcwButton, + } = this.#opts; + + firstPageButton.disabled = this.pageNumber <= 1; + lastPageButton.disabled = this.pageNumber >= this.pagesCount; + pageRotateCwButton.disabled = this.pagesCount === 0; + pageRotateCcwButton.disabled = this.pagesCount === 0; } - #bindClickListeners() { + #bindListeners(buttons) { + const { eventBus } = this; + const { toggleButton } = this.#opts; // Button to toggle the visibility of the secondary toolbar. - this.toggleButton.addEventListener("click", this.toggle.bind(this)); + toggleButton.addEventListener("click", this.toggle.bind(this)); // All items within the secondary toolbar. - for (const { element, eventName, close, eventDetails } of this.buttons) { + for (const { element, eventName, close, eventDetails } of buttons) { element.addEventListener("click", evt => { if (eventName !== null) { - this.eventBus.dispatch(eventName, { source: this, ...eventDetails }); + eventBus.dispatch(eventName, { source: this, ...eventDetails }); } if (close) { this.close(); } - this.eventBus.dispatch("reporttelemetry", { + eventBus.dispatch("reporttelemetry", { source: this, details: { type: "buttons", @@ -222,72 +224,58 @@ class SecondaryToolbar { }); }); } + + eventBus._on("cursortoolchanged", this.#cursorToolChanged.bind(this)); + eventBus._on("scrollmodechanged", this.#scrollModeChanged.bind(this)); + eventBus._on("spreadmodechanged", this.#spreadModeChanged.bind(this)); } - #bindCursorToolsListener({ cursorSelectToolButton, cursorHandToolButton }) { - this.eventBus._on("cursortoolchanged", ({ tool }) => { - toggleCheckedBtn(cursorSelectToolButton, tool === CursorTool.SELECT); - toggleCheckedBtn(cursorHandToolButton, tool === CursorTool.HAND); - }); + #cursorToolChanged({ tool }) { + const { cursorSelectToolButton, cursorHandToolButton } = this.#opts; + + toggleCheckedBtn(cursorSelectToolButton, tool === CursorTool.SELECT); + toggleCheckedBtn(cursorHandToolButton, tool === CursorTool.HAND); } - #bindScrollModeListener({ - scrollPageButton, - scrollVerticalButton, - scrollHorizontalButton, - scrollWrappedButton, - spreadNoneButton, - spreadOddButton, - spreadEvenButton, - }) { - const scrollModeChanged = ({ mode }) => { - toggleCheckedBtn(scrollPageButton, mode === ScrollMode.PAGE); - toggleCheckedBtn(scrollVerticalButton, mode === ScrollMode.VERTICAL); - toggleCheckedBtn(scrollHorizontalButton, mode === ScrollMode.HORIZONTAL); - toggleCheckedBtn(scrollWrappedButton, mode === ScrollMode.WRAPPED); + #scrollModeChanged({ mode }) { + const { + scrollPageButton, + scrollVerticalButton, + scrollHorizontalButton, + scrollWrappedButton, + spreadNoneButton, + spreadOddButton, + spreadEvenButton, + } = this.#opts; - // Permanently *disable* the Scroll buttons when PAGE-scrolling is being - // enforced for *very* long/large documents; please see the `BaseViewer`. - const forceScrollModePage = - this.pagesCount > PagesCountLimit.FORCE_SCROLL_MODE_PAGE; - scrollPageButton.disabled = forceScrollModePage; - scrollVerticalButton.disabled = forceScrollModePage; - scrollHorizontalButton.disabled = forceScrollModePage; - scrollWrappedButton.disabled = forceScrollModePage; + toggleCheckedBtn(scrollPageButton, mode === ScrollMode.PAGE); + toggleCheckedBtn(scrollVerticalButton, mode === ScrollMode.VERTICAL); + toggleCheckedBtn(scrollHorizontalButton, mode === ScrollMode.HORIZONTAL); + toggleCheckedBtn(scrollWrappedButton, mode === ScrollMode.WRAPPED); - // Temporarily *disable* the Spread buttons when horizontal scrolling is - // enabled, since the non-default Spread modes doesn't affect the layout. - const isHorizontal = mode === ScrollMode.HORIZONTAL; - spreadNoneButton.disabled = isHorizontal; - spreadOddButton.disabled = isHorizontal; - spreadEvenButton.disabled = isHorizontal; - }; - this.eventBus._on("scrollmodechanged", scrollModeChanged); + // Permanently *disable* the Scroll buttons when PAGE-scrolling is being + // enforced for *very* long/large documents; please see the `BaseViewer`. + const forceScrollModePage = + this.pagesCount > PagesCountLimit.FORCE_SCROLL_MODE_PAGE; + scrollPageButton.disabled = forceScrollModePage; + scrollVerticalButton.disabled = forceScrollModePage; + scrollHorizontalButton.disabled = forceScrollModePage; + scrollWrappedButton.disabled = forceScrollModePage; - this.eventBus._on("secondarytoolbarreset", evt => { - if (evt.source === this) { - scrollModeChanged({ mode: ScrollMode.VERTICAL }); - } - }); + // Temporarily *disable* the Spread buttons when horizontal scrolling is + // enabled, since the non-default Spread modes doesn't affect the layout. + const isHorizontal = mode === ScrollMode.HORIZONTAL; + spreadNoneButton.disabled = isHorizontal; + spreadOddButton.disabled = isHorizontal; + spreadEvenButton.disabled = isHorizontal; } - #bindSpreadModeListener({ - spreadNoneButton, - spreadOddButton, - spreadEvenButton, - }) { - const spreadModeChanged = ({ mode }) => { - toggleCheckedBtn(spreadNoneButton, mode === SpreadMode.NONE); - toggleCheckedBtn(spreadOddButton, mode === SpreadMode.ODD); - toggleCheckedBtn(spreadEvenButton, mode === SpreadMode.EVEN); - }; - this.eventBus._on("spreadmodechanged", spreadModeChanged); + #spreadModeChanged({ mode }) { + const { spreadNoneButton, spreadOddButton, spreadEvenButton } = this.#opts; - this.eventBus._on("secondarytoolbarreset", evt => { - if (evt.source === this) { - spreadModeChanged({ mode: SpreadMode.NONE }); - } - }); + toggleCheckedBtn(spreadNoneButton, mode === SpreadMode.NONE); + toggleCheckedBtn(spreadOddButton, mode === SpreadMode.ODD); + toggleCheckedBtn(spreadEvenButton, mode === SpreadMode.EVEN); } open() { @@ -295,7 +283,9 @@ class SecondaryToolbar { return; } this.opened = true; - toggleExpandedBtn(this.toggleButton, true, this.toolbar); + + const { toggleButton, toolbar } = this.#opts; + toggleExpandedBtn(toggleButton, true, toolbar); } close() { @@ -303,7 +293,9 @@ class SecondaryToolbar { return; } this.opened = false; - toggleExpandedBtn(this.toggleButton, false, this.toolbar); + + const { toggleButton, toolbar } = this.#opts; + toggleExpandedBtn(toggleButton, false, toolbar); } toggle() {