From d9ce17642f3647ec23598f32138c8298b9551be1 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 1 Jul 2022 10:14:57 +0200 Subject: [PATCH] [api-minor] Further modernize the `ProgressBar` class (PR 14918 follow-up) - Simplify how we look-up the DOM-element, which should also be a tiny bit more efficent. - Use private class-fields, rather than property-names prefixed with underscores. - Inline the `#updateBar` helper-method directly in the `percent`-setter, since having a separate method doesn't seem necessary in this case. - Set the `indeterminate`-class on the ProgressBar DOM-element, to simplify the code. Finally, also (slightly) re-factors the `PDFViewerApplication.progress`-method to make it a bit smaller. --- examples/mobile-viewer/viewer.css | 4 +-- examples/mobile-viewer/viewer.js | 2 +- web/app.js | 52 ++++++++++++++++--------------- web/ui_utils.js | 52 ++++++++++++++----------------- web/viewer.css | 4 +-- 5 files changed, 56 insertions(+), 58 deletions(-) diff --git a/examples/mobile-viewer/viewer.css b/examples/mobile-viewer/viewer.css index 1e21c642a..0c86e18d5 100644 --- a/examples/mobile-viewer/viewer.css +++ b/examples/mobile-viewer/viewer.css @@ -221,13 +221,13 @@ canvas { } } -#loadingBar .progress.indeterminate { +#loadingBar.indeterminate .progress { transform: none; background-color: rgba(153, 153, 153, 1); transition: none; } -#loadingBar .indeterminate .glimmer { +#loadingBar.indeterminate .progress .glimmer { position: absolute; top: 0; left: 0; diff --git a/examples/mobile-viewer/viewer.js b/examples/mobile-viewer/viewer.js index 7ee86fe0e..4eb31e685 100644 --- a/examples/mobile-viewer/viewer.js +++ b/examples/mobile-viewer/viewer.js @@ -161,7 +161,7 @@ const PDFViewerApplication = { }, get loadingBar() { - const bar = new pdfjsViewer.ProgressBar("#loadingBar"); + const bar = new pdfjsViewer.ProgressBar("loadingBar"); return pdfjsLib.shadow(this, "loadingBar", bar); }, diff --git a/web/app.js b/web/app.js index 5482ed7d6..c60405522 100644 --- a/web/app.js +++ b/web/app.js @@ -718,7 +718,7 @@ const PDFViewerApplication = { }, get loadingBar() { - const bar = new ProgressBar("#loadingBar"); + const bar = new ProgressBar("loadingBar"); return shadow(this, "loadingBar", bar); }, @@ -1160,31 +1160,33 @@ const PDFViewerApplication = { // that we discard some of the loaded data. This can cause the loading // bar to move backwards. So prevent this by only updating the bar if it // increases. - if (percent > this.loadingBar.percent || isNaN(percent)) { - this.loadingBar.percent = percent; - - // When disableAutoFetch is enabled, it's not uncommon for the entire file - // to never be fetched (depends on e.g. the file structure). In this case - // the loading bar will not be completely filled, nor will it be hidden. - // To prevent displaying a partially filled loading bar permanently, we - // hide it when no data has been loaded during a certain amount of time. - const disableAutoFetch = this.pdfDocument - ? this.pdfDocument.loadingParams.disableAutoFetch - : AppOptions.get("disableAutoFetch"); - - if (disableAutoFetch && percent) { - if (this.disableAutoFetchLoadingBarTimeout) { - clearTimeout(this.disableAutoFetchLoadingBarTimeout); - this.disableAutoFetchLoadingBarTimeout = null; - } - this.loadingBar.show(); - - this.disableAutoFetchLoadingBarTimeout = setTimeout(() => { - this.loadingBar.hide(); - this.disableAutoFetchLoadingBarTimeout = null; - }, DISABLE_AUTO_FETCH_LOADING_BAR_TIMEOUT); - } + if (percent <= this.loadingBar.percent) { + return; } + this.loadingBar.percent = percent; + + // When disableAutoFetch is enabled, it's not uncommon for the entire file + // to never be fetched (depends on e.g. the file structure). In this case + // the loading bar will not be completely filled, nor will it be hidden. + // To prevent displaying a partially filled loading bar permanently, we + // hide it when no data has been loaded during a certain amount of time. + const disableAutoFetch = + this.pdfDocument?.loadingParams.disableAutoFetch ?? + AppOptions.get("disableAutoFetch"); + + if (!disableAutoFetch || isNaN(percent)) { + return; + } + if (this.disableAutoFetchLoadingBarTimeout) { + clearTimeout(this.disableAutoFetchLoadingBarTimeout); + this.disableAutoFetchLoadingBarTimeout = null; + } + this.loadingBar.show(); + + this.disableAutoFetchLoadingBarTimeout = setTimeout(() => { + this.loadingBar.hide(); + this.disableAutoFetchLoadingBarTimeout = null; + }, DISABLE_AUTO_FETCH_LOADING_BAR_TIMEOUT); }, load(pdfDocument) { diff --git a/web/ui_utils.js b/web/ui_utils.js index e31c4e617..cb5aff78d 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -689,6 +689,12 @@ function clamp(v, min, max) { } class ProgressBar { + #classList = null; + + #percent = 0; + + #visible = true; + constructor(id) { if ( (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) && @@ -699,34 +705,24 @@ class ProgressBar { "please use CSS rules to modify its appearance instead." ); } - this.visible = true; - - // Fetch the sub-elements for later. - this.div = document.querySelector(id + " .progress"); - // Get the loading bar element, so it can be resized to fit the viewer. - this.bar = this.div.parentNode; - - this.percent = 0; - } - - #updateBar() { - if (this._indeterminate) { - this.div.classList.add("indeterminate"); - return; - } - this.div.classList.remove("indeterminate"); - - docStyle.setProperty("--progressBar-percent", `${this._percent}%`); + const bar = document.getElementById(id); + this.#classList = bar.classList; } get percent() { - return this._percent; + return this.#percent; } set percent(val) { - this._indeterminate = isNaN(val); - this._percent = clamp(val, 0, 100); - this.#updateBar(); + this.#percent = clamp(val, 0, 100); + + if (isNaN(val)) { + this.#classList.add("indeterminate"); + return; + } + this.#classList.remove("indeterminate"); + + docStyle.setProperty("--progressBar-percent", `${this.#percent}%`); } setWidth(viewer) { @@ -741,19 +737,19 @@ class ProgressBar { } hide() { - if (!this.visible) { + if (!this.#visible) { return; } - this.visible = false; - this.bar.classList.add("hidden"); + this.#visible = false; + this.#classList.add("hidden"); } show() { - if (this.visible) { + if (this.#visible) { return; } - this.visible = true; - this.bar.classList.remove("hidden"); + this.#visible = true; + this.#classList.remove("hidden"); } } diff --git a/web/viewer.css b/web/viewer.css index 865c25541..acc4b76de 100644 --- a/web/viewer.css +++ b/web/viewer.css @@ -384,13 +384,13 @@ select { } } -#loadingBar .progress.indeterminate { +#loadingBar.indeterminate .progress { transform: none; background-color: var(--progressBar-indeterminate-bg-color); transition: none; } -#loadingBar .progress.indeterminate .glimmer { +#loadingBar.indeterminate .progress .glimmer { position: absolute; top: 0; left: 0;