From 46e1d5daa4eb53e68e05ae10de512ca38f9e7c6c Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 8 Jul 2018 10:55:56 +0200 Subject: [PATCH] Simplify resetting of the `SecondaryToolbar` Scroll/Spread mode buttons, and add a missing comment in `PDFCursorTools` The names 'resetscrollmode'/'resetspreadmode' were probably *not* great choices, given that the only thing being reset are toolbar buttons and not the actual Scroll/Spread modes. Furthermore, there's really no need for two separate events here. The patch also adds a comment that ought to have been included in PR 9040, to prevent future refactoring/removing of what may appear to be an unnecessary `Promise.resolve` call. --- web/pdf_cursor_tools.js | 2 ++ web/secondary_toolbar.js | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/web/pdf_cursor_tools.js b/web/pdf_cursor_tools.js index 8d230a598..252963944 100644 --- a/web/pdf_cursor_tools.js +++ b/web/pdf_cursor_tools.js @@ -47,6 +47,8 @@ class PDFCursorTools { this._addEventListeners(); + // Defer the initial `switchTool` call, to give other viewer components + // time to initialize *and* register 'cursortoolchanged' event listeners. Promise.resolve().then(() => { this.switchTool(cursorToolOnLoad); }); diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js index ac46becde..2b59a93d0 100644 --- a/web/secondary_toolbar.js +++ b/web/secondary_toolbar.js @@ -142,8 +142,7 @@ class SecondaryToolbar { this._updateUIState(); // Reset the Scroll/Spread buttons too, since they're document specific. - this.eventBus.dispatch('resetscrollmode', { source: this, }); - this.eventBus.dispatch('resetspreadmode', { source: this, }); + this.eventBus.dispatch('secondarytoolbarreset', { source: this, }); } _updateUIState() { @@ -212,7 +211,7 @@ class SecondaryToolbar { } this.eventBus.on('scrollmodechanged', scrollModeChanged); - this.eventBus.on('resetscrollmode', (evt) => { + this.eventBus.on('secondarytoolbarreset', (evt) => { if (evt.source === this) { scrollModeChanged({ mode: ScrollMode.VERTICAL, }); } @@ -239,7 +238,7 @@ class SecondaryToolbar { } this.eventBus.on('spreadmodechanged', spreadModeChanged); - this.eventBus.on('resetspreadmode', (evt) => { + this.eventBus.on('secondarytoolbarreset', (evt) => { if (evt.source === this) { spreadModeChanged({ mode: SpreadMode.NONE, }); }