From 06cda4c2e7a1b0fce1ca3f9bbffa0481a7de1cde Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 23 Jan 2019 09:11:06 +0100 Subject: [PATCH] Move additional code/methods into `BaseViewer` and have the extending classes override/extend methods as necessary This attempts to provide more "default" methods in the base class, in order to reduce unnecessary duplication and to improve self-documentation of the `BaseViewer` class slightly. The following changes are made (in no particular order): - Have `BaseViewer` implement the `_scrollIntoView` method, and *extend* it as necessary in `PDFViewer`/`PDFSinglePageViewer`. - Simply inline the `BaseViewer._resizeBuffer` method, in `BaseViewer.update`, since there's only one call-site at this point. - Provide a default implementation of `_isScrollModeHorizontal` in `BaseViewer`, and have `PDFSinglePageViewer` override it. - Provide a default implementation of `_getVisiblePages`, and have `PDFViewer` extend it and `PDFSinglePageViewer` override it. --- web/base_viewer.js | 32 ++++++++++++++------------------ web/pdf_single_page_viewer.js | 3 +-- web/pdf_viewer.js | 27 +++++++++------------------ 3 files changed, 24 insertions(+), 38 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 8e54b554f..d3f03214c 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -15,9 +15,10 @@ import { CSS_UNITS, DEFAULT_SCALE, DEFAULT_SCALE_VALUE, getGlobalEventBus, - isPortraitOrientation, isValidRotation, MAX_AUTO_SCALE, moveToEndOfArray, - NullL10n, PresentationModeState, RendererType, SCROLLBAR_PADDING, - TextLayerMode, UNKNOWN_SCALE, VERTICAL_PADDING, watchScroll + getVisibleElements, isPortraitOrientation, isValidRotation, MAX_AUTO_SCALE, + moveToEndOfArray, NullL10n, PresentationModeState, RendererType, + SCROLLBAR_PADDING, scrollIntoView, TextLayerMode, UNKNOWN_SCALE, + VERTICAL_PADDING, watchScroll } from './ui_utils'; import { PDFRenderingQueue, RenderingStates } from './pdf_rendering_queue'; import { AnnotationLayerBuilder } from './annotation_layer_builder'; @@ -360,6 +361,7 @@ class BaseViewer { } get _setDocumentViewerElement() { + // In most viewers, e.g. `PDFViewer`, this should return `this.viewer`. throw new Error('Not implemented: _setDocumentViewerElement'); } @@ -545,7 +547,7 @@ class BaseViewer { } _scrollIntoView({ pageDiv, pageSpot = null, pageNumber = null, }) { - throw new Error('Not implemented: _scrollIntoView'); + scrollIntoView(pageDiv, pageSpot); } _setScaleUpdatePages(newScale, newValue, noScroll = false, preset = false) { @@ -784,17 +786,6 @@ class BaseViewer { }); } - /** - * visiblePages is optional; if present, it should be an array of pages and in - * practice its length is going to be numVisiblePages, but this is not - * required. The new size of the buffer depends only on numVisiblePages. - */ - _resizeBuffer(numVisiblePages, visiblePages) { - let suggestedCacheSize = Math.max(DEFAULT_CACHE_SIZE, - 2 * numVisiblePages + 1); - this._buffer.resize(suggestedCacheSize, visiblePages); - } - _updateLocation(firstPage) { let currentScale = this._currentScale; let currentScaleValue = this._currentScaleValue; @@ -835,7 +826,8 @@ class BaseViewer { if (numVisiblePages === 0) { return; } - this._resizeBuffer(numVisiblePages, visiblePages); + const newCacheSize = Math.max(DEFAULT_CACHE_SIZE, 2 * numVisiblePages + 1); + this._buffer.resize(newCacheSize, visiblePages); this.renderingQueue.renderHighestPriority(visible); @@ -857,7 +849,10 @@ class BaseViewer { } get _isScrollModeHorizontal() { - throw new Error('Not implemented: _isScrollModeHorizontal'); + // Used to ensure that pre-rendering of the next/previous page works + // correctly, since Scroll/Spread modes are ignored in Presentation Mode. + return (this.isInPresentationMode ? + false : this._scrollMode === ScrollMode.HORIZONTAL); } get isInPresentationMode() { @@ -903,7 +898,8 @@ class BaseViewer { } _getVisiblePages() { - throw new Error('Not implemented: _getVisiblePages'); + return getVisibleElements(this.container, this._pages, true, + this._isScrollModeHorizontal); } /** diff --git a/web/pdf_single_page_viewer.js b/web/pdf_single_page_viewer.js index e07f555c9..8ef656728 100644 --- a/web/pdf_single_page_viewer.js +++ b/web/pdf_single_page_viewer.js @@ -14,7 +14,6 @@ */ import { BaseViewer } from './base_viewer'; -import { scrollIntoView } from './ui_utils'; import { shadow } from 'pdfjs-lib'; class PDFSinglePageViewer extends BaseViewer { @@ -87,7 +86,7 @@ class PDFSinglePageViewer extends BaseViewer { let previousLocation = this._location; this._ensurePageViewVisible(); - scrollIntoView(pageDiv, pageSpot); + super._scrollIntoView({ pageDiv, pageSpot, pageNumber, }); // Since scrolling is tracked using `requestAnimationFrame`, update the // scroll direction during the next `this._scrollUpdate` invocation. diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index bc439e995..7bf464728 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -13,8 +13,7 @@ * limitations under the License. */ -import { BaseViewer, ScrollMode } from './base_viewer'; -import { getVisibleElements, scrollIntoView } from './ui_utils'; +import { BaseViewer } from './base_viewer'; import { shadow } from 'pdfjs-lib'; class PDFViewer extends BaseViewer { @@ -22,27 +21,26 @@ class PDFViewer extends BaseViewer { return shadow(this, '_setDocumentViewerElement', this.viewer); } - _scrollIntoView({ pageDiv, pageSpot = null, }) { + _scrollIntoView({ pageDiv, pageSpot = null, pageNumber = null, }) { if (!pageSpot && !this.isInPresentationMode) { const left = pageDiv.offsetLeft + pageDiv.clientLeft; const right = left + pageDiv.clientWidth; const { scrollLeft, clientWidth, } = this.container; - if (this._scrollMode === ScrollMode.HORIZONTAL || + if (this._isScrollModeHorizontal || left < scrollLeft || right > scrollLeft + clientWidth) { pageSpot = { left: 0, top: 0, }; } } - scrollIntoView(pageDiv, pageSpot); + super._scrollIntoView({ pageDiv, pageSpot, pageNumber, }); } _getVisiblePages() { - if (!this.isInPresentationMode) { - return getVisibleElements(this.container, this._pages, true, - this._scrollMode === ScrollMode.HORIZONTAL); + if (this.isInPresentationMode) { + // The algorithm in `getVisibleElements` doesn't work in all browsers and + // configurations (e.g. Chrome) when Presentation Mode is active. + return this._getCurrentVisiblePage(); } - // The algorithm in `getVisibleElements` doesn't work in all browsers and - // configurations when presentation mode is active. - return this._getCurrentVisiblePage(); + return super._getVisiblePages(); } _updateHelper(visiblePages) { @@ -66,13 +64,6 @@ class PDFViewer extends BaseViewer { } this._setCurrentPageNumber(currentId); } - - get _isScrollModeHorizontal() { - // Used to ensure that pre-rendering of the next/previous page works - // correctly, since Scroll/Spread modes are ignored in Presentation Mode. - return (this.isInPresentationMode ? - false : this._scrollMode === ScrollMode.HORIZONTAL); - } } export {