Re-factor updating of Scroll/Spread modes, and place all the code in …

…`BaseViewer` with overrides (as necessary) in the extending classes This structure probably makes somewhat more sense, given that `PDFSinglePageViewer` is somewhat of a special case.
Snuffleupagus · Jun 29, 2018 · 4c09213 · 4c09213
1 parent bdca269
commit 4c09213
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 62 deletions.
diff --git a/web/base_viewer.js b/web/base_viewer.js
@@ -182,7 +182,7 @@ class BaseViewer {
       this.viewer.classList.add('removePageBorders');
     }
     if (this._scrollMode !== ScrollMode.VERTICAL) {
-      this._updateScrollModeClasses();
+      this._updateScrollMode();
     }
   }
 
@@ -442,7 +442,7 @@ class BaseViewer {
         this._pages.push(pageView);
       }
       if (this._spreadMode !== SpreadMode.NONE) {
-        this._regroupSpreads();
+        this._updateSpreadMode();
       }
 
       // Fetch all the pages since the viewport is needed before printing
@@ -1045,16 +1045,30 @@ class BaseViewer {
     this._scrollMode = mode;
     this.eventBus.dispatch('scrollmodechanged', { source: this, mode, });
 
-    this._updateScrollModeClasses();
+    this._updateScrollMode(/* pageNumber = */ this._currentPageNumber);
+  }
 
-    if (!this.pdfDocument) {
+  _updateScrollMode(pageNumber = null) {
+    const scrollMode = this._scrollMode, viewer = this.viewer;
+
+    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');
+    }
+
+    if (!this.pdfDocument || !pageNumber) {
       return;
     }
-    const pageNumber = this._currentPageNumber;
     // Non-numeric scale modes can be sensitive to the scroll orientation.
     // Call this before re-scrolling to the current page, to ensure that any
     // changes in scale don't move the current page.
-    if (isNaN(this._currentScaleValue)) {
+    if (this._currentScaleValue && isNaN(this._currentScaleValue)) {
       this._setScale(this._currentScaleValue, true);
     }
     this.scrollPageIntoView({ pageNumber, });
@@ -1065,10 +1079,6 @@ class BaseViewer {
     this.scrollMode = mode;
   }
 
-  _updateScrollModeClasses() {
-    // No-op in the base class.
-  }
-
   /**
    * @return {number} One of the values in {SpreadMode}.
    */
@@ -1091,15 +1101,46 @@ class BaseViewer {
     this._spreadMode = mode;
     this.eventBus.dispatch('spreadmodechanged', { source: this, mode, });
 
-    this._regroupSpreads();
+    this._updateSpreadMode(/* pageNumber = */ this._currentPageNumber);
   }
 
-  setSpreadMode(mode) {
-    this.spreadMode = mode;
+  _updateSpreadMode(pageNumber = null) {
+    if (!this.pdfDocument) {
+      return;
+    }
+    const viewer = this.viewer, pages = this._pages;
+    // Temporarily remove all the pages from the DOM.
+    viewer.textContent = '';
+
+    if (this._spreadMode === SpreadMode.NONE) {
+      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
+        viewer.appendChild(pages[i].div);
+      }
+    } else {
+      const parity = this._spreadMode - 1;
+      let spread = null;
+      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
+        if (spread === null) {
+          spread = document.createElement('div');
+          spread.className = 'spread';
+          viewer.appendChild(spread);
+        } else if (i % 2 === parity) {
+          spread = spread.cloneNode(false);
+          viewer.appendChild(spread);
+        }
+        spread.appendChild(pages[i].div);
+      }
+    }
+
+    if (!pageNumber) {
+      return;
+    }
+    this.scrollPageIntoView({ pageNumber, });
+    this.update();
   }
 
-  _regroupSpreads() {
-    // No-op in the base class.
+  setSpreadMode(mode) {
+    this.spreadMode = mode;
   }
 }
 

diff --git a/web/pdf_single_page_viewer.js b/web/pdf_single_page_viewer.js
@@ -147,6 +147,10 @@ class PDFSinglePageViewer extends BaseViewer {
     // The Scroll/Spread modes are never used in `PDFSinglePageViewer`.
     return shadow(this, '_isScrollModeHorizontal', false);
   }
+
+  _updateScrollMode() { }
+
+  _updateSpreadMode() { }
 }
 
 export {

diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js
@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { BaseViewer, ScrollMode, SpreadMode } from './base_viewer';
+import { BaseViewer, ScrollMode } from './base_viewer';
 import { getVisibleElements, scrollIntoView } from './ui_utils';
 import { shadow } from 'pdfjs-lib';
 
@@ -93,52 +93,6 @@ class PDFViewer extends BaseViewer {
     return (this.isInPresentationMode ?
             false : this._scrollMode === ScrollMode.HORIZONTAL);
   }
-
-  _updateScrollModeClasses() {
-    const scrollMode = this._scrollMode, viewer = this.viewer;
-
-    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');
-    }
-  }
-
-  _regroupSpreads() {
-    if (!this.pdfDocument) {
-      return;
-    }
-    const viewer = this.viewer, pages = this._pages;
-    // Temporarily remove all the pages from the DOM.
-    viewer.textContent = '';
-
-    if (this._spreadMode === SpreadMode.NONE) {
-      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
-        viewer.appendChild(pages[i].div);
-      }
-    } else {
-      const parity = this._spreadMode - 1;
-      let spread = null;
-      for (let i = 0, iMax = pages.length; i < iMax; ++i) {
-        if (spread === null) {
-          spread = document.createElement('div');
-          spread.className = 'spread';
-          viewer.appendChild(spread);
-        } else if (i % 2 === parity) {
-          spread = spread.cloneNode(false);
-          viewer.appendChild(spread);
-        }
-        spread.appendChild(pages[i].div);
-      }
-    }
-    this.scrollPageIntoView({ pageNumber: this._currentPageNumber, });
-    this.update();
-  }
 }
 
 export {