From 36111a1f40ae800556ab4196f4e7f299939d9e22 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 21 Jun 2018 17:55:59 +0200 Subject: [PATCH] [Regression] Remove instances of `Element.classList.toggle()` with *two* parameters, since browser support is limited The Secondary Toolbar buttons for, not to mention the actual toggling of, Scroll/Spread modes are currently completely broken in older browsers (such as IE11). As a follow-up, it'd probably be a good idea to try and find a *feature complete* `classList` polyfill that could be used instead, but this patch at least addresses the immediate regression. Please refer to the compatibility information in https://developer.mozilla.org/en-US/docs/Web/API/Element/classList#Browser_compatibility --- web/base_viewer.js | 15 +++++++++++--- web/secondary_toolbar.js | 42 ++++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index e084512b0..2eee52ac9 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1035,9 +1035,18 @@ class BaseViewer { } _updateScrollModeClasses() { - const mode = this.scrollMode, { classList, } = this.viewer; - classList.toggle('scrollHorizontal', mode === ScrollMode.HORIZONTAL); - classList.toggle('scrollWrapped', mode === ScrollMode.WRAPPED); + const { scrollMode, viewer, } = this; + + if (scrollMode === ScrollMode.HORIZONTAL) { + viewer.classList.add('scrollHorizontal'); + } else { + viewer.classList.remove('scrollHorizontal'); + } + if (scrollMode === ScrollMode.WRAPPED) { + viewer.classList.add('scrollWrapped'); + } else { + viewer.classList.remove('scrollWrapped'); + } } setSpreadMode(mode) { diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js index a44be0642..233f5f6fa 100644 --- a/web/secondary_toolbar.js +++ b/web/secondary_toolbar.js @@ -190,23 +190,41 @@ class SecondaryToolbar { _bindScrollModeListener(buttons) { this.eventBus.on('scrollmodechanged', function(evt) { - buttons.scrollVerticalButton.classList.toggle('toggled', - evt.mode === ScrollMode.VERTICAL); - buttons.scrollHorizontalButton.classList.toggle('toggled', - evt.mode === ScrollMode.HORIZONTAL); - buttons.scrollWrappedButton.classList.toggle('toggled', - evt.mode === ScrollMode.WRAPPED); + buttons.scrollVerticalButton.classList.remove('toggled'); + buttons.scrollHorizontalButton.classList.remove('toggled'); + buttons.scrollWrappedButton.classList.remove('toggled'); + + switch (evt.mode) { + case ScrollMode.VERTICAL: + buttons.scrollVerticalButton.classList.add('toggled'); + break; + case ScrollMode.HORIZONTAL: + buttons.scrollHorizontalButton.classList.add('toggled'); + break; + case ScrollMode.WRAPPED: + buttons.scrollWrappedButton.classList.add('toggled'); + break; + } }); } _bindSpreadModeListener(buttons) { this.eventBus.on('spreadmodechanged', function(evt) { - buttons.spreadNoneButton.classList.toggle('toggled', - evt.mode === SpreadMode.NONE); - buttons.spreadOddButton.classList.toggle('toggled', - evt.mode === SpreadMode.ODD); - buttons.spreadEvenButton.classList.toggle('toggled', - evt.mode === SpreadMode.EVEN); + buttons.spreadNoneButton.classList.remove('toggled'); + buttons.spreadOddButton.classList.remove('toggled'); + buttons.spreadEvenButton.classList.remove('toggled'); + + switch (evt.mode) { + case SpreadMode.NONE: + buttons.spreadNoneButton.classList.add('toggled'); + break; + case SpreadMode.ODD: + buttons.spreadOddButton.classList.add('toggled'); + break; + case SpreadMode.EVEN: + buttons.spreadEvenButton.classList.add('toggled'); + break; + } }); }