From 7c81a8dd40796158edcc135c9ebe06a06e93f3b2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 31 Aug 2021 17:08:43 +0200 Subject: [PATCH 1/2] [api-minor] Change `{PDFPageView, PDFThumbnailView}.update` to take a parameter object The old `update`-signature started to annoy me back when I added optional content support to the viewer, since we're (often) forced to pass in a bunch of arguments that we don't care about whenever these methods are called. This is tagged `api-minor` since `PDFPageView` is being used in the `pageviewer` component example, and it's thus possible that these changes could affect some users; the next commit adds fallback handling for the old format. --- web/base_viewer.js | 14 ++++++++------ web/pdf_page_view.js | 7 +++---- web/pdf_thumbnail_view.js | 6 +++--- web/pdf_thumbnail_viewer.js | 5 +++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index ecd42f44c..59ab4d61d 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -405,9 +405,9 @@ class BaseViewer { const pageNumber = this._currentPageNumber; - for (let i = 0, ii = this._pages.length; i < ii; i++) { - const pageView = this._pages[i]; - pageView.update(pageView.scale, rotation); + const updateArgs = { rotation }; + for (const pageView of this._pages) { + pageView.update(updateArgs); } // Prevent errors in case the rotation changes *before* the scale has been // set to a non-default value. @@ -717,8 +717,9 @@ class BaseViewer { } this._doc.style.setProperty("--zoom-factor", newScale); - for (let i = 0, ii = this._pages.length; i < ii; i++) { - this._pages[i].update(newScale); + const updateArgs = { scale: newScale }; + for (const pageView of this._pages) { + pageView.update(updateArgs); } this._currentScale = newScale; @@ -1435,8 +1436,9 @@ class BaseViewer { } this._optionalContentConfigPromise = promise; + const updateArgs = { optionalContentConfigPromise: promise }; for (const pageView of this._pages) { - pageView.update(pageView.scale, pageView.rotation, promise); + pageView.update(updateArgs); } this.update(); diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 3cbe8e872..ac1dab94e 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -297,11 +297,10 @@ class PDFPageView { div.appendChild(this.loadingIconDiv); } - update(scale, rotation, optionalContentConfigPromise = null) { + update({ scale = 0, rotation = null, optionalContentConfigPromise = null }) { this.scale = scale || this.scale; - // The rotation may be zero. - if (typeof rotation !== "undefined") { - this.rotation = rotation; + if (typeof rotation === "number") { + this.rotation = rotation; // The rotation may be zero. } if (optionalContentConfigPromise instanceof Promise) { this._optionalContentConfigPromise = optionalContentConfigPromise; diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 3295daf6e..74fc2eaeb 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -195,9 +195,9 @@ class PDFThumbnailView { } } - update(rotation) { - if (typeof rotation !== "undefined") { - this.rotation = rotation; + update({ rotation = null }) { + if (typeof rotation === "number") { + this.rotation = rotation; // The rotation may be zero. } const totalRotation = (this.rotation + this.pdfPageRotate) % 360; this.viewport = this.viewport.clone({ diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 3514dc12a..36d2e6bd6 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -144,8 +144,9 @@ class PDFThumbnailViewer { } this._pagesRotation = rotation; - for (let i = 0, ii = this._thumbnails.length; i < ii; i++) { - this._thumbnails[i].update(rotation); + const updateArgs = { rotation }; + for (const thumbnail of this._thumbnails) { + thumbnail.update(updateArgs); } } From 846620438417c395f14f3cb60a02806c1b3bebc8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 31 Aug 2021 17:24:21 +0200 Subject: [PATCH 2/2] [GENERIC viewer] Add fallback logic for the old `PDFPageView.update` method signature This is done separately from the previous commit, to make it easier to revert these changes in the future. --- web/pdf_page_view.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index ac1dab94e..1ffb2dbe9 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -298,6 +298,23 @@ class PDFPageView { } update({ scale = 0, rotation = null, optionalContentConfigPromise = null }) { + if ( + typeof PDFJSDev !== "undefined" && + PDFJSDev.test("GENERIC") && + typeof arguments[0] !== "object" + ) { + console.error( + "PDFPageView.update called with separate parameters, please use an object instead." + ); + + this.update({ + scale: arguments[0], + rotation: arguments[1], + optionalContentConfigPromise: arguments[2], + }); + return; + } + this.scale = scale || this.scale; if (typeof rotation === "number") { this.rotation = rotation; // The rotation may be zero.