From 2594f0c7380332a4e12ceee6a4c32b86c838904f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 9 Sep 2020 13:26:38 +0200 Subject: [PATCH] Revert "[PDFSidebarResizer] Refactor the clamping in `_updateWidth`" This reverts commit 9e4552d7920fcba13b3d74e1e180fc49180f4436 for causing the sidebar to become too narrow when the entire viewer is resized. **Steps to reproduce:** 1. Load the viewer. 2. Open the sidebar. 3. Resize the sidebar, making it wider. 4. Resize the entire viewer, i.e. the browser window, making it *narrower* than 400 pixels. **Expected result:** The sidebar width is clamped at 200 pixels. **Actual result:** The sidebar becomes too narrow. The cause of this bug is, in hindsight, quite obvious since the `clamp` helper function implicitly assumes that the `min`/`max` arguments are correctly sorted. At viewer widths *below* 400 pixels, that assumption is broken which explains the bug. --- web/pdf_sidebar_resizer.js | 20 +++++++++++--------- web/ui_utils.js | 1 - 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/web/pdf_sidebar_resizer.js b/web/pdf_sidebar_resizer.js index 668c59dcb..b3f5b178c 100644 --- a/web/pdf_sidebar_resizer.js +++ b/web/pdf_sidebar_resizer.js @@ -13,7 +13,7 @@ * limitations under the License. */ -import { clamp, NullL10n } from "./ui_utils.js"; +import { NullL10n } from "./ui_utils.js"; const SIDEBAR_WIDTH_VAR = "--sidebar-width"; const SIDEBAR_MIN_WIDTH = 200; // pixels @@ -88,17 +88,19 @@ class PDFSidebarResizer { } // Prevent the sidebar from becoming too narrow, or from occupying more // than half of the available viewer width. - const newWidth = clamp( - width, - SIDEBAR_MIN_WIDTH, - Math.floor(this.outerContainerWidth / 2) - ); + const maxWidth = Math.floor(this.outerContainerWidth / 2); + if (width > maxWidth) { + width = maxWidth; + } + if (width < SIDEBAR_MIN_WIDTH) { + width = SIDEBAR_MIN_WIDTH; + } // Only update the UI when the sidebar width did in fact change. - if (newWidth === this._width) { + if (width === this._width) { return false; } - this._width = newWidth; - this.doc.style.setProperty(SIDEBAR_WIDTH_VAR, `${newWidth}px`); + this._width = width; + this.doc.style.setProperty(SIDEBAR_WIDTH_VAR, `${width}px`); return true; } diff --git a/web/ui_utils.js b/web/ui_utils.js index d08497165..4d007703d 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -1006,7 +1006,6 @@ export { SpreadMode, NullL10n, EventBus, - clamp, ProgressBar, getPDFFileNameFromURL, noContextMenuHandler,