From ce82772fa76c1892c9e5514d070615d64be67153 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Mon, 3 Dec 2018 14:57:31 -0800 Subject: [PATCH 01/29] New: Thumbnails toggle button (#875) --- src/i18n/en-US.properties | 2 + src/lib/icons/icons.js | 2 + src/lib/icons/thumbnails-toggle-icon.svg | 6 ++ src/lib/viewers/doc/DocBaseViewer.js | 41 ++++++++++- src/lib/viewers/doc/DocumentViewer.js | 24 ------- src/lib/viewers/doc/PresentationViewer.js | 25 ------- .../doc/__tests__/DocBaseViewer-test.js | 68 ++++++++++++++++++- .../doc/__tests__/DocumentViewer-test.js | 41 ----------- .../doc/__tests__/PresentationViewer-test.js | 48 ------------- 9 files changed, 115 insertions(+), 142 deletions(-) create mode 100644 src/lib/icons/thumbnails-toggle-icon.svg diff --git a/src/i18n/en-US.properties b/src/i18n/en-US.properties index 43c7a421d..4f6dae52c 100644 --- a/src/i18n/en-US.properties +++ b/src/i18n/en-US.properties @@ -34,6 +34,8 @@ loading_preview=Loading Preview... download_file=Download File # Text shown when a text file has been truncated due to size limits. text_truncated=This file has been truncated due to size limits. Please download to view the whole file. +# Button tooltip to toggle Thumbnails Sidebar +toggle_thumbnails=Toggle thumbnails # Error messages # Default preview error message diff --git a/src/lib/icons/icons.js b/src/lib/icons/icons.js index 95243ec1f..cfc7665b2 100644 --- a/src/lib/icons/icons.js +++ b/src/lib/icons/icons.js @@ -44,6 +44,7 @@ import FIND_DROP_UP from './arrow_drop_up.svg'; import CLOSE from './close.svg'; import SEARCH from './search.svg'; import PRINT_CHECKMARK from './print_checkmark.svg'; +import THUMBNAILS_TOGGLE from './thumbnails-toggle-icon.svg'; export const ICON_DROP_DOWN = DROP_DOWN; export const ICON_DROP_UP = DROP_UP; @@ -68,6 +69,7 @@ export const ICON_FIND_DROP_UP = FIND_DROP_UP; export const ICON_CLOSE = CLOSE; export const ICON_SEARCH = SEARCH; export const ICON_PRINT_CHECKMARK = PRINT_CHECKMARK; +export const ICON_THUMBNAILS_TOGGLE = THUMBNAILS_TOGGLE; const FILE_LOADING_ICONS = { FILE_AUDIO, diff --git a/src/lib/icons/thumbnails-toggle-icon.svg b/src/lib/icons/thumbnails-toggle-icon.svg new file mode 100644 index 000000000..86fccb296 --- /dev/null +++ b/src/lib/icons/thumbnails-toggle-icon.svg @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 58d402b9f..254be1da5 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -29,7 +29,14 @@ import { getDistance, getClosestPageToPinch } from '../../util'; -import { ICON_PRINT_CHECKMARK } from '../../icons/icons'; +import { + ICON_PRINT_CHECKMARK, + ICON_ZOOM_OUT, + ICON_ZOOM_IN, + ICON_FULLSCREEN_IN, + ICON_FULLSCREEN_OUT, + ICON_THUMBNAILS_TOGGLE +} from '../../icons/icons'; import { JS, PRELOAD_JS, CSS } from './docAssets'; import { ERROR_CODE, VIEWER_EVENT, LOAD_METRIC } from '../../events'; import Timer from '../../Timer'; @@ -83,6 +90,7 @@ class DocBaseViewer extends BaseViewer { this.pinchToZoomChangeHandler = this.pinchToZoomChangeHandler.bind(this); this.pinchToZoomEndHandler = this.pinchToZoomEndHandler.bind(this); this.emitMetric = this.emitMetric.bind(this); + this.toggleThumbnails = this.toggleThumbnails.bind(this); } /** @@ -971,12 +979,31 @@ class DocBaseViewer extends BaseViewer { } /** - * Binds listeners for document controls. Overridden. + * Binds listeners for document controls * * @protected * @return {void} */ - bindControlListeners() {} + bindControlListeners() { + this.controls.add( + __('toggle_thumbnails'), + this.toggleThumbnails, + 'bp-toggle-thumbnails-icon', + ICON_THUMBNAILS_TOGGLE + ); + this.controls.add(__('zoom_out'), this.zoomOut, 'bp-doc-zoom-out-icon', ICON_ZOOM_OUT); + this.controls.add(__('zoom_in'), this.zoomIn, 'bp-doc-zoom-in-icon', ICON_ZOOM_IN); + + this.pageControls.add(this.pdfViewer.currentPageNumber, this.pdfViewer.pagesCount); + + this.controls.add( + __('enter_fullscreen'), + this.toggleFullscreen, + 'bp-enter-fullscreen-icon', + ICON_FULLSCREEN_IN + ); + this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); + } /** * Handler for 'pagesinit' event. @@ -1237,6 +1264,14 @@ class DocBaseViewer extends BaseViewer { this.pinchScale = 1; this.pinchPage = null; } + + /** + * Callback when the toggle thumbnail sidebar button is clicked. + * + * @protected + * @return {void} + */ + toggleThumbnails() {} } export default DocBaseViewer; diff --git a/src/lib/viewers/doc/DocumentViewer.js b/src/lib/viewers/doc/DocumentViewer.js index f76a55cfd..a6e74e290 100644 --- a/src/lib/viewers/doc/DocumentViewer.js +++ b/src/lib/viewers/doc/DocumentViewer.js @@ -1,7 +1,6 @@ import DocBaseViewer from './DocBaseViewer'; import DocPreloader from './DocPreloader'; import fullscreen from '../../Fullscreen'; -import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT, ICON_ZOOM_IN, ICON_ZOOM_OUT } from '../../icons/icons'; import './Document.scss'; class DocumentViewer extends DocBaseViewer { @@ -59,29 +58,6 @@ class DocumentViewer extends DocBaseViewer { //-------------------------------------------------------------------------- // Event Listeners //-------------------------------------------------------------------------- - - /** - * Bind event listeners for document controls - * - * @private - * @return {void} - */ - bindControlListeners() { - super.bindControlListeners(); - - this.controls.add(__('zoom_out'), this.zoomOut, 'bp-doc-zoom-out-icon', ICON_ZOOM_OUT); - this.controls.add(__('zoom_in'), this.zoomIn, 'bp-doc-zoom-in-icon', ICON_ZOOM_IN); - - this.pageControls.add(this.pdfViewer.currentPageNumber, this.pdfViewer.pagesCount); - - this.controls.add( - __('enter_fullscreen'), - this.toggleFullscreen, - 'bp-enter-fullscreen-icon', - ICON_FULLSCREEN_IN - ); - this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); - } } export default DocumentViewer; diff --git a/src/lib/viewers/doc/PresentationViewer.js b/src/lib/viewers/doc/PresentationViewer.js index c355c1516..f7189257f 100644 --- a/src/lib/viewers/doc/PresentationViewer.js +++ b/src/lib/viewers/doc/PresentationViewer.js @@ -2,7 +2,6 @@ import throttle from 'lodash/throttle'; import DocBaseViewer from './DocBaseViewer'; import PresentationPreloader from './PresentationPreloader'; import { CLASS_INVISIBLE } from '../../constants'; -import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT, ICON_ZOOM_IN, ICON_ZOOM_OUT } from '../../icons/icons'; import './Presentation.scss'; const WHEEL_THROTTLE = 200; @@ -177,30 +176,6 @@ class PresentationViewer extends DocBaseViewer { } } - /** - * Adds event listeners for presentation controls - * - * @override - * @return {void} - * @protected - */ - bindControlListeners() { - super.bindControlListeners(); - - this.controls.add(__('zoom_out'), this.zoomOut, 'bp-exit-zoom-out-icon', ICON_ZOOM_OUT); - this.controls.add(__('zoom_in'), this.zoomIn, 'bp-enter-zoom-in-icon', ICON_ZOOM_IN); - - this.pageControls.add(this.pdfViewer.currentPageNumber, this.pdfViewer.pagesCount); - - this.controls.add( - __('enter_fullscreen'), - this.toggleFullscreen, - 'bp-enter-fullscreen-icon', - ICON_FULLSCREEN_IN - ); - this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); - } - /** * Handler for mobile scroll events. * diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 5dfa3d472..5f9ebba94 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -20,7 +20,14 @@ import { QUERY_PARAM_ENCODING, ENCODING_TYPES } from '../../../constants'; -import { ICON_PRINT_CHECKMARK } from '../../../icons/icons'; +import { + ICON_PRINT_CHECKMARK, + ICON_THUMBNAILS_TOGGLE, + ICON_ZOOM_OUT, + ICON_ZOOM_IN, + ICON_FULLSCREEN_IN, + ICON_FULLSCREEN_OUT +} from '../../../icons/icons'; import { VIEWER_EVENT, LOAD_METRIC } from '../../../events'; import Timer from '../../../Timer'; @@ -2013,4 +2020,63 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(docBase.getStartPage(startAt)).to.be.undefined; }); }); + + describe('bindControlListeners()', () => { + beforeEach(() => { + docBase.pdfViewer = { + pagesCount: 4, + currentPageNumber: 1, + cleanup: sandbox.stub() + }; + + docBase.controls = { + add: sandbox.stub(), + removeListener: sandbox.stub() + }; + + docBase.pageControls = { + add: sandbox.stub(), + removeListener: sandbox.stub() + }; + }); + + it('should add the correct controls', () => { + docBase.bindControlListeners(); + + expect(docBase.controls.add).to.be.calledWith( + __('toggle_thumbnails'), + docBase.toggleThumbnails, + 'bp-toggle-thumbnails-icon', + ICON_THUMBNAILS_TOGGLE + ); + + expect(docBase.controls.add).to.be.calledWith( + __('zoom_out'), + docBase.zoomOut, + 'bp-doc-zoom-out-icon', + ICON_ZOOM_OUT + ); + expect(docBase.controls.add).to.be.calledWith( + __('zoom_in'), + docBase.zoomIn, + 'bp-doc-zoom-in-icon', + ICON_ZOOM_IN + ); + + expect(docBase.pageControls.add).to.be.calledWith(1, 4); + + expect(docBase.controls.add).to.be.calledWith( + __('enter_fullscreen'), + docBase.toggleFullscreen, + 'bp-enter-fullscreen-icon', + ICON_FULLSCREEN_IN + ); + expect(docBase.controls.add).to.be.calledWith( + __('exit_fullscreen'), + docBase.toggleFullscreen, + 'bp-exit-fullscreen-icon', + ICON_FULLSCREEN_OUT + ); + }); + }); }); diff --git a/src/lib/viewers/doc/__tests__/DocumentViewer-test.js b/src/lib/viewers/doc/__tests__/DocumentViewer-test.js index 1d6d4b989..53635bcc2 100644 --- a/src/lib/viewers/doc/__tests__/DocumentViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocumentViewer-test.js @@ -4,7 +4,6 @@ import DocBaseViewer from '../DocBaseViewer'; import BaseViewer from '../../BaseViewer'; import DocPreloader from '../DocPreloader'; import fullscreen from '../../../Fullscreen'; -import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT, ICON_ZOOM_IN, ICON_ZOOM_OUT } from '../../../icons/icons'; const sandbox = sinon.sandbox.create(); @@ -147,44 +146,4 @@ describe('lib/viewers/doc/DocumentViewer', () => { expect(docbaseStub).to.have.been.calledTwice; }); }); - - describe('bindControlListeners()', () => { - beforeEach(() => { - doc.pdfViewer = { - pagesCount: 4, - cleanup: sandbox.stub() - }; - - doc.pageControls = { - add: sandbox.stub(), - removeListener: sandbox.stub() - }; - }); - - it('should add the correct controls', () => { - doc.bindControlListeners(); - expect(doc.controls.add).to.be.calledWith( - __('zoom_out'), - doc.zoomOut, - 'bp-doc-zoom-out-icon', - ICON_ZOOM_OUT - ); - expect(doc.controls.add).to.be.calledWith(__('zoom_in'), doc.zoomIn, 'bp-doc-zoom-in-icon', ICON_ZOOM_IN); - - expect(doc.pageControls.add).to.be.called; - - expect(doc.controls.add).to.be.calledWith( - __('enter_fullscreen'), - doc.toggleFullscreen, - 'bp-enter-fullscreen-icon', - ICON_FULLSCREEN_IN - ); - expect(doc.controls.add).to.be.calledWith( - __('exit_fullscreen'), - doc.toggleFullscreen, - 'bp-exit-fullscreen-icon', - ICON_FULLSCREEN_OUT - ); - }); - }); }); diff --git a/src/lib/viewers/doc/__tests__/PresentationViewer-test.js b/src/lib/viewers/doc/__tests__/PresentationViewer-test.js index 4823e600d..5bd7b6cfa 100644 --- a/src/lib/viewers/doc/__tests__/PresentationViewer-test.js +++ b/src/lib/viewers/doc/__tests__/PresentationViewer-test.js @@ -5,8 +5,6 @@ import DocBaseViewer from '../DocBaseViewer'; import PresentationPreloader from '../PresentationPreloader'; import { CLASS_INVISIBLE } from '../../../constants'; -import { ICON_ZOOM_OUT, ICON_ZOOM_IN, ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT } from '../../../icons/icons'; - const sandbox = sinon.sandbox.create(); let containerEl; @@ -293,52 +291,6 @@ describe('lib/viewers/doc/PresentationViewer', () => { }); }); - describe('bindControlListeners()', () => { - beforeEach(() => { - presentation.pdfViewer = { - pagesCount: 4, - currentPageNumber: 1, - cleanup: sandbox.stub() - }; - - presentation.pageControls = { - add: sandbox.stub(), - removeListener: sandbox.stub() - }; - }); - - it('should add the correct controls', () => { - presentation.bindControlListeners(); - expect(presentation.controls.add).to.be.calledWith( - __('zoom_out'), - presentation.zoomOut, - 'bp-exit-zoom-out-icon', - ICON_ZOOM_OUT - ); - expect(presentation.controls.add).to.be.calledWith( - __('zoom_in'), - presentation.zoomIn, - 'bp-enter-zoom-in-icon', - ICON_ZOOM_IN - ); - - expect(presentation.pageControls.add).to.be.calledWith(1, 4); - - expect(presentation.controls.add).to.be.calledWith( - __('enter_fullscreen'), - presentation.toggleFullscreen, - 'bp-enter-fullscreen-icon', - ICON_FULLSCREEN_IN - ); - expect(presentation.controls.add).to.be.calledWith( - __('exit_fullscreen'), - presentation.toggleFullscreen, - 'bp-exit-fullscreen-icon', - ICON_FULLSCREEN_OUT - ); - }); - }); - describe('mobileScrollHandler()', () => { beforeEach(() => { stubs.checkOverflow = sandbox.stub(presentation, 'checkOverflow').returns(false); From 76bab2a7fa0404ed5c990c1682609fbf5fba5c9d Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 11 Dec 2018 13:13:57 -0800 Subject: [PATCH 02/29] New: Thumbnails toggle sidebar (#876) --- src/lib/__tests__/PreviewUI-test.html | 1 + src/lib/_common.scss | 11 ++++-- src/lib/_loading.scss | 6 +++ src/lib/constants.js | 4 ++ src/lib/shell.html | 26 +++++++------ src/lib/viewers/BaseViewer.js | 37 ++++++++++++++++--- .../viewers/__tests__/BaseViewer-test.html | 4 +- src/lib/viewers/__tests__/BaseViewer-test.js | 31 +++++++++++++++- src/lib/viewers/box3d/Box3DViewer.js | 2 +- .../box3d/__tests__/Box3DViewer-test.html | 8 +++- .../box3d/__tests__/Box3DViewer-test.js | 23 +++++++++--- .../__tests__/Image360Viewer-test.html | 8 +++- .../image360/__tests__/Image360Viewer-test.js | 3 +- .../model3d/__tests__/Model3DViewer-test.html | 8 +++- .../__tests__/Video360Viewer-test.html | 8 +++- src/lib/viewers/doc/DocBaseViewer.js | 19 ++++++++-- .../doc/__tests__/DocBaseViewer-test.html | 9 ++++- .../doc/__tests__/DocBaseViewer-test.js | 31 +++++++++++++++- .../doc/__tests__/DocumentViewer-test.html | 8 +++- .../__tests__/PresentationViewer-test.html | 8 +++- .../doc/__tests__/SinglePageViewer-test.html | 8 +++- src/lib/viewers/doc/_docBase.scss | 15 ++++++++ src/lib/viewers/error/PreviewErrorViewer.js | 2 +- .../__tests__/PreviewErrorViewer-test.html | 8 +++- src/lib/viewers/iframe/IFrameViewer.js | 2 +- .../iframe/__tests__/IFrameViewer-test.html | 8 +++- src/lib/viewers/image/ImageViewer.js | 2 +- src/lib/viewers/image/MultiImageViewer.js | 2 +- .../image/__tests__/ImageViewer-test.html | 10 ++++- .../__tests__/MultiImageViewer-test.html | 11 +++++- src/lib/viewers/media/MediaBaseViewer.js | 2 +- .../media/__tests__/DashViewer-test.html | 6 ++- .../media/__tests__/MP3Viewer-test.html | 8 +++- .../media/__tests__/MP4Viewer-test.html | 8 +++- .../media/__tests__/MediaBaseViewer-test.html | 8 +++- .../media/__tests__/VideoBaseViewer-test.html | 8 +++- src/lib/viewers/office/OfficeViewer.js | 2 +- .../office/__tests__/OfficeViewer-test.html | 12 +++++- src/lib/viewers/swf/SWFViewer.js | 2 +- .../viewers/swf/__tests__/SWFViewer-test.html | 12 +++++- src/lib/viewers/text/CSVViewer.js | 2 +- src/lib/viewers/text/PlainTextViewer.js | 2 +- .../text/__tests__/CSVViewer-test.html | 8 +++- .../text/__tests__/MarkdownViewer-test.html | 8 +++- .../text/__tests__/PlainTextViewer-test.html | 8 +++- 45 files changed, 349 insertions(+), 70 deletions(-) diff --git a/src/lib/__tests__/PreviewUI-test.html b/src/lib/__tests__/PreviewUI-test.html index ff06f11c4..af105a43c 100644 --- a/src/lib/__tests__/PreviewUI-test.html +++ b/src/lib/__tests__/PreviewUI-test.html @@ -5,4 +5,5 @@ +
diff --git a/src/lib/_common.scss b/src/lib/_common.scss index 14c9946dd..c402f7464 100644 --- a/src/lib/_common.scss +++ b/src/lib/_common.scss @@ -118,13 +118,11 @@ $header-height: 48px; } .bp { - align-items: center; background-color: $ffive; border: 0 none; bottom: 0; display: flex; - flex-direction: column; - justify-content: center; + flex: 1 1 100%; left: 0; margin: 0; outline: none; @@ -146,6 +144,13 @@ $header-height: 48px; &.bp-dark { background-color: $black; } + + .bp-content { + align-items: center; + display: flex; + flex: 1 1 auto; + position: relative; + } } .accessibility-hidden { diff --git a/src/lib/_loading.scss b/src/lib/_loading.scss index ae9189306..73876d168 100644 --- a/src/lib/_loading.scss +++ b/src/lib/_loading.scss @@ -29,8 +29,14 @@ animation-duration: 1s; animation-iteration-count: 1; animation-name: fadeIn; + bottom: 0; display: flex; flex-direction: column; + justify-content: center; + left: 0; + position: absolute; + right: 0; + top: 0; transition: .25s opacity, .25s transform; .bp-loaded & { diff --git a/src/lib/constants.js b/src/lib/constants.js index 8248a0280..b847368cc 100644 --- a/src/lib/constants.js +++ b/src/lib/constants.js @@ -5,6 +5,7 @@ export const CLASS_PREVIEW_LOADED = 'bp-loaded'; export const CLASS_BOX_PREVIEW = 'bp'; export const CLASS_BOX_PREVIEW_BUTTON = 'bp-btn'; export const CLASS_BOX_PREVIEW_CONTAINER = 'bp-container'; +export const CLASS_BOX_PREVIEW_CONTENT = 'bp-content'; export const CLASS_BOX_PREVIEW_FIND_BAR = 'bp-find-bar'; export const CLASS_BOX_PREVIEW_HAS_HEADER = 'bp-has-header'; export const CLASS_BOX_PREVIEW_HAS_NAVIGATION = 'bp-has-navigation'; @@ -33,6 +34,7 @@ export const CLASS_BOX_PREVIEW_NOTIFICATION = 'bp-notification'; export const CLASS_BOX_PREVIEW_NOTIFICATION_WRAPPER = 'bp-notifications-wrapper'; export const CLASS_BOX_PREVIEW_TOGGLE_OVERLAY = 'bp-toggle-overlay'; export const CLASS_BOX_PREVIEW_THEME_DARK = 'bp-theme-dark'; +export const CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER = 'bp-thumbnails-container'; export const CLASS_ELEM_KEYBOARD_FOCUS = 'bp-has-keyboard-focus'; export const CLASS_FULLSCREEN = 'bp-is-fullscreen'; export const CLASS_FULLSCREEN_UNSUPPORTED = 'bp-fullscreen-unsupported'; @@ -49,6 +51,7 @@ export const CLASS_SPINNER = 'bp-spinner'; export const SELECTOR_BOX_PREVIEW_CONTAINER = `.${CLASS_BOX_PREVIEW_CONTAINER}`; export const SELECTOR_BOX_PREVIEW = `.${CLASS_BOX_PREVIEW}`; +export const SELECTOR_BOX_PREVIEW_CONTENT = `.${CLASS_BOX_PREVIEW_CONTENT}`; export const SELECTOR_BOX_PREVIEW_CRAWLER_WRAPPER = '.bp-crawler-wrapper'; export const SELECTOR_BOX_PREVIEW_HEADER_BTNS = `.${CLASS_BOX_PREVIEW_HEADER_BTNS}`; export const SELECTOR_NAVIGATION_LEFT = '.bp-navigate-left'; @@ -67,6 +70,7 @@ export const SELECTOR_BOX_PREVIEW_LOGO_CUSTOM = `.${CLASS_BOX_PREVIEW_LOGO_CUSTO export const SELECTOR_BOX_PREVIEW_LOGO_DEFAULT = `.${CLASS_BOX_PREVIEW_LOGO_DEFAULT}`; export const SELECTOR_BOX_PREVIEW_PROGRESS_BAR = `.${CLASS_BOX_PREVIEW_PROGRESS_BAR}`; export const SELECTOR_BOX_PREVIEW_NOTIFICATION = `.${CLASS_BOX_PREVIEW_NOTIFICATION}`; +export const SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER = `.${CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER}`; export const PERMISSION_DOWNLOAD = 'can_download'; export const PERMISSION_PREVIEW = 'can_preview'; diff --git a/src/lib/shell.html b/src/lib/shell.html index 773554553..ea11e09a4 100644 --- a/src/lib/shell.html +++ b/src/lib/shell.html @@ -50,17 +50,19 @@ +
+ + +
- - diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 7e56232b7..4b1537777 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -18,12 +18,12 @@ import { replacePlaceholders } from '../util'; import { - CLASS_HIDDEN, CLASS_BOX_PREVIEW_MOBILE, + CLASS_HIDDEN, FILE_OPTION_START, - SELECTOR_BOX_PREVIEW, - SELECTOR_BOX_PREVIEW_BTN_ANNOTATE_POINT, SELECTOR_BOX_PREVIEW_BTN_ANNOTATE_DRAW, + SELECTOR_BOX_PREVIEW_BTN_ANNOTATE_POINT, + SELECTOR_BOX_PREVIEW_CONTENT, SELECTOR_BOX_PREVIEW_CRAWLER_WRAPPER, SELECTOR_BOX_PREVIEW_ICON, STATUS_SUCCESS, @@ -141,6 +141,7 @@ class BaseViewer extends EventEmitter { this.viewerLoadHandler = this.viewerLoadHandler.bind(this); this.initAnnotations = this.initAnnotations.bind(this); this.loadBoxAnnotations = this.loadBoxAnnotations.bind(this); + this.createViewer = this.createViewer.bind(this); } /** @@ -163,8 +164,8 @@ class BaseViewer extends EventEmitter { container = document.querySelector(container); } - // From the perspective of viewers bp holds everything - this.containerEl = container.querySelector(SELECTOR_BOX_PREVIEW); + // From the perspective of viewers bp-content holds everything + this.containerEl = container.querySelector(SELECTOR_BOX_PREVIEW_CONTENT); // Attach event listeners this.addCommonListeners(); @@ -1099,6 +1100,32 @@ class BaseViewer extends EventEmitter { handleAssetAndRepLoad() { this.loadBoxAnnotations().then(this.createAnnotator); } + + /** + * Method to insert the viewer wrapper + * + * @param {HTMLElement} element Element to be inserted into the DOM + * @return {HTMLElement} inserted element + */ + createViewer(element) { + if (!element) { + return null; + } + + const firstChildEl = this.containerEl.firstChild; + let addedElement; + + if (!firstChildEl) { + addedElement = this.containerEl.appendChild(element); + } else { + // Need to insert the viewer wrapper as the first element in the container + // so that we can perserve the natural stacking context to have the prev/next + // file buttons on top of the previewed content + addedElement = this.containerEl.insertBefore(element, firstChildEl); + } + + return addedElement; + } } export default BaseViewer; diff --git a/src/lib/viewers/__tests__/BaseViewer-test.html b/src/lib/viewers/__tests__/BaseViewer-test.html index 5b6c68a9e..d26c2c122 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.html +++ b/src/lib/viewers/__tests__/BaseViewer-test.html @@ -1,3 +1,5 @@
-
+
+
+
diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 624a1755d..0a3a2a311 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -81,7 +81,7 @@ describe('lib/viewers/BaseViewer', () => { showAnnotations: true }); - expect(base.containerEl).to.have.class('bp'); + expect(base.containerEl).to.have.class(constants.CLASS_BOX_PREVIEW_CONTENT); expect(base.addCommonListeners).to.be.called; expect(getIconFromExtensionStub).to.be.called; expect(base.loadTimeout).to.be.a('number'); @@ -97,7 +97,7 @@ describe('lib/viewers/BaseViewer', () => { base.setup(); - const container = document.querySelector('.bp'); + const container = document.querySelector(constants.SELECTOR_BOX_PREVIEW_CONTENT); expect(container).to.have.class('bp-is-mobile'); }); @@ -1385,4 +1385,31 @@ describe('lib/viewers/BaseViewer', () => { expect(base.createAnnotator).to.be.called; }); }); + + describe('createViewer()', () => { + it('should return null if no element is provided', () => { + expect(base.createViewer()).to.be.null; + }); + + it('should append the element if containerEl has no first child', () => { + base.containerEl = document.querySelector(constants.SELECTOR_BOX_PREVIEW_CONTENT); + const newDiv = document.createElement('div'); + sandbox.stub(base.containerEl, 'appendChild'); + base.createViewer(newDiv); + expect(base.containerEl.appendChild).to.be.called; + }); + + it('should insert the provided element before the other children', () => { + base.containerEl = document.querySelector(constants.SELECTOR_BOX_PREVIEW_CONTENT); + const existingChild = document.createElement('div'); + existingChild.className = 'existing-child'; + base.containerEl.appendChild(existingChild); + + sandbox.stub(base.containerEl, 'insertBefore'); + const newDiv = document.createElement('div'); + newDiv.className = 'new-div'; + base.createViewer(newDiv); + expect(base.containerEl.insertBefore).to.be.called; + }); + }); }); diff --git a/src/lib/viewers/box3d/Box3DViewer.js b/src/lib/viewers/box3d/Box3DViewer.js index ba4bb5581..ffbc45864 100644 --- a/src/lib/viewers/box3d/Box3DViewer.js +++ b/src/lib/viewers/box3d/Box3DViewer.js @@ -67,7 +67,7 @@ class Box3DViewer extends BaseViewer { this.renderer = null; - this.wrapperEl = this.containerEl.appendChild(document.createElement('div')); + this.wrapperEl = this.createViewer(document.createElement('div')); this.wrapperEl.className = CSS_CLASS_BOX3D; this.contextNotification = new Notification(this.wrapperEl); diff --git a/src/lib/viewers/box3d/__tests__/Box3DViewer-test.html b/src/lib/viewers/box3d/__tests__/Box3DViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/box3d/__tests__/Box3DViewer-test.html +++ b/src/lib/viewers/box3d/__tests__/Box3DViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/box3d/__tests__/Box3DViewer-test.js b/src/lib/viewers/box3d/__tests__/Box3DViewer-test.js index 4f2bf9bef..6a91aacb9 100644 --- a/src/lib/viewers/box3d/__tests__/Box3DViewer-test.js +++ b/src/lib/viewers/box3d/__tests__/Box3DViewer-test.js @@ -17,6 +17,7 @@ import { EVENT_WEBGL_CONTEXT_RESTORED } from '../box3DConstants'; import { VIEWER_EVENT } from '../../../events'; +import { SELECTOR_BOX_PREVIEW_CONTENT } from '../../../constants'; const sandbox = sinon.sandbox.create(); @@ -54,7 +55,7 @@ describe('lib/viewers/box3d/Box3DViewer', () => { }); Object.defineProperty(BaseViewer.prototype, 'setup', { value: sandbox.mock() }); - box3d.containerEl = containerEl; + box3d.containerEl = document.querySelector(SELECTOR_BOX_PREVIEW_CONTENT); box3d.setup(); sandbox.stub(box3d, 'createSubModules'); @@ -104,7 +105,7 @@ describe('lib/viewers/box3d/Box3DViewer', () => { } }); Object.defineProperty(BaseViewer.prototype, 'setup', { value: sandbox.mock() }); - box3d.containerEl = containerEl; + box3d.containerEl = document.querySelector(SELECTOR_BOX_PREVIEW_CONTENT); box3d.setup(); box3d.createSubModules(); @@ -358,7 +359,7 @@ describe('lib/viewers/box3d/Box3DViewer', () => { it('should call renderer.load()', () => { Object.defineProperty(BaseViewer.prototype, 'setup', { value: sandbox.mock() }); - box3d.containerEl = containerEl; + box3d.containerEl = document.querySelector(SELECTOR_BOX_PREVIEW_CONTENT); Object.defineProperty(BaseViewer.prototype, 'load', { value: sandbox.mock() }); sandbox.stub(box3d, 'loadAssets').returns(Promise.resolve()); sandbox.stub(box3d, 'getRepStatus').returns({ getPromise: () => Promise.resolve() }); @@ -383,7 +384,11 @@ describe('lib/viewers/box3d/Box3DViewer', () => { it('should call renderer.load() with the entities.json file and options', () => { const contentUrl = 'someEntitiesJsonUrl'; sandbox.stub(box3d, 'createContentUrl').returns(contentUrl); - sandbox.mock(box3d.renderer).expects('load').withArgs(contentUrl, box3d.options).returns(Promise.resolve()); + sandbox + .mock(box3d.renderer) + .expects('load') + .withArgs(contentUrl, box3d.options) + .returns(Promise.resolve()); box3d.postLoad(); }); @@ -409,14 +414,20 @@ describe('lib/viewers/box3d/Box3DViewer', () => { sandbox.stub(box3d, 'createContentUrl').returns(contentUrl); sandbox.stub(box3d, 'appendAuthHeader').returns(headers); sandbox.stub(box3d, 'isRepresentationReady').returns(true); - sandbox.mock(util).expects('get').withArgs(contentUrl, headers, 'any'); + sandbox + .mock(util) + .expects('get') + .withArgs(contentUrl, headers, 'any'); box3d.prefetch({ assets: false, content: true }); }); it('should not prefetch content if content is true but representation is not ready', () => { sandbox.stub(box3d, 'isRepresentationReady').returns(false); - sandbox.mock(util).expects('get').never(); + sandbox + .mock(util) + .expects('get') + .never(); box3d.prefetch({ assets: false, content: true }); }); }); diff --git a/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.html b/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.html +++ b/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.js b/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.js index 67f357744..676fecb8a 100644 --- a/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.js +++ b/src/lib/viewers/box3d/image360/__tests__/Image360Viewer-test.js @@ -3,6 +3,7 @@ import Image360Viewer from '../Image360Viewer'; import BaseViewer from '../../../BaseViewer'; import Box3DControls from '../../Box3DControls'; import Image360Renderer from '../Image360Renderer'; +import { SELECTOR_BOX_PREVIEW_CONTENT } from '../../../../constants'; const sandbox = sinon.sandbox.create(); const CSS_CLASS_IMAGE_360 = 'bp-image-360'; @@ -27,7 +28,7 @@ describe('lib/viewers/box3d/image360/Image360Viewer', () => { } }); Object.defineProperty(BaseViewer.prototype, 'setup', { value: sandbox.mock() }); - viewer.containerEl = containerEl; + viewer.containerEl = document.querySelector(SELECTOR_BOX_PREVIEW_CONTENT); }); afterEach(() => { diff --git a/src/lib/viewers/box3d/model3d/__tests__/Model3DViewer-test.html b/src/lib/viewers/box3d/model3d/__tests__/Model3DViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/box3d/model3d/__tests__/Model3DViewer-test.html +++ b/src/lib/viewers/box3d/model3d/__tests__/Model3DViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/box3d/video360/__tests__/Video360Viewer-test.html b/src/lib/viewers/box3d/video360/__tests__/Video360Viewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/box3d/video360/__tests__/Video360Viewer-test.html +++ b/src/lib/viewers/box3d/video360/__tests__/Video360Viewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 254be1da5..462195af3 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -18,7 +18,8 @@ import { PRELOAD_REP_NAME, STATUS_SUCCESS, QUERY_PARAM_ENCODING, - ENCODING_TYPES + ENCODING_TYPES, + CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER } from '../../constants'; import { checkPermission, getRepresentation } from '../../file'; import { @@ -100,7 +101,7 @@ class DocBaseViewer extends BaseViewer { // Call super() to set up common layout super.setup(); - this.docEl = this.containerEl.appendChild(document.createElement('div')); + this.docEl = this.createViewer(document.createElement('div')); this.docEl.classList.add('bp-doc'); if (Browser.getName() === 'Safari') { @@ -121,6 +122,10 @@ class DocBaseViewer extends BaseViewer { this.loadTimeout = LOAD_TIMEOUT_MS; this.startPageNum = this.getStartPage(this.startAt); + + this.thumbnailsSidebarEl = document.createElement('div'); + this.thumbnailsSidebarEl.className = `${CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER} ${CLASS_HIDDEN}`; + this.containerEl.parentNode.insertBefore(this.thumbnailsSidebarEl, this.containerEl); } /** @@ -1271,7 +1276,15 @@ class DocBaseViewer extends BaseViewer { * @protected * @return {void} */ - toggleThumbnails() {} + toggleThumbnails() { + if (!this.thumbnailsSidebarEl) { + return; + } + + this.thumbnailsSidebarEl.classList.toggle(CLASS_HIDDEN); + + this.resize(); + } } export default DocBaseViewer; diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.html b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.html index 7dd9073c1..c51f940ec 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.html +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.html @@ -1 +1,8 @@ -
+
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 5f9ebba94..dc7e032ac 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -18,7 +18,9 @@ import { STATUS_PENDING, STATUS_SUCCESS, QUERY_PARAM_ENCODING, - ENCODING_TYPES + ENCODING_TYPES, + SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER, + SELECTOR_BOX_PREVIEW_CONTENT } from '../../../constants'; import { ICON_PRINT_CHECKMARK, @@ -68,7 +70,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { beforeEach(() => { fixture.load('viewers/doc/__tests__/DocBaseViewer-test.html'); - containerEl = document.querySelector('.container'); + containerEl = document.querySelector(SELECTOR_BOX_PREVIEW_CONTENT); docBase = new DocBaseViewer({ cache: { set: () => {}, @@ -2079,4 +2081,29 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { ); }); }); + + describe('toggleThumbnails()', () => { + beforeEach(() => { + sandbox.stub(docBase, 'resize'); + }); + + it('should do nothing if thumbnails sidebar does not exit', () => { + docBase.thumbnailsSidebarEl = undefined; + + docBase.toggleThumbnails(); + + expect(docBase.resize).not.to.be.called; + }); + + it('should toggle the bp-is-hidden class and resize the viewer', () => { + const thumbnailsSidebarEl = document.querySelector(SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER); + docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; + expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.true; + + docBase.toggleThumbnails(); + + expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.false; + expect(docBase.resize).to.be.called; + }); + }); }); diff --git a/src/lib/viewers/doc/__tests__/DocumentViewer-test.html b/src/lib/viewers/doc/__tests__/DocumentViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/doc/__tests__/DocumentViewer-test.html +++ b/src/lib/viewers/doc/__tests__/DocumentViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/doc/__tests__/PresentationViewer-test.html b/src/lib/viewers/doc/__tests__/PresentationViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/doc/__tests__/PresentationViewer-test.html +++ b/src/lib/viewers/doc/__tests__/PresentationViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/doc/__tests__/SinglePageViewer-test.html b/src/lib/viewers/doc/__tests__/SinglePageViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/doc/__tests__/SinglePageViewer-test.html +++ b/src/lib/viewers/doc/__tests__/SinglePageViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index e9f2a5a2c..ba9e64577 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -1,5 +1,20 @@ @import '../../boxuiVariables'; +.bp { + .bp-thumbnails-container { + border-right: solid 1px $seesee; + flex: 0 0 180px; + } +} + +.bp-theme-dark { + .bp { + .bp-thumbnails-container { + border-right-color: $twos; + } + } +} + .bp-doc { bottom: 0; height: 100%; diff --git a/src/lib/viewers/error/PreviewErrorViewer.js b/src/lib/viewers/error/PreviewErrorViewer.js index dfb639029..a303f30d6 100644 --- a/src/lib/viewers/error/PreviewErrorViewer.js +++ b/src/lib/viewers/error/PreviewErrorViewer.js @@ -24,7 +24,7 @@ class PreviewErrorViewer extends BaseViewer { // Call super() first to set up common layout super.setup(); - this.infoEl = this.containerEl.appendChild(document.createElement('div')); + this.infoEl = this.createViewer(document.createElement('div')); this.infoEl.className = 'bp-error'; this.iconEl = this.infoEl.appendChild(document.createElement('div')); diff --git a/src/lib/viewers/error/__tests__/PreviewErrorViewer-test.html b/src/lib/viewers/error/__tests__/PreviewErrorViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/error/__tests__/PreviewErrorViewer-test.html +++ b/src/lib/viewers/error/__tests__/PreviewErrorViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/iframe/IFrameViewer.js b/src/lib/viewers/iframe/IFrameViewer.js index 8e97f9252..19fe75747 100644 --- a/src/lib/viewers/iframe/IFrameViewer.js +++ b/src/lib/viewers/iframe/IFrameViewer.js @@ -9,7 +9,7 @@ class IFrameViewer extends BaseViewer { // Call super() to set up common layout super.setup(); - this.iframeEl = this.containerEl.appendChild(document.createElement('iframe')); + this.iframeEl = this.createViewer(document.createElement('iframe')); this.iframeEl.setAttribute('width', '100%'); this.iframeEl.setAttribute('height', '100%'); this.iframeEl.setAttribute('frameborder', 0); diff --git a/src/lib/viewers/iframe/__tests__/IFrameViewer-test.html b/src/lib/viewers/iframe/__tests__/IFrameViewer-test.html index e1e597132..dd4e7e124 100644 --- a/src/lib/viewers/iframe/__tests__/IFrameViewer-test.html +++ b/src/lib/viewers/iframe/__tests__/IFrameViewer-test.html @@ -10,4 +10,10 @@ background-color: #eee; } -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/image/ImageViewer.js b/src/lib/viewers/image/ImageViewer.js index 1895b80e9..a2edb6bc8 100644 --- a/src/lib/viewers/image/ImageViewer.js +++ b/src/lib/viewers/image/ImageViewer.js @@ -31,7 +31,7 @@ class ImageViewer extends ImageBaseViewer { // Call super() to set up common layout super.setup(); - this.wrapperEl = this.containerEl.appendChild(document.createElement('div')); + this.wrapperEl = this.createViewer(document.createElement('div')); this.wrapperEl.classList.add(CSS_CLASS_IMAGE); this.imageEl = this.wrapperEl.appendChild(document.createElement('img')); diff --git a/src/lib/viewers/image/MultiImageViewer.js b/src/lib/viewers/image/MultiImageViewer.js index 7a539790b..5fdb3e6ea 100644 --- a/src/lib/viewers/image/MultiImageViewer.js +++ b/src/lib/viewers/image/MultiImageViewer.js @@ -32,7 +32,7 @@ class MultiImageViewer extends ImageBaseViewer { // Call super() to set up common layout super.setup(); - this.wrapperEl = this.containerEl.appendChild(document.createElement('div')); + this.wrapperEl = this.createViewer(document.createElement('div')); this.wrapperEl.classList.add(CSS_CLASS_IMAGE_WRAPPER); this.imageEl = this.wrapperEl.appendChild(document.createElement('div')); diff --git a/src/lib/viewers/image/__tests__/ImageViewer-test.html b/src/lib/viewers/image/__tests__/ImageViewer-test.html index 2ac756dad..6c239e49b 100644 --- a/src/lib/viewers/image/__tests__/ImageViewer-test.html +++ b/src/lib/viewers/image/__tests__/ImageViewer-test.html @@ -10,4 +10,12 @@ background-color: #eee; } -
+
+
+
+
+ +
+
+
+
diff --git a/src/lib/viewers/image/__tests__/MultiImageViewer-test.html b/src/lib/viewers/image/__tests__/MultiImageViewer-test.html index c8a751ef9..35bfca794 100644 --- a/src/lib/viewers/image/__tests__/MultiImageViewer-test.html +++ b/src/lib/viewers/image/__tests__/MultiImageViewer-test.html @@ -1,8 +1,10 @@ -
+
+
+
+
+
+
diff --git a/src/lib/viewers/media/MediaBaseViewer.js b/src/lib/viewers/media/MediaBaseViewer.js index 276ee5539..c94a4a2ca 100644 --- a/src/lib/viewers/media/MediaBaseViewer.js +++ b/src/lib/viewers/media/MediaBaseViewer.js @@ -54,7 +54,7 @@ class MediaBaseViewer extends BaseViewer { super.setup(); // Media Wrapper - this.wrapperEl = this.containerEl.appendChild(document.createElement('div')); + this.wrapperEl = this.createViewer(document.createElement('div')); this.wrapperEl.className = CSS_CLASS_MEDIA; // Media Container diff --git a/src/lib/viewers/media/__tests__/DashViewer-test.html b/src/lib/viewers/media/__tests__/DashViewer-test.html index af8d6055b..325864e03 100644 --- a/src/lib/viewers/media/__tests__/DashViewer-test.html +++ b/src/lib/viewers/media/__tests__/DashViewer-test.html @@ -1 +1,5 @@ -
>
+
+
+
+
> +
diff --git a/src/lib/viewers/media/__tests__/MP3Viewer-test.html b/src/lib/viewers/media/__tests__/MP3Viewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/media/__tests__/MP3Viewer-test.html +++ b/src/lib/viewers/media/__tests__/MP3Viewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/media/__tests__/MP4Viewer-test.html b/src/lib/viewers/media/__tests__/MP4Viewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/media/__tests__/MP4Viewer-test.html +++ b/src/lib/viewers/media/__tests__/MP4Viewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/media/__tests__/MediaBaseViewer-test.html b/src/lib/viewers/media/__tests__/MediaBaseViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/media/__tests__/MediaBaseViewer-test.html +++ b/src/lib/viewers/media/__tests__/MediaBaseViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/media/__tests__/VideoBaseViewer-test.html b/src/lib/viewers/media/__tests__/VideoBaseViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/media/__tests__/VideoBaseViewer-test.html +++ b/src/lib/viewers/media/__tests__/VideoBaseViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/office/OfficeViewer.js b/src/lib/viewers/office/OfficeViewer.js index d2e093b9f..9feecf8b6 100644 --- a/src/lib/viewers/office/OfficeViewer.js +++ b/src/lib/viewers/office/OfficeViewer.js @@ -171,7 +171,7 @@ class OfficeViewer extends BaseViewer { setupIframe() { const { appHost, apiHost, file, sharedLink, location: { locale } } = this.options; const iframeEl = this.createIframeElement(); - this.containerEl.appendChild(iframeEl); + this.createViewer(iframeEl); if (this.platformSetup) { const formEl = this.createFormElement(apiHost, file.id, sharedLink, locale); diff --git a/src/lib/viewers/office/__tests__/OfficeViewer-test.html b/src/lib/viewers/office/__tests__/OfficeViewer-test.html index e1e597132..35bfca794 100644 --- a/src/lib/viewers/office/__tests__/OfficeViewer-test.html +++ b/src/lib/viewers/office/__tests__/OfficeViewer-test.html @@ -1,8 +1,10 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/swf/SWFViewer.js b/src/lib/viewers/swf/SWFViewer.js index 462aa8e16..4459bb017 100644 --- a/src/lib/viewers/swf/SWFViewer.js +++ b/src/lib/viewers/swf/SWFViewer.js @@ -21,7 +21,7 @@ class SWFViewer extends BaseViewer { setup() { // Call super() to set up common layout super.setup(); - this.playerEl = this.containerEl.appendChild(document.createElement('div')); + this.playerEl = this.createViewer(document.createElement('div')); this.playerEl.id = 'flash-player'; } diff --git a/src/lib/viewers/swf/__tests__/SWFViewer-test.html b/src/lib/viewers/swf/__tests__/SWFViewer-test.html index e1e597132..35bfca794 100644 --- a/src/lib/viewers/swf/__tests__/SWFViewer-test.html +++ b/src/lib/viewers/swf/__tests__/SWFViewer-test.html @@ -1,8 +1,10 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/text/CSVViewer.js b/src/lib/viewers/text/CSVViewer.js index 17f6af65a..9935d7e06 100644 --- a/src/lib/viewers/text/CSVViewer.js +++ b/src/lib/viewers/text/CSVViewer.js @@ -15,7 +15,7 @@ class CSVViewer extends TextBaseViewer { // Call super() first to set up common layout super.setup(); - this.csvEl = this.containerEl.appendChild(document.createElement('div')); + this.csvEl = this.createViewer(document.createElement('div')); this.csvEl.className = 'bp-text bp-text-csv'; } diff --git a/src/lib/viewers/text/PlainTextViewer.js b/src/lib/viewers/text/PlainTextViewer.js index 9c1724a26..82d209263 100644 --- a/src/lib/viewers/text/PlainTextViewer.js +++ b/src/lib/viewers/text/PlainTextViewer.js @@ -108,7 +108,7 @@ class PlainTextViewer extends TextBaseViewer { // Call super() first to set up common layout super.setup(); - this.textEl = this.containerEl.appendChild(document.createElement('pre')); + this.textEl = this.createViewer(document.createElement('pre')); this.textEl.className = 'bp-text bp-text-plain hljs'; this.textEl.classList.add(CLASS_HIDDEN); diff --git a/src/lib/viewers/text/__tests__/CSVViewer-test.html b/src/lib/viewers/text/__tests__/CSVViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/text/__tests__/CSVViewer-test.html +++ b/src/lib/viewers/text/__tests__/CSVViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/text/__tests__/MarkdownViewer-test.html b/src/lib/viewers/text/__tests__/MarkdownViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/text/__tests__/MarkdownViewer-test.html +++ b/src/lib/viewers/text/__tests__/MarkdownViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
diff --git a/src/lib/viewers/text/__tests__/PlainTextViewer-test.html b/src/lib/viewers/text/__tests__/PlainTextViewer-test.html index 7dd9073c1..58be94f4d 100644 --- a/src/lib/viewers/text/__tests__/PlainTextViewer-test.html +++ b/src/lib/viewers/text/__tests__/PlainTextViewer-test.html @@ -1 +1,7 @@ -
+
+
+
+
+
+
+
From 3586c89cedfa184f181f14035f0acaed17fbb64b Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 11 Dec 2018 15:32:13 -0800 Subject: [PATCH 03/29] New: Expose thumbnails sidebar as preview option (#879) --- UPGRADING.md | 36 ++++++++ src/lib/Preview.js | 3 + src/lib/__tests__/Preview-test.js | 8 +- src/lib/viewers/doc/DocBaseViewer.js | 23 +++-- .../doc/__tests__/DocBaseViewer-test.js | 89 ++++++++++++++++++- 5 files changed, 146 insertions(+), 13 deletions(-) create mode 100644 UPGRADING.md diff --git a/UPGRADING.md b/UPGRADING.md new file mode 100644 index 000000000..f709ac161 --- /dev/null +++ b/UPGRADING.md @@ -0,0 +1,36 @@ +Upgrading Guide +========================= + +Upgrading from 1.x to 2.x +------------------------- +Version 2 includes a breaking change to the DOM structure of the Preview element. + +In version 1, the `.bp-navigate` buttons were siblings with the `.bp` container div +``` +
+ ... +
+ + +``` +But in version 2, the buttons are now inside a new container div `.bp-content`. +``` +
+
+ + +
+
+``` + +`.bp-content` is also the new point in which the various viewers will be dynamically inserted as children, i.e. `.bp-doc`, `.bp-image`, etc... + +This change in structure is to account for the new thumbnails sidebar which will appear to the left of the viewer content. diff --git a/src/lib/Preview.js b/src/lib/Preview.js index c523cb852..aa37acde4 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -937,6 +937,9 @@ class Preview extends EventEmitter { // Options that are applicable to certain file ids this.options.fileOptions = options.fileOptions || {}; + // Option to enable use of thumbnails sidebar for document types + this.options.enableThumbnailsSidebar = !!options.enableThumbnailsSidebar; + // Prefix any user created loaders before our default ones this.loaders = (options.loaders || []).concat(loaderList); diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js index 0da224f5e..43f4b039e 100644 --- a/src/lib/__tests__/Preview-test.js +++ b/src/lib/__tests__/Preview-test.js @@ -1197,7 +1197,8 @@ describe('lib/Preview', () => { showAnnotations: true, fixDependencies: true, collection: stubs.collection, - loaders: stubs.loaders + loaders: stubs.loaders, + enableThumbnailsSidebar: true }; stubs.assign = sandbox.spy(Object, 'assign'); @@ -1310,6 +1311,11 @@ describe('lib/Preview', () => { expect(stubs.disableViewers).to.be.calledWith('Office'); expect(stubs.enableViewers).to.be.calledWith('text'); }); + + it('should set whether to enable thumbnails sidebar', () => { + preview.parseOptions(preview.previewOptions); + expect(preview.options.enableThumbnailsSidebar).to.be.true; + }); }); describe('createViewerOptions()', () => { diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 462195af3..b89e8fe6a 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -123,9 +123,11 @@ class DocBaseViewer extends BaseViewer { this.startPageNum = this.getStartPage(this.startAt); - this.thumbnailsSidebarEl = document.createElement('div'); - this.thumbnailsSidebarEl.className = `${CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER} ${CLASS_HIDDEN}`; - this.containerEl.parentNode.insertBefore(this.thumbnailsSidebarEl, this.containerEl); + if (this.options.enableThumbnailsSidebar) { + this.thumbnailsSidebarEl = document.createElement('div'); + this.thumbnailsSidebarEl.className = `${CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER} ${CLASS_HIDDEN}`; + this.containerEl.parentNode.insertBefore(this.thumbnailsSidebarEl, this.containerEl); + } } /** @@ -990,12 +992,15 @@ class DocBaseViewer extends BaseViewer { * @return {void} */ bindControlListeners() { - this.controls.add( - __('toggle_thumbnails'), - this.toggleThumbnails, - 'bp-toggle-thumbnails-icon', - ICON_THUMBNAILS_TOGGLE - ); + if (this.options.enableThumbnailsSidebar) { + this.controls.add( + __('toggle_thumbnails'), + this.toggleThumbnails, + 'bp-toggle-thumbnails-icon', + ICON_THUMBNAILS_TOGGLE + ); + } + this.controls.add(__('zoom_out'), this.zoomOut, 'bp-doc-zoom-out-icon', ICON_ZOOM_OUT); this.controls.add(__('zoom_in'), this.zoomIn, 'bp-doc-zoom-in-icon', ICON_ZOOM_IN); diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index dc7e032ac..5191236aa 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -20,7 +20,8 @@ import { QUERY_PARAM_ENCODING, ENCODING_TYPES, SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER, - SELECTOR_BOX_PREVIEW_CONTENT + SELECTOR_BOX_PREVIEW_CONTENT, + CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER } from '../../../constants'; import { ICON_PRINT_CHECKMARK, @@ -87,7 +88,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { file: { id: '0', extension: 'ppt' - } + }, + enableThumbnailsSidebar: true }); Object.defineProperty(BaseViewer.prototype, 'setup', { value: sandbox.stub() }); docBase.containerEl = containerEl; @@ -110,15 +112,44 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }); describe('setup()', () => { - it('should correctly set a doc element, viewer element, and a timeout', () => { + it('should correctly set a doc element, viewer element, thumbnails sidebar element, and a timeout', () => { expect(docBase.docEl.classList.contains('bp-doc')).to.be.true; expect(docBase.docEl.parentNode).to.deep.equal(docBase.containerEl); expect(docBase.viewerEl.classList.contains('pdfViewer')).to.be.true; expect(docBase.viewerEl.parentNode).to.equal(docBase.docEl); + expect(docBase.thumbnailsSidebarEl.classList.contains(CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER)).to.be.true; + expect(docBase.thumbnailsSidebarEl.parentNode).to.equal(docBase.containerEl.parentNode); + expect(docBase.loadTimeout).to.equal(LOAD_TIMEOUT_MS); }); + + it('should not set a thumbnails sidebar element if the option is not enabled', () => { + docBase = new DocBaseViewer({ + cache: { + set: () => {}, + has: () => {}, + get: () => {}, + unset: () => {} + }, + container: containerEl, + representation: { + content: { + url_template: 'foo' + } + }, + file: { + id: '0', + extension: 'ppt' + }, + enableThumbnailsSidebar: false + }); + docBase.containerEl = containerEl; + docBase.setup(); + + expect(docBase.thumbnailsSidebarEl).to.be.undefined; + }); }); describe('destroy()', () => { @@ -2080,6 +2111,58 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { ICON_FULLSCREEN_OUT ); }); + + it('should not add the toggle thumbnails control if the option is not enabled', () => { + // Create a new instance that has enableThumbnailsSidebar as false + docBase = new DocBaseViewer({ + cache: { + set: () => {}, + has: () => {}, + get: () => {}, + unset: () => {} + }, + container: containerEl, + representation: { + content: { + url_template: 'foo' + } + }, + file: { + id: '0', + extension: 'ppt' + }, + enableThumbnailsSidebar: false + }); + docBase.containerEl = containerEl; + docBase.setup(); + + docBase.controls = { + add: sandbox.stub(), + removeListener: sandbox.stub() + }; + + docBase.pageControls = { + add: sandbox.stub(), + removeListener: sandbox.stub() + }; + + docBase.pdfViewer = { + pagesCount: 4, + currentPageNumber: 1, + cleanup: sandbox.stub() + }; + + // Invoke the method to test + docBase.bindControlListeners(); + + // Check expectations + expect(docBase.controls.add).to.not.be.calledWith( + __('toggle_thumbnails'), + docBase.toggleThumbnails, + 'bp-toggle-thumbnails-icon', + ICON_THUMBNAILS_TOGGLE + ); + }); }); describe('toggleThumbnails()', () => { From 631c5544cb71a1d7c296effe31dbb3eba57bfa2e Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 14 Dec 2018 14:09:39 -0800 Subject: [PATCH 04/29] New: Virtual scroll for the thumbnails sidebar (#880) --- src/lib/Preview.js | 1 + src/lib/Preview.scss | 1 + src/lib/VirtualScroller.js | 239 ++++++++++++++++++ src/lib/VirtualScroller.scss | 18 ++ src/lib/viewers/doc/DocBaseViewer.js | 36 +++ .../doc/__tests__/DocBaseViewer-test.js | 44 ++++ src/lib/viewers/doc/_docBase.scss | 12 + 7 files changed, 351 insertions(+) create mode 100644 src/lib/VirtualScroller.js create mode 100644 src/lib/VirtualScroller.scss diff --git a/src/lib/Preview.js b/src/lib/Preview.js index aa37acde4..59f7bda3b 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -938,6 +938,7 @@ class Preview extends EventEmitter { this.options.fileOptions = options.fileOptions || {}; // Option to enable use of thumbnails sidebar for document types + // this.options.enableThumbnailsSidebar = !!options.enableThumbnailsSidebar; this.options.enableThumbnailsSidebar = !!options.enableThumbnailsSidebar; // Prefix any user created loaders before our default ones diff --git a/src/lib/Preview.scss b/src/lib/Preview.scss index 8812952b6..6df3fe019 100644 --- a/src/lib/Preview.scss +++ b/src/lib/Preview.scss @@ -3,3 +3,4 @@ @import 'navigation'; @import './Controls'; @import './ProgressBar'; +@import './VirtualScroller'; diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js new file mode 100644 index 000000000..27dcbc130 --- /dev/null +++ b/src/lib/VirtualScroller.js @@ -0,0 +1,239 @@ +import isFinite from 'lodash/isFinite'; +import isFunction from 'lodash/isFunction'; +import throttle from 'lodash/throttle'; + +const BUFFERED_ITEM_MULTIPLIER = 3; + +class VirtualScroller { + /** @property {HTMLElement} - The anchor element for this Virtual Scroller */ + anchorEl; + + /** @property {HTMLElement} - The reference to the scrolling element container */ + scrollingEl; + + /** @property {number} - The height of the scrolling container */ + containerHeight; + + /** @property {number} - The height of a single list item */ + itemHeight; + + /** @property {HTMLElement} - The reference to the list element */ + listEl; + + /** @property {number} - The margin at the top of the list and below every list item */ + margin; + + /** @property {number} - The height of the buffer before virtual scroll renders the next set */ + maxBufferHeight; + + /** @property {number} - The max number of items to render at any one given time */ + maxRenderedItems; + + /** @property {Function} - The callback function that to allow users generate the item */ + renderItemFn; + + /** @property {number} - The total number items to be scrolled */ + totalItems; + + /** @property {number} - The number of items that can fit in the visible scrolling element */ + totalViewItems; + + /** + * [constructor] + * + * @param {HTMLElement} anchor - The HTMLElement that will anchor the virtual scroller + * @return {VirtualScroller} Instance of VirtualScroller + */ + constructor(anchor) { + this.anchorEl = anchor; + + this.previousScrollTop = 0; + + this.createListElement = this.createListElement.bind(this); + this.onScrollHandler = this.onScrollHandler.bind(this); + this.throttledOnScrollHandler = throttle(this.onScrollHandler, 50); + this.renderItems = this.renderItems.bind(this); + } + + /** + * Destroys the virtual scroller + * + * @return {void} + */ + destroy() { + if (this.scrollingEl) { + this.scrollingEl.remove(); + } + + this.scrollingEl = null; + this.listEl = null; + } + + /** + * Initializes the virtual scroller + * + * @param {Object} config - The config + * @return {void} + */ + init(config) { + this.validateRequiredConfig(config); + + this.totalItems = config.totalItems; + this.itemHeight = config.itemHeight; + this.containerHeight = config.containerHeight; + this.renderItemFn = config.renderItemFn; + this.margin = config.margin || 0; + this.totalViewItems = Math.floor(this.containerHeight / (this.itemHeight + this.margin)); + this.maxBufferHeight = this.totalViewItems * this.itemHeight; + this.maxRenderedItems = (this.totalViewItems + 1) * BUFFERED_ITEM_MULTIPLIER; + + // Create the scrolling container element + this.scrollingEl = document.createElement('div'); + this.scrollingEl.className = 'bp-vs'; + + // Create the true height content container + this.listEl = this.createListElement(); + + this.scrollingEl.appendChild(this.listEl); + this.anchorEl.appendChild(this.scrollingEl); + + this.renderItems(); + + this.bindDOMListeners(); + } + + /** + * Utility function to validate the required config is present + * + * @param {Object} config - the config object + * @return {void} + * @throws Error + */ + validateRequiredConfig(config) { + if (!config.totalItems || !isFinite(config.totalItems)) { + throw new Error('totalItems is required'); + } + + if (!config.itemHeight || !isFinite(config.itemHeight)) { + throw new Error('itemHeight is required'); + } + + if (!config.renderItemFn || !isFunction(config.renderItemFn)) { + throw new Error('renderItemFn is required'); + } + + if (!config.containerHeight || !isFinite(config.containerHeight)) { + throw new Error('containerHeight is required'); + } + } + + /** + * Binds DOM listeners + * + * @return {void} + */ + bindDOMListeners() { + this.scrollingEl.addEventListener('scroll', this.throttledOnScrollHandler, { passive: true }); + } + + /** + * Unbinds DOM listeners + * + * @return {void} + */ + unbindDOMListeners() { + if (this.scrollingEl) { + this.scrollingEl.removeEventListener('scroll', this.throttledOnScrollHandler); + } + } + + /** + * Handler for 'scroll' event + * + * @param {Event} e - The scroll event + * @return {void} + */ + onScrollHandler(e) { + const { scrollTop } = e.target; + + if (Math.abs(scrollTop - this.previousScrollTop) > this.maxBufferHeight) { + // The first item to be re-rendered will be a totalViewItems height up from the + // item at the current location + const firstIndex = Math.floor(scrollTop / (this.itemHeight + this.margin)) - this.totalViewItems; + this.renderItems(Math.max(firstIndex, 0)); + + this.previousScrollTop = scrollTop; + } + } + + /** + * Render a set of items, starting from the offset index + * + * @param {number} offset - The offset to start rendering items + * @return {void} + */ + renderItems(offset = 0) { + let count = this.maxRenderedItems; + // If the default count of items to render exceeds the totalItems count + // then just render the difference + if (count + offset > this.totalItems) { + count = this.totalItems - offset; + } + + let numItemsRendered = 0; + + // Create a new list element to be swapped out for the existing one + const newListEl = this.createListElement(); + while (numItemsRendered < count) { + const rowEl = this.renderItem(offset + numItemsRendered); + newListEl.appendChild(rowEl); + numItemsRendered += 1; + } + + this.scrollingEl.replaceChild(newListEl, this.listEl); + this.listEl = newListEl; + } + + /** + * Render a single item + * + * @param {number} rowIndex - The index of the item to be rendered + * @return {HTMLElement} The newly created row item + */ + renderItem(rowIndex) { + const rowEl = document.createElement('li'); + const topPosition = (this.itemHeight + this.margin) * rowIndex + this.margin; + + let renderedThumbnail; + try { + renderedThumbnail = this.renderItemFn.call(this, rowIndex); + } catch (err) { + // eslint-disable-next-line + console.error(`Error rendering thumbnail - ${err}`); + } + + rowEl.style.top = `${topPosition}px`; + rowEl.style.height = `${this.itemHeight}px`; + rowEl.classList.add('bp-vs-list-item'); + + if (renderedThumbnail) { + rowEl.appendChild(renderedThumbnail); + } + + return rowEl; + } + + /** + * Utility to create the list element + * + * @return {HTMLElement} The list element + */ + createListElement() { + const newListEl = document.createElement('ol'); + newListEl.className = 'bp-vs-list'; + newListEl.style.height = `${this.totalItems * (this.itemHeight + this.margin) + this.margin}px`; + return newListEl; + } +} + +export default VirtualScroller; diff --git a/src/lib/VirtualScroller.scss b/src/lib/VirtualScroller.scss new file mode 100644 index 000000000..ca31b1ddf --- /dev/null +++ b/src/lib/VirtualScroller.scss @@ -0,0 +1,18 @@ +.bp-vs { + flex: 1 0 auto; + overflow-y: auto; + + .bp-vs-list { + margin: 0; + padding: 0; + position: relative; + } + + .bp-vs-list-item { + box-sizing: border-box; + display: flex; + left: 0; + position: absolute; + right: 0; + } +} diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index b89e8fe6a..742ff748a 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -7,6 +7,7 @@ import DocFindBar from './DocFindBar'; import Popup from '../../Popup'; import RepStatus from '../../RepStatus'; import PreviewError from '../../PreviewError'; +import VirtualScroller from '../../VirtualScroller'; import { CLASS_BOX_PREVIEW_FIND_BAR, CLASS_CRAWLER, @@ -62,6 +63,7 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536 const PINCH_PAGE_CLASS = 'pinch-page'; const PINCHING_CLASS = 'pinching'; const PAGES_UNIT_NAME = 'pages'; +const DEFAULT_THUMBNAILS_SIDEBAR_WIDTH = 150; class DocBaseViewer extends BaseViewer { //-------------------------------------------------------------------------- @@ -181,6 +183,10 @@ class DocBaseViewer extends BaseViewer { this.printPopup.destroy(); } + if (this.thumbnailsSidebar) { + this.thumbnailsSidebar.destroy(); + } + super.destroy(); } @@ -1048,6 +1054,36 @@ class DocBaseViewer extends BaseViewer { // Add page IDs to each page after page structure is available this.setupPageIds(); } + + if (this.options.enableThumbnailsSidebar) { + this.initThumbnails(); + } + } + + initThumbnails() { + this.thumbnailsSidebar = new VirtualScroller(this.thumbnailsSidebarEl); + + // Get the first page of the document, and use its dimensions + // to set the thumbnails size of the thumbnails sidebar + this.pdfViewer.pdfDocument.getPage(1).then((page) => { + const desiredWidth = DEFAULT_THUMBNAILS_SIDEBAR_WIDTH; + const viewport = page.getViewport(1); + const scale = desiredWidth / viewport.width; + const scaledViewport = page.getViewport(scale); + + this.thumbnailsSidebar.init({ + totalItems: this.pdfViewer.pagesCount, + itemHeight: scaledViewport.height, + containerHeight: this.docEl.clientHeight, + margin: 15, + renderItemFn: (itemIndex) => { + const thumbnail = document.createElement('button'); + thumbnail.className = 'bp-thumbnail'; + thumbnail.textContent = `${itemIndex}`; + return thumbnail; + } + }); + }); } /** diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 5191236aa..2746b5b06 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -1589,6 +1589,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { stubs.getCachedPage = sandbox.stub(docBase, 'getCachedPage'); stubs.emit = sandbox.stub(docBase, 'emit'); stubs.setupPages = sandbox.stub(docBase, 'setupPageIds'); + stubs.initThumbnails = sandbox.stub(docBase, 'initThumbnails'); }); it('should load UI, check the pagination buttons, set the page, and make document scrollable', () => { @@ -1601,6 +1602,49 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(stubs.setPage).to.be.called; expect(docBase.docEl).to.have.class('bp-is-scrollable'); expect(stubs.setupPages).to.be.called; + expect(stubs.initThumbnails).to.be.called; + }); + + it('should not init thumbnails if not enabled', () => { + docBase = new DocBaseViewer({ + cache: { + set: () => {}, + has: () => {}, + get: () => {}, + unset: () => {} + }, + container: containerEl, + representation: { + content: { + url_template: 'foo' + } + }, + file: { + id: '0', + extension: 'ppt' + }, + enableThumbnailsSidebar: false + }); + Object.defineProperty(BaseViewer.prototype, 'setup', { value: sandbox.stub() }); + docBase.containerEl = containerEl; + docBase.setup(); + stubs.loadUI = sandbox.stub(docBase, 'loadUI'); + stubs.setPage = sandbox.stub(docBase, 'setPage'); + stubs.getCachedPage = sandbox.stub(docBase, 'getCachedPage'); + stubs.emit = sandbox.stub(docBase, 'emit'); + stubs.setupPages = sandbox.stub(docBase, 'setupPageIds'); + stubs.initThumbnails = sandbox.stub(docBase, 'initThumbnails'); + + docBase.pdfViewer = { + currentScale: 'unknown' + }; + + docBase.pagesinitHandler(); + expect(stubs.loadUI).to.be.called; + expect(stubs.setPage).to.be.called; + expect(docBase.docEl).to.have.class('bp-is-scrollable'); + expect(stubs.setupPages).to.be.called; + expect(stubs.initThumbnails).not.to.be.called; }); it('should broadcast that the preview is loaded if it hasn\'t already', () => { diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index ba9e64577..fcffd6c2f 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -3,8 +3,20 @@ .bp { .bp-thumbnails-container { border-right: solid 1px $seesee; + display: flex; flex: 0 0 180px; } + + .bp-thumbnail { + align-items: center; + background-color: rgba(0, 0, 255, .5); + border-radius: 4px; + display: flex; + flex: 1 1 auto; + justify-content: center; + margin: 0 15px; + padding: 0; + } } .bp-theme-dark { From ca1dcebb38e6c24589235b402ee9599d61440e83 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 4 Jan 2019 14:12:10 -0800 Subject: [PATCH 05/29] Chore: adding unit tests for VirtualScroller (#882) --- src/lib/VirtualScroller.js | 4 +- src/lib/__tests__/VirtualScroller-test.html | 1 + src/lib/__tests__/VirtualScroller-test.js | 223 ++++++++++++++++++++ 3 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 src/lib/__tests__/VirtualScroller-test.html create mode 100644 src/lib/__tests__/VirtualScroller-test.js diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 27dcbc130..d99b36415 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -3,6 +3,7 @@ import isFunction from 'lodash/isFunction'; import throttle from 'lodash/throttle'; const BUFFERED_ITEM_MULTIPLIER = 3; +const SCROLL_THROTTLE_MS = 50; class VirtualScroller { /** @property {HTMLElement} - The anchor element for this Virtual Scroller */ @@ -50,8 +51,7 @@ class VirtualScroller { this.previousScrollTop = 0; this.createListElement = this.createListElement.bind(this); - this.onScrollHandler = this.onScrollHandler.bind(this); - this.throttledOnScrollHandler = throttle(this.onScrollHandler, 50); + this.onScrollHandler = throttle(this.onScrollHandler.bind(this), SCROLL_THROTTLE_MS); this.renderItems = this.renderItems.bind(this); } diff --git a/src/lib/__tests__/VirtualScroller-test.html b/src/lib/__tests__/VirtualScroller-test.html new file mode 100644 index 000000000..03a74a801 --- /dev/null +++ b/src/lib/__tests__/VirtualScroller-test.html @@ -0,0 +1 @@ +
diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js new file mode 100644 index 000000000..d5a6d8694 --- /dev/null +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -0,0 +1,223 @@ +/* eslint-disable no-unused-expressions */ +import VirtualScroller from '../VirtualScroller'; + +let virtualScroller; +let stubs = {}; + +const sandbox = sinon.sandbox.create(); + +describe('VirtualScroller', () => { + before(() => fixture.setBase('src/lib')); + + beforeEach(() => { + fixture.load('__tests__/VirtualScroller-test.html'); + virtualScroller = new VirtualScroller(document.getElementById('test-virtual-scroller')); + }); + + afterEach(() => { + fixture.cleanup(); + sandbox.verifyAndRestore(); + + if (virtualScroller && typeof virtualScroller.destroy === 'function') { + virtualScroller.destroy(); + } + + virtualScroller = null; + stubs = {}; + }); + + describe('constructor()', () => { + it('should initialize anchorEl and previousScrollTop', () => { + expect(virtualScroller.anchorEl.id).to.be.equal('test-virtual-scroller'); + expect(virtualScroller.previousScrollTop).to.be.equal(0); + }); + }); + + describe('destroy()', () => { + it('should remove the HTML element references', () => { + const scrollingEl = { remove: () => {} }; + sandbox.stub(scrollingEl, 'remove'); + + virtualScroller.scrollingEl = scrollingEl; + virtualScroller.listEl = {}; + + virtualScroller.destroy(); + + expect(scrollingEl.remove).to.be.called; + expect(virtualScroller.scrollingEl).to.be.null; + expect(virtualScroller.listEl).to.be.null; + }); + }); + + describe('init()', () => { + beforeEach(() => { + stubs.validateRequiredConfig = sandbox.stub(virtualScroller, 'validateRequiredConfig'); + }); + + it('should parse the config object', () => { + stubs.renderItemFn = sandbox.stub(); + stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems'); + stubs.bindDOMListeners = sandbox.stub(virtualScroller, 'bindDOMListeners'); + + virtualScroller.init({ + totalItems: 10, + itemHeight: 100, + containerHeight: 500, + renderItemFn: stubs.renderItemFn + }); + + expect(virtualScroller.totalItems).to.be.equal(10); + expect(virtualScroller.itemHeight).to.be.equal(100); + expect(virtualScroller.containerHeight).to.be.equal(500); + expect(virtualScroller.renderItemFn).to.be.equal(stubs.renderItemFn); + expect(virtualScroller.margin).to.be.equal(0); + expect(virtualScroller.totalViewItems).to.be.equal(5); + expect(virtualScroller.maxBufferHeight).to.be.equal(500); + expect(virtualScroller.maxRenderedItems).to.be.equal(18); + + expect(virtualScroller.scrollingEl.classList.contains('bp-vs')).to.be.true; + expect(virtualScroller.listEl.classList.contains('bp-vs-list')).to.be.true; + + expect(stubs.renderItems).to.be.called; + expect(stubs.bindDOMListeners).to.be.called; + }); + }); + + describe('validateRequiredConfig()', () => { + it('should not throw an error if config is good', () => { + expect(() => + virtualScroller.validateRequiredConfig({ + totalItems: 10, + itemHeight: 100, + renderItemFn: () => {}, + containerHeight: 500 + }) + ).to.not.throw(); + }); + + [ + { name: 'totalItems falsy', config: {} }, + { name: 'totalItems not finite', config: { totalItems: '10' } }, + { name: 'itemHeight falsy', config: { totalItems: 10 } }, + { name: 'itemHeight not finite', config: { totalItems: 10, itemHeight: '100' } }, + { name: 'renderItemFn falsy', config: { totalItems: 10, itemHeight: 100 } }, + { name: 'renderItemFn not a function', config: { totalItems: 10, itemHeight: 100, renderItemFn: 'hi' } }, + { name: 'containerHeight falsy', config: { totalItems: 10, itemHeight: 100, renderItemFn: () => {} } }, + { + name: 'containerHeight not finite', + config: { totalItems: 10, itemHeight: 100, renderItemFn: () => {}, containerHeight: '500' } + } + ].forEach((data) => { + it(`should throw an error if config is bad: ${data.name}`, () => { + expect(() => virtualScroller.validateRequiredConfig(data.config)).to.throw(); + }); + }); + }); + + describe('onScrollHandler()', () => { + beforeEach(() => { + stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems'); + virtualScroller.maxBufferHeight = 100; + }); + + it('should not proceed if the scroll movement < maxBufferHeight', () => { + virtualScroller.previousScrollTop = 0; + virtualScroller.onScrollHandler({ target: { scrollTop: 10 } }); + + expect(stubs.renderItems).to.not.be.called; + }); + + it('should proceed if positive scroll movement > maxBufferHeight', () => { + virtualScroller.previousScrollTop = 0; + virtualScroller.onScrollHandler({ target: { scrollTop: 101 } }); + + expect(stubs.renderItems).to.be.called; + expect(virtualScroller.previousScrollTop).to.be.equal(101); + }); + + it('should proceed if negative scroll movement > maxBufferHeight', () => { + virtualScroller.previousScrollTop = 102; + virtualScroller.onScrollHandler({ target: { scrollTop: 1 } }); + + expect(stubs.renderItems).to.be.called; + expect(virtualScroller.previousScrollTop).to.be.equal(1); + }); + }); + + describe('renderItems()', () => { + let newListEl; + + beforeEach(() => { + virtualScroller.scrollingEl = { replaceChild: () => {} }; + newListEl = { appendChild: () => {} }; + stubs.createListElement = sandbox.stub(virtualScroller, 'createListElement').returns(newListEl); + stubs.renderItem = sandbox.stub(virtualScroller, 'renderItem'); + stubs.replaceChild = sandbox.stub(virtualScroller.scrollingEl, 'replaceChild'); + stubs.appendChild = sandbox.stub(newListEl, 'appendChild'); + }); + + afterEach(() => { + virtualScroller.scrollingEl = null; + }); + + it('should render maxRenderedItems', () => { + virtualScroller.maxRenderedItems = 10; + virtualScroller.totalItems = 100; + virtualScroller.renderItems(); + + expect(newListEl.appendChild.callCount).to.be.equal(10); + expect(virtualScroller.scrollingEl.replaceChild).to.be.called; + }); + + it('should render the remaining items up to totalItems', () => { + virtualScroller.maxRenderedItems = 10; + virtualScroller.totalItems = 100; + virtualScroller.renderItems(95); + + expect(newListEl.appendChild.callCount).to.be.equal(5); + expect(virtualScroller.scrollingEl.replaceChild).to.be.called; + }); + }); + + describe('renderItem()', () => { + it('should render an item absolutely positioned with arbitrary content', () => { + const renderedThumbnail = document.createElement('div'); + renderedThumbnail.className = 'rendered-thumbnail'; + stubs.renderItemFn = sandbox.stub().returns(renderedThumbnail); + + virtualScroller.itemHeight = 100; + virtualScroller.margin = 0; + virtualScroller.renderItemFn = stubs.renderItemFn; + + const item = virtualScroller.renderItem(0); + expect(stubs.renderItemFn).to.be.called; + expect(item.classList.contains('bp-vs-list-item')).to.be.true; + expect(item.firstChild.classList.contains('rendered-thumbnail')).to.be.true; + }); + + it('should still render the item even if renderItemFn throws an error', () => { + const renderedThumbnail = document.createElement('div'); + renderedThumbnail.className = 'rendered-thumbnail'; + stubs.renderItemFn = sandbox.stub().throws(); + + virtualScroller.itemHeight = 100; + virtualScroller.margin = 0; + virtualScroller.renderItemFn = stubs.renderItemFn; + + const item = virtualScroller.renderItem(0); + expect(stubs.renderItemFn).to.be.called; + expect(item.classList.contains('bp-vs-list-item')).to.be.true; + expect(item.firstChild).to.be.null; + }); + }); + + describe('createListElement()', () => { + it('should return the list element', () => { + virtualScroller.totalItems = 10; + virtualScroller.itemHeight = 100; + virtualScroller.margin = 0; + + expect(virtualScroller.createListElement().classList.contains('bp-vs-list')).to.be.true; + }); + }); +}); From 5be7787c46079e83f4ea15ba08b212833cb95c27 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 9 Jan 2019 16:37:37 -0800 Subject: [PATCH 06/29] Update: Thumbnails sidebar images (#887) Now cuts thumbnail images to insert into sidebar. Current approach as to *when* to render thumbnail images is to introduce a `onScrollEnd` callback from the `VirtualScroller` so that when scrolling ends, we look at what list items are rendered and then ask PDFJS to generate the thumbnail image. The `VirtualScroller` `renderItemFn` is invoked by a throttled scroll event handler which is used to create a placeholder thumbnail which includes the page number indicator --- src/lib/Preview.js | 1 - src/lib/ThumbnailsSidebar.js | 186 ++++++++++++++++++++++ src/lib/VirtualScroller.js | 151 ++++++++++++++++-- src/lib/__tests__/VirtualScroller-test.js | 21 ++- src/lib/viewers/doc/DocBaseViewer.js | 28 +--- src/lib/viewers/doc/_docBase.scss | 17 +- 6 files changed, 362 insertions(+), 42 deletions(-) create mode 100644 src/lib/ThumbnailsSidebar.js diff --git a/src/lib/Preview.js b/src/lib/Preview.js index 59f7bda3b..aa37acde4 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -938,7 +938,6 @@ class Preview extends EventEmitter { this.options.fileOptions = options.fileOptions || {}; // Option to enable use of thumbnails sidebar for document types - // this.options.enableThumbnailsSidebar = !!options.enableThumbnailsSidebar; this.options.enableThumbnailsSidebar = !!options.enableThumbnailsSidebar; // Prefix any user created loaders before our default ones diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js new file mode 100644 index 000000000..7be6c956f --- /dev/null +++ b/src/lib/ThumbnailsSidebar.js @@ -0,0 +1,186 @@ +import isFinite from 'lodash/isFinite'; +import VirtualScroller from './VirtualScroller'; + +const DEFAULT_THUMBNAILS_SIDEBAR_WIDTH = 150; +const THUMBNAIL_WIDTH_MAX = 210; +const THUMBNAIL_MARGIN = 15; + +class ThumbnailsSidebar { + /** @property {HTMLElement} - The anchor element for this ThumbnailsSidebar */ + anchorEl; + + /** @property {PDfViewer} - The PDFJS viewer instance */ + pdfViewer; + + /** @property {Object} - Cache for the thumbnail image elements */ + thumbnailImageCache; + + /** + * [constructor] + * + * @param {HTMLElement} element - the HTMLElement that will anchor the thumbnail sidebar + * @param {PDFViewer} pdfViewer - the PDFJS viewer + */ + constructor(element, pdfViewer) { + this.anchorEl = element; + this.pdfViewer = pdfViewer; + this.thumbnailImageCache = {}; + + this.createPlaceholderThumbnail = this.createPlaceholderThumbnail.bind(this); + this.requestThumbnailImage = this.requestThumbnailImage.bind(this); + this.createThumbnailImage = this.createThumbnailImage.bind(this); + this.generateThumbnailImages = this.generateThumbnailImages.bind(this); + } + + /** + * Destroys the thumbnails sidebar + * + * @return {void} + */ + destroy() { + if (this.virtualScroller) { + this.virtualScroller.destroy(); + this.virtualScroller = null; + } + + this.thumbnailImageCache = null; + this.pdfViewer = null; + } + + /** + * Initializes the Thumbnails Sidebar + * + * @return {void} + */ + init() { + this.virtualScroller = new VirtualScroller(this.anchorEl); + + // Get the first page of the document, and use its dimensions + // to set the thumbnails size of the thumbnails sidebar + this.pdfViewer.pdfDocument.getPage(1).then((page) => { + const { width, height } = page.getViewport(1); + + // If the dimensions of the page are invalid then don't proceed further + if (!(isFinite(width) && width > 0 && isFinite(height) && height > 0)) { + console.error('Page dimensions invalid when initializing the thumbnails sidebar'); + return; + } + + // Amount to scale down from fullsize to thumbnail size + this.scale = DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / width; + // Width : Height ratio of the page + this.pageRatio = width / height; + const scaledViewport = page.getViewport(this.scale); + + this.virtualScroller.init({ + totalItems: this.pdfViewer.pagesCount, + itemHeight: scaledViewport.height, + containerHeight: this.anchorEl.parentNode.clientHeight, + margin: THUMBNAIL_MARGIN, + renderItemFn: this.createPlaceholderThumbnail, + onScrollEnd: this.generateThumbnailImages, + onInit: this.generateThumbnailImages + }); + }); + } + + /** + * Generates the thumbnail images that are not yet created + * + * @param {Object} currentListInfo - VirtualScroller info object which contains startOffset, endOffset, and the thumbnail elements + * @return {void} + */ + generateThumbnailImages({ items, startOffset }) { + items.forEach((thumbnailEl, index) => { + if (thumbnailEl.classList.contains('bp-thumbnail-image-loaded')) { + return; + } + + this.requestThumbnailImage(index + startOffset, thumbnailEl); + }); + } + + /** + * Creates the placeholder thumbnail with page indication. This element will + * not yet have the image of the page + * + * @param {number} itemIndex - The item index into the overall list (0 indexed) + * @return {HTMLElement} - thumbnail button element + */ + createPlaceholderThumbnail(itemIndex) { + const thumbnailEl = document.createElement('button'); + thumbnailEl.className = 'bp-thumbnail'; + thumbnailEl.setAttribute('type', 'button'); + thumbnailEl.appendChild(this.createPageNumber(itemIndex + 1)); + return thumbnailEl; + } + + /** + * Request the thumbnail image to be made + * + * @param {number} itemIndex - the item index in the overall list (0 indexed) + * @param {HTMLElement} thumbnailEl - the thumbnail button element + * @return {void} + */ + requestThumbnailImage(itemIndex, thumbnailEl) { + requestAnimationFrame(() => { + this.createThumbnailImage(itemIndex).then((imageEl) => { + thumbnailEl.appendChild(imageEl); + thumbnailEl.classList.add('bp-thumbnail-image-loaded'); + }); + }); + } + + /** + * Make a thumbnail image element + * + * @param {number} itemIndex - the item index for the overall list (0 indexed) + * @return {Promise} - promise reolves with the image HTMLElement + */ + createThumbnailImage(itemIndex) { + // If this page has already been cached, use it + if (this.thumbnailImageCache[itemIndex]) { + return Promise.resolve(this.thumbnailImageCache[itemIndex]); + } + + const canvas = document.createElement('canvas'); + + return this.pdfViewer.pdfDocument + .getPage(itemIndex + 1) + .then((page) => { + const viewport = page.getViewport(1); + canvas.width = THUMBNAIL_WIDTH_MAX; + canvas.height = THUMBNAIL_WIDTH_MAX / this.pageRatio; + const scale = THUMBNAIL_WIDTH_MAX / viewport.width; + return page.render({ + canvasContext: canvas.getContext('2d'), + viewport: page.getViewport(scale) + }); + }) + .then(() => { + const imageEl = document.createElement('img'); + imageEl.src = canvas.toDataURL(); + imageEl.style.maxWidth = '100%'; + + // Cache this image element for future use + this.thumbnailImageCache[itemIndex] = imageEl; + + return imageEl; + }); + } + + /** + * Creates a page number element + * + * @param {number} pageNumber - Page number of the document + * @return {HTMLElement} - A div containing the page number + */ + createPageNumber(pageNumber) { + const pageNumberEl = document.createElement('div'); + pageNumberEl.className = 'bp-thumbnail-page-number'; + pageNumberEl.textContent = `${pageNumber}`; + return pageNumberEl; + } +} + +export default ThumbnailsSidebar; diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index d99b36415..3ab884151 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -1,9 +1,11 @@ import isFinite from 'lodash/isFinite'; import isFunction from 'lodash/isFunction'; import throttle from 'lodash/throttle'; +import debounce from 'lodash/debounce'; const BUFFERED_ITEM_MULTIPLIER = 3; -const SCROLL_THROTTLE_MS = 50; +const THROTTLE_SCROLL_THRESHOLD = 150; +const DEBOUNCE_SCROLL_THRESHOLD = 151; class VirtualScroller { /** @property {HTMLElement} - The anchor element for this Virtual Scroller */ @@ -30,6 +32,9 @@ class VirtualScroller { /** @property {number} - The max number of items to render at any one given time */ maxRenderedItems; + /** @property {number} - The previously recorded scrollTop value */ + previousScrollTop; + /** @property {Function} - The callback function that to allow users generate the item */ renderItemFn; @@ -51,8 +56,13 @@ class VirtualScroller { this.previousScrollTop = 0; this.createListElement = this.createListElement.bind(this); - this.onScrollHandler = throttle(this.onScrollHandler.bind(this), SCROLL_THROTTLE_MS); + this.onScrollEndHandler = this.onScrollEndHandler.bind(this); + this.onScrollHandler = this.onScrollHandler.bind(this); + this.getCurrentListInfo = this.getCurrentListInfo.bind(this); this.renderItems = this.renderItems.bind(this); + + this.debouncedOnScrollEndHandler = debounce(this.onScrollEndHandler, DEBOUNCE_SCROLL_THRESHOLD); + this.throttledOnScrollHandler = throttle(this.onScrollHandler, THROTTLE_SCROLL_THRESHOLD); } /** @@ -83,6 +93,9 @@ class VirtualScroller { this.containerHeight = config.containerHeight; this.renderItemFn = config.renderItemFn; this.margin = config.margin || 0; + this.onScrollEnd = config.onScrollEnd; + this.onScrollStart = config.onScrollStart; + this.totalViewItems = Math.floor(this.containerHeight / (this.itemHeight + this.margin)); this.maxBufferHeight = this.totalViewItems * this.itemHeight; this.maxRenderedItems = (this.totalViewItems + 1) * BUFFERED_ITEM_MULTIPLIER; @@ -100,6 +113,11 @@ class VirtualScroller { this.renderItems(); this.bindDOMListeners(); + + if (config.onInit) { + const listInfo = this.getCurrentListInfo(); + config.onInit(listInfo); + } } /** @@ -134,6 +152,7 @@ class VirtualScroller { */ bindDOMListeners() { this.scrollingEl.addEventListener('scroll', this.throttledOnScrollHandler, { passive: true }); + this.scrollingEl.addEventListener('scroll', this.debouncedOnScrollEndHandler, { passive: true }); } /** @@ -144,6 +163,7 @@ class VirtualScroller { unbindDOMListeners() { if (this.scrollingEl) { this.scrollingEl.removeEventListener('scroll', this.throttledOnScrollHandler); + this.scrollingEl.removeEventListener('scroll', this.debouncedOnScrollEndHandler); } } @@ -166,6 +186,38 @@ class VirtualScroller { } } + /** + * Debounced scroll handler to signal when scrolling has stopped + * + * @return {void} + */ + onScrollEndHandler() { + if (this.onScrollEnd) { + const listInfo = this.getCurrentListInfo(); + this.onScrollEnd(listInfo); + } + } + + /** + * Gets information about what the current start offset, end offset and rendered items array + * + * @return {Object} - info object + */ + getCurrentListInfo() { + const { firstElementChild, lastElementChild } = this.listEl; + + const curStartOffset = firstElementChild ? Number.parseInt(firstElementChild.dataset.bpVsRowIndex, 10) : -1; + const curEndOffset = lastElementChild ? Number.parseInt(lastElementChild.dataset.bpVsRowIndex, 10) : -1; + + const items = Array.prototype.slice.call(this.listEl.children).map((listItemEl) => listItemEl.children[0]); + + return { + startOffset: curStartOffset, + endOffset: curEndOffset, + items + }; + } + /** * Render a set of items, starting from the offset index * @@ -173,27 +225,101 @@ class VirtualScroller { * @return {void} */ renderItems(offset = 0) { - let count = this.maxRenderedItems; - // If the default count of items to render exceeds the totalItems count - // then just render the difference - if (count + offset > this.totalItems) { - count = this.totalItems - offset; + // calculate the diff between what is already rendered + // and what needs to be rendered + const { startOffset: curStartOffset, endOffset: curEndOffset } = this.getCurrentListInfo(); + + if (curStartOffset === offset) { + return; } - let numItemsRendered = 0; + let newStartOffset = offset; + let newEndOffset = offset + this.maxRenderedItems; + // If the default count of items to render exceeds the totalItems count + // then just render up to the end + if (newEndOffset >= this.totalItems) { + newEndOffset = this.totalItems - 1; + } // Create a new list element to be swapped out for the existing one const newListEl = this.createListElement(); - while (numItemsRendered < count) { - const rowEl = this.renderItem(offset + numItemsRendered); - newListEl.appendChild(rowEl); - numItemsRendered += 1; + + if (curStartOffset <= offset && offset <= curEndOffset) { + // Scenario #1: New start offset falls within the current range of items rendered + // |--------------------| + // curStartOffset curEndOffset + // |--------------------| + // newStartOffset newEndOffset + newStartOffset = curEndOffset + 1; + // clone elements from newStartOffset to curEndOffset + this.cloneItems(newListEl, this.listEl, offset - curStartOffset, curEndOffset - curStartOffset); + // then create elements from curEnd + 1 to newEndOffset + this.createItems(newListEl, newStartOffset, newEndOffset); + } else if (curStartOffset <= newEndOffset && newEndOffset <= curEndOffset) { + // Scenario #2: New end offset falls within the current range of items rendered + // |--------------------| + // curStartOffset curEndOffset + // |--------------------| + // newStartOffset newEndOffset + + // create elements from newStartOffset to curStart - 1 + this.createItems(newListEl, offset, curStartOffset - 1); + // then clone elements from curStartOffset to newEndOffset + this.cloneItems(newListEl, this.listEl, 0, newEndOffset - curStartOffset); + } else { + // Scenario #3: New range has no overlap with current range of items + // |--------------------| + // curStartOffset curEndOffset + // |--------------------| + // newStartOffset newEndOffset + this.createItems(newListEl, newStartOffset, newEndOffset); } this.scrollingEl.replaceChild(newListEl, this.listEl); this.listEl = newListEl; } + /** + * Clones a subset of the HTMLElements from the oldList to the newList. + * The newList element is modified. + * + * @param {HTMLElement} newListEl - the new `ol` element + * @param {HTMLElement} oldListEl - the old `ol` element + * @param {number} start - start index + * @param {number} end - end index + * @return {void} + */ + cloneItems(newListEl, oldListEl, start, end) { + if (!newListEl || !oldListEl || start < 0 || end < 0) { + return; + } + + const { children } = oldListEl; + + for (let i = start; i <= end; i++) { + newListEl.appendChild(children[i].cloneNode(true)); + } + } + + /** + * Creates new HTMLElements appended to the newList + * + * @param {HTMLElement} newList - the new `li` element + * @param {number} start - start index + * @param {number} end - end index + * @return {void} + */ + createItems(newList, start, end) { + if (!newList || start < 0 || end < 0) { + return; + } + + for (let i = start; i <= end; i++) { + const newEl = this.renderItem(i); + newList.appendChild(newEl); + } + } + /** * Render a single item * @@ -215,6 +341,7 @@ class VirtualScroller { rowEl.style.top = `${topPosition}px`; rowEl.style.height = `${this.itemHeight}px`; rowEl.classList.add('bp-vs-list-item'); + rowEl.dataset.bpVsRowIndex = rowIndex; if (renderedThumbnail) { rowEl.appendChild(renderedThumbnail); diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js index d5a6d8694..e3e82af4d 100644 --- a/src/lib/__tests__/VirtualScroller-test.js +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -146,35 +146,50 @@ describe('VirtualScroller', () => { describe('renderItems()', () => { let newListEl; + const curListEl = {}; beforeEach(() => { virtualScroller.scrollingEl = { replaceChild: () => {} }; + virtualScroller.listEl = curListEl; newListEl = { appendChild: () => {} }; stubs.createListElement = sandbox.stub(virtualScroller, 'createListElement').returns(newListEl); stubs.renderItem = sandbox.stub(virtualScroller, 'renderItem'); stubs.replaceChild = sandbox.stub(virtualScroller.scrollingEl, 'replaceChild'); stubs.appendChild = sandbox.stub(newListEl, 'appendChild'); + stubs.getCurrentListInfo = sandbox.stub(virtualScroller, 'getCurrentListInfo'); + stubs.cloneItems = sandbox.stub(virtualScroller, 'cloneItems'); + stubs.createItems = sandbox.stub(virtualScroller, 'createItems'); }); afterEach(() => { virtualScroller.scrollingEl = null; }); - it('should render maxRenderedItems', () => { + it('should render the whole range of items (no reuse)', () => { virtualScroller.maxRenderedItems = 10; virtualScroller.totalItems = 100; + stubs.getCurrentListInfo.returns({ + startOffset: -1, + endOffset: -1 + }); virtualScroller.renderItems(); - expect(newListEl.appendChild.callCount).to.be.equal(10); + expect(stubs.cloneItems).not.to.be.called; + expect(stubs.createItems).to.be.calledWith(newListEl, 0, 10); expect(virtualScroller.scrollingEl.replaceChild).to.be.called; }); it('should render the remaining items up to totalItems', () => { virtualScroller.maxRenderedItems = 10; virtualScroller.totalItems = 100; + stubs.getCurrentListInfo.returns({ + startOffset: -1, + endOffset: -1 + }); virtualScroller.renderItems(95); - expect(newListEl.appendChild.callCount).to.be.equal(5); + expect(stubs.cloneItems).not.to.be.called; + expect(stubs.createItems).to.be.calledWith(newListEl, 95, 99); expect(virtualScroller.scrollingEl.replaceChild).to.be.called; }); }); diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 742ff748a..2d5e2b306 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -7,7 +7,7 @@ import DocFindBar from './DocFindBar'; import Popup from '../../Popup'; import RepStatus from '../../RepStatus'; import PreviewError from '../../PreviewError'; -import VirtualScroller from '../../VirtualScroller'; +import ThumbnailsSidebar from '../../ThumbnailsSidebar'; import { CLASS_BOX_PREVIEW_FIND_BAR, CLASS_CRAWLER, @@ -63,7 +63,6 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536 const PINCH_PAGE_CLASS = 'pinch-page'; const PINCHING_CLASS = 'pinching'; const PAGES_UNIT_NAME = 'pages'; -const DEFAULT_THUMBNAILS_SIDEBAR_WIDTH = 150; class DocBaseViewer extends BaseViewer { //-------------------------------------------------------------------------- @@ -1061,29 +1060,8 @@ class DocBaseViewer extends BaseViewer { } initThumbnails() { - this.thumbnailsSidebar = new VirtualScroller(this.thumbnailsSidebarEl); - - // Get the first page of the document, and use its dimensions - // to set the thumbnails size of the thumbnails sidebar - this.pdfViewer.pdfDocument.getPage(1).then((page) => { - const desiredWidth = DEFAULT_THUMBNAILS_SIDEBAR_WIDTH; - const viewport = page.getViewport(1); - const scale = desiredWidth / viewport.width; - const scaledViewport = page.getViewport(scale); - - this.thumbnailsSidebar.init({ - totalItems: this.pdfViewer.pagesCount, - itemHeight: scaledViewport.height, - containerHeight: this.docEl.clientHeight, - margin: 15, - renderItemFn: (itemIndex) => { - const thumbnail = document.createElement('button'); - thumbnail.className = 'bp-thumbnail'; - thumbnail.textContent = `${itemIndex}`; - return thumbnail; - } - }); - }); + this.thumbnailsSidebar = new ThumbnailsSidebar(this.thumbnailsSidebarEl, this.pdfViewer); + this.thumbnailsSidebar.init(); } /** diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index fcffd6c2f..97d058578 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -9,13 +9,28 @@ .bp-thumbnail { align-items: center; - background-color: rgba(0, 0, 255, .5); + background-color: #d8d8d8; + border: 0; border-radius: 4px; display: flex; flex: 1 1 auto; justify-content: center; margin: 0 15px; + overflow: hidden; padding: 0; + position: relative; + } + + .bp-thumbnail-page-number { + background-color: $black; + border-radius: 4px; + bottom: 10px; + color: $white; + font-size: 11px; + line-height: 16px; + opacity: .7; + padding: 0 5px; + position: absolute; } } From 63061a70b101fddf0edf7a0b9cb5bd789c96f8eb Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 11 Jan 2019 13:11:05 -0800 Subject: [PATCH 07/29] Chore: unit tests for ThumbnailsSidebar and VirtualScroller (#888) --- src/lib/ThumbnailsSidebar.js | 11 + src/lib/VirtualScroller.js | 32 ++- src/lib/__tests__/ThumbnailsSidebar-test.html | 1 + src/lib/__tests__/ThumbnailsSidebar-test.js | 224 ++++++++++++++++++ src/lib/__tests__/VirtualScroller-test.js | 193 ++++++++++++++- 5 files changed, 451 insertions(+), 10 deletions(-) create mode 100644 src/lib/__tests__/ThumbnailsSidebar-test.html create mode 100644 src/lib/__tests__/ThumbnailsSidebar-test.js diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index 7be6c956f..f8061d1c7 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -9,9 +9,15 @@ class ThumbnailsSidebar { /** @property {HTMLElement} - The anchor element for this ThumbnailsSidebar */ anchorEl; + /** @property {number} - The width : height ratio of the pages of the document */ + pageRatio; + /** @property {PDfViewer} - The PDFJS viewer instance */ pdfViewer; + /** @property {number} - The percentage (0-1) to scale down from the full page to thumbnail size */ + scale; + /** @property {Object} - Cache for the thumbnail image elements */ thumbnailImageCache; @@ -62,6 +68,7 @@ class ThumbnailsSidebar { // If the dimensions of the page are invalid then don't proceed further if (!(isFinite(width) && width > 0 && isFinite(height) && height > 0)) { + // eslint-disable-next-line console.error('Page dimensions invalid when initializing the thumbnails sidebar'); return; } @@ -91,6 +98,10 @@ class ThumbnailsSidebar { * @return {void} */ generateThumbnailImages({ items, startOffset }) { + if (!isFinite(startOffset) || startOffset < 0) { + return; + } + items.forEach((thumbnailEl, index) => { if (thumbnailEl.classList.contains('bp-thumbnail-image-loaded')) { return; diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 3ab884151..f4d02d32c 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -204,12 +204,24 @@ class VirtualScroller { * @return {Object} - info object */ getCurrentListInfo() { - const { firstElementChild, lastElementChild } = this.listEl; + const { firstElementChild, lastElementChild, children } = this.listEl; - const curStartOffset = firstElementChild ? Number.parseInt(firstElementChild.dataset.bpVsRowIndex, 10) : -1; - const curEndOffset = lastElementChild ? Number.parseInt(lastElementChild.dataset.bpVsRowIndex, 10) : -1; + // Parse the row index from the data-attribute + let curStartOffset = firstElementChild ? Number.parseInt(firstElementChild.dataset.bpVsRowIndex, 10) : -1; + let curEndOffset = lastElementChild ? Number.parseInt(lastElementChild.dataset.bpVsRowIndex, 10) : -1; - const items = Array.prototype.slice.call(this.listEl.children).map((listItemEl) => listItemEl.children[0]); + // If the data-attribute value is not present default to invalid -1 + curStartOffset = isFinite(curStartOffset) ? curStartOffset : -1; + curEndOffset = isFinite(curEndOffset) ? curEndOffset : -1; + + let items = []; + + if (children) { + // Extract an array of the user's created HTMLElements + items = Array.prototype.slice + .call(children) + .map((listItemEl) => (listItemEl && listItemEl.children ? listItemEl.children[0] : null)); + } return { startOffset: curStartOffset, @@ -296,6 +308,10 @@ class VirtualScroller { const { children } = oldListEl; + if (!children || start >= children.length || end >= children.length) { + return; + } + for (let i = start; i <= end; i++) { newListEl.appendChild(children[i].cloneNode(true)); } @@ -304,19 +320,19 @@ class VirtualScroller { /** * Creates new HTMLElements appended to the newList * - * @param {HTMLElement} newList - the new `li` element + * @param {HTMLElement} newListEl - the new `li` element * @param {number} start - start index * @param {number} end - end index * @return {void} */ - createItems(newList, start, end) { - if (!newList || start < 0 || end < 0) { + createItems(newListEl, start, end) { + if (!newListEl || start < 0 || end < 0) { return; } for (let i = start; i <= end; i++) { const newEl = this.renderItem(i); - newList.appendChild(newEl); + newListEl.appendChild(newEl); } } diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.html b/src/lib/__tests__/ThumbnailsSidebar-test.html new file mode 100644 index 000000000..40d85cb37 --- /dev/null +++ b/src/lib/__tests__/ThumbnailsSidebar-test.html @@ -0,0 +1 @@ +
diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js new file mode 100644 index 000000000..48b515a1c --- /dev/null +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -0,0 +1,224 @@ +/* eslint-disable no-unused-expressions */ +import ThumbnailsSidebar from '../ThumbnailsSidebar'; +import VirtualScroller from '../VirtualScroller'; + +const sandbox = sinon.sandbox.create(); + +describe('ThumbnailsSidebar', () => { + let thumbnailsSidebar; + let stubs = {}; + let pdfViewer = {}; + let page; + let virtualScroller; + let pagePromise; + + before(() => fixture.setBase('src/lib')); + + beforeEach(() => { + fixture.load('__tests__/ThumbnailsSidebar-test.html'); + + stubs.raf = sandbox.stub(window, 'requestAnimationFrame').callsFake((callback) => callback()); + + stubs.getViewport = sandbox.stub(); + stubs.render = sandbox.stub(); + + page = { + getViewport: stubs.getViewport, + render: stubs.render + }; + pagePromise = Promise.resolve(page); + + stubs.getPage = sandbox.stub().returns(pagePromise); + stubs.vsInit = sandbox.stub(VirtualScroller.prototype, 'init'); + stubs.vsDestroy = sandbox.stub(VirtualScroller.prototype, 'destroy'); + + virtualScroller = { + destroy: stubs.vsDestroy + }; + + pdfViewer = { + pdfDocument: { + getPage: stubs.getPage + } + }; + + thumbnailsSidebar = new ThumbnailsSidebar(document.getElementById('test-thumbnails-sidebar'), pdfViewer); + }); + + afterEach(() => { + fixture.cleanup(); + sandbox.verifyAndRestore(); + + if (thumbnailsSidebar && typeof thumbnailsSidebar.destroy === 'function') { + thumbnailsSidebar.destroy(); + } + + thumbnailsSidebar = null; + stubs = {}; + }); + + describe('constructor()', () => { + it('should initialize properties', () => { + expect(thumbnailsSidebar.anchorEl.id).to.be.equal('test-thumbnails-sidebar'); + expect(thumbnailsSidebar.pdfViewer).to.be.equal(pdfViewer); + expect(thumbnailsSidebar.thumbnailImageCache).to.be.empty; + expect(thumbnailsSidebar.scale).to.be.undefined; + expect(thumbnailsSidebar.pageRatio).to.be.undefined; + }); + }); + + describe('destroy()', () => { + it('should clean up the instance properties', () => { + thumbnailsSidebar.destroy(); + + expect(thumbnailsSidebar.thumbnailImageCache).to.be.null; + expect(thumbnailsSidebar.pdfViewer).to.be.null; + }); + + it('should destroy virtualScroller if it exists', () => { + thumbnailsSidebar.virtualScroller = virtualScroller; + thumbnailsSidebar.destroy(); + + expect(stubs.vsDestroy).to.be.called; + expect(thumbnailsSidebar.virtualScroller).to.be.null; + expect(thumbnailsSidebar.thumbnailImageCache).to.be.null; + expect(thumbnailsSidebar.pdfViewer).to.be.null; + }); + }); + + describe('init()', () => { + it('should initialize the render properties', () => { + stubs.getViewport.returns({ width: 10, height: 10 }); + + thumbnailsSidebar.init(); + + return pagePromise.then(() => { + expect(stubs.getViewport).to.be.called; + expect(thumbnailsSidebar.scale).to.be.equal(15); // DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / width + expect(thumbnailsSidebar.pageRatio).to.be.equal(1); + expect(stubs.vsInit).to.be.called; + }); + }); + + it('should not initialize the render properties if viewport does not return width', () => { + stubs.getViewport.returns({ width: undefined, height: 10 }); + + thumbnailsSidebar.init(); + + return pagePromise.then(() => { + expect(stubs.getViewport).to.be.called; + expect(thumbnailsSidebar.scale).to.be.undefined; + expect(thumbnailsSidebar.pageRatio).to.be.undefined; + expect(stubs.vsInit).not.to.be.called; + }); + }); + + it('should not initialize the render properties if viewport does not return height', () => { + stubs.getViewport.returns({ width: 10, height: undefined }); + + thumbnailsSidebar.init(); + + return pagePromise.then(() => { + expect(stubs.getViewport).to.be.called; + expect(thumbnailsSidebar.scale).to.be.undefined; + expect(thumbnailsSidebar.pageRatio).to.be.undefined; + expect(stubs.vsInit).not.to.be.called; + }); + }); + + it('should not initialize the render properties if viewport does not return non zero width & height', () => { + stubs.getViewport.returns({ width: 0, height: 0 }); + + thumbnailsSidebar.init(); + + return pagePromise.then(() => { + expect(stubs.getViewport).to.be.called; + expect(thumbnailsSidebar.scale).to.be.undefined; + expect(thumbnailsSidebar.pageRatio).to.be.undefined; + expect(stubs.vsInit).not.to.be.called; + }); + }); + }); + + describe('generateThumbnailImage()', () => { + beforeEach(() => { + stubs.requestThumbnailImage = sandbox.stub(thumbnailsSidebar, 'requestThumbnailImage'); + }); + + it('should do nothing if startOffset is -1', () => { + thumbnailsSidebar.generateThumbnailImages({ items: [], startOffset: -1 }); + + expect(stubs.requestThumbnailImage).not.to.be.called; + }); + + it('should not request thumbnail images if thumbnail already contains image loaded class', () => { + stubs.contains = sandbox.stub().returns(true); + + const items = [{ classList: { contains: stubs.contains } }]; + + thumbnailsSidebar.generateThumbnailImages({ items, startOffset: 0 }); + + expect(stubs.requestThumbnailImage).not.to.be.called; + }); + + it('should request thumbnail images if thumbnail does not already contains image loaded class', () => { + stubs.contains = sandbox.stub().returns(false); + + const items = [{ classList: { contains: stubs.contains } }]; + + thumbnailsSidebar.generateThumbnailImages({ items, startOffset: 0 }); + + expect(stubs.requestThumbnailImage).to.be.called; + }); + }); + + describe('requestThumbnailImage()', () => { + it('should add the image to the thumbnail element', () => { + const imageEl = {}; + const createImagePromise = Promise.resolve(imageEl); + stubs.createThumbnailImage = sandbox + .stub(thumbnailsSidebar, 'createThumbnailImage') + .returns(createImagePromise); + stubs.appendChild = sandbox.stub(); + stubs.addClass = sandbox.stub(); + + const thumbnailEl = { + appendChild: stubs.appendChild, + classList: { add: stubs.addClass } + }; + + thumbnailsSidebar.requestThumbnailImage(0, thumbnailEl); + + return createImagePromise.then(() => { + expect(stubs.appendChild).to.be.called; + expect(stubs.addClass).to.be.called; + }); + }); + }); + + describe('createThumbnailImage()', () => { + it('should resolve immediately if the image is in cache', () => { + const cachedImage = {}; + thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; + + return thumbnailsSidebar.createThumbnailImage(1).then(() => { + expect(stubs.getPage).not.to.be.called; + }); + }); + + it('should create an image element if not in cache', () => { + const cachedImage = {}; + thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; + thumbnailsSidebar.pageRatio = 1; + + stubs.getViewport.returns({ width: 10, height: 10 }); + stubs.render.returns(Promise.resolve()); + + return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => { + expect(stubs.getPage).to.be.called; + expect(stubs.getViewport).to.be.called; + expect(thumbnailsSidebar.thumbnailImageCache[0]).to.be.eql(imageEl); + }); + }); + }); +}); diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js index e3e82af4d..c3d664a2a 100644 --- a/src/lib/__tests__/VirtualScroller-test.js +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -81,6 +81,22 @@ describe('VirtualScroller', () => { expect(stubs.renderItems).to.be.called; expect(stubs.bindDOMListeners).to.be.called; }); + + it('should call onInit if provided', () => { + const mockListInfo = {}; + stubs.getCurrentListInfo = sandbox.stub(virtualScroller, 'getCurrentListInfo').returns(mockListInfo); + stubs.onInitHandler = sandbox.stub(); + + virtualScroller.init({ + totalItems: 10, + itemHeight: 100, + containerHeight: 500, + renderItemFn: stubs.renderItemFn, + onInit: stubs.onInitHandler + }); + + expect(stubs.onInitHandler).to.be.calledWith(mockListInfo); + }); }); describe('validateRequiredConfig()', () => { @@ -196,7 +212,7 @@ describe('VirtualScroller', () => { describe('renderItem()', () => { it('should render an item absolutely positioned with arbitrary content', () => { - const renderedThumbnail = document.createElement('div'); + const renderedThumbnail = document.createElement('button'); renderedThumbnail.className = 'rendered-thumbnail'; stubs.renderItemFn = sandbox.stub().returns(renderedThumbnail); @@ -211,7 +227,7 @@ describe('VirtualScroller', () => { }); it('should still render the item even if renderItemFn throws an error', () => { - const renderedThumbnail = document.createElement('div'); + const renderedThumbnail = document.createElement('button'); renderedThumbnail.className = 'rendered-thumbnail'; stubs.renderItemFn = sandbox.stub().throws(); @@ -235,4 +251,177 @@ describe('VirtualScroller', () => { expect(virtualScroller.createListElement().classList.contains('bp-vs-list')).to.be.true; }); }); + + describe('onScrollEndHandler()', () => { + beforeEach(() => { + stubs.getCurrentListInfo = sandbox.stub(virtualScroller, 'getCurrentListInfo'); + }); + + it('should do nothing if onScrollEnd is not set', () => { + virtualScroller.onScrollEndHandler(); + + expect(stubs.getCurrentListInfo).not.to.be.called; + }); + + it('should call onScrollEnd with listInfo object', () => { + stubs.onScrollEnd = sandbox.stub(); + virtualScroller.onScrollEnd = stubs.onScrollEnd; + + virtualScroller.onScrollEndHandler(); + + expect(stubs.getCurrentListInfo).to.be.called; + expect(stubs.onScrollEnd).to.be.called; + }); + }); + + describe('getCurrentListInfo()', () => { + let item1; + let item2; + + beforeEach(() => { + item1 = { data: 'hello' }; + item2 = { data: 'bye' }; + }); + + it('should return -1 for offsets if elements do not exist', () => { + virtualScroller.listEl = { + children: [{ children: [item1] }, { children: [item2] }] + }; + + const retObj = virtualScroller.getCurrentListInfo(); + expect(retObj.startOffset).to.be.equal(-1); + expect(retObj.endOffset).to.be.equal(-1); + expect(retObj.items).to.be.eql([item1, item2]); + }); + + it('should return -1 for offsets if data attribute is not a number', () => { + virtualScroller.listEl = { + firstElementChild: { children: [item1], dataset: {} }, + lastElementChild: { children: [item2], dataset: {} }, + children: [{ children: [item1] }, { children: [item2] }] + }; + + const retObj = virtualScroller.getCurrentListInfo(); + expect(retObj.startOffset).to.be.equal(-1); + expect(retObj.endOffset).to.be.equal(-1); + expect(retObj.items).to.be.eql([item1, item2]); + }); + + it('should retrieve the correct data attributes for start and end offsets', () => { + virtualScroller.listEl = { + firstElementChild: { children: [item1], dataset: { bpVsRowIndex: '0' } }, + lastElementChild: { children: [item2], dataset: { bpVsRowIndex: '10' } }, + children: [{ children: [item1] }, { children: [item2] }] + }; + + const retObj = virtualScroller.getCurrentListInfo(); + expect(retObj.startOffset).to.be.equal(0); + expect(retObj.endOffset).to.be.equal(10); + expect(retObj.items).to.be.eql([item1, item2]); + }); + + it('should return [] for items if no children', () => { + virtualScroller.listEl = { + firstElementChild: { children: [item1], dataset: {} }, + lastElementChild: { children: [item2], dataset: {} } + }; + + const retObj = virtualScroller.getCurrentListInfo(); + expect(retObj.startOffset).to.be.equal(-1); + expect(retObj.endOffset).to.be.equal(-1); + expect(retObj.items).to.be.empty; + }); + }); + + describe('cloneItems()', () => { + let newListEl; + + beforeEach(() => { + stubs.appendChild = sandbox.stub(); + newListEl = { appendChild: stubs.appendChild }; + }); + + const paramaterizedTests = [ + { name: 'no newListEl provided', newListEl: undefined, start: 1, end: 2 }, + { name: 'no start provided', newListEl, start: undefined, end: 2 }, + { name: 'no end provided', newListEl, start: 1, end: undefined }, + { name: 'start is < 0 provided', newListEl, start: -1, end: 2 }, + { name: 'end is < 0 provided', newListEl, start: 1, end: -1 }, + { name: 'no oldListEl.children', newListEl, start: 1, end: 2 }, + { + name: 'start is greater than oldListEl.children length', + newListEl, + oldListEl: { children: { length: 1 } }, + start: 1, + end: 3 + }, + { + name: 'end is greater than oldListEl.children length', + newListEl, + oldListEl: { children: { length: 1 } }, + start: 0, + end: 1 + } + ]; + + paramaterizedTests.forEach((testData) => { + it(`should do nothing if ${testData.name}`, () => { + const { newListEl: newList, oldListEl, start, end } = testData; + + virtualScroller.cloneItems(newList, oldListEl, start, end); + + expect(stubs.appendChild).not.to.be.called; + }); + }); + + it('should clone over the items specified', () => { + const oldListEl = { + children: [ + { cloneNode: () => {} }, + { cloneNode: () => {} }, + { cloneNode: () => {} }, + { cloneNode: () => {} } + ] + }; + + virtualScroller.cloneItems(newListEl, oldListEl, 0, 1); + + expect(stubs.appendChild).to.be.calledTwice; + }); + }); + + describe('createItems()', () => { + let newListEl; + + beforeEach(() => { + stubs.appendChild = sandbox.stub(); + stubs.renderItem = sandbox.stub(virtualScroller, 'renderItem'); + newListEl = { appendChild: stubs.appendChild }; + }); + + const paramaterizedTests = [ + { name: 'no newListEl provided', newListEl: undefined, oldListEl: {}, start: 1, end: 2 }, + { name: 'no start provided', newListEl, oldListEl: {}, start: undefined, end: 2 }, + { name: 'no end provided', newListEl, oldListEl: {}, start: 1, end: undefined }, + { name: 'start is < 0 provided', newListEl, oldListEl: {}, start: -1, end: 2 }, + { name: 'end is < 0 provided', newListEl, oldListEl: {}, start: 1, end: -1 } + ]; + + paramaterizedTests.forEach((testData) => { + it(`should do nothing if ${testData.name}`, () => { + const { newListEl: newList, start, end } = testData; + + virtualScroller.createItems(newList, start, end); + + expect(stubs.appendChild).not.to.be.called; + }); + }); + + it('should create the new items specified', () => { + virtualScroller.createItems(newListEl, 0, 2); + + expect(stubs.renderItem).to.be.calledThrice; + expect(stubs.appendChild).to.be.calledThrice; + }); + }); }); From 5a132f394ad73633332bf892c0e4f79832cd8400 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 11 Jan 2019 16:20:51 -0800 Subject: [PATCH 08/29] Update: Use correct thumbnails toggle icon (#890) --- src/lib/icons/thumbnails-toggle-icon.svg | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/lib/icons/thumbnails-toggle-icon.svg b/src/lib/icons/thumbnails-toggle-icon.svg index 86fccb296..f7644db48 100644 --- a/src/lib/icons/thumbnails-toggle-icon.svg +++ b/src/lib/icons/thumbnails-toggle-icon.svg @@ -1,6 +1 @@ - - - - - - + From 64a3fb92c6729d273a5c10c0ab4f722aec04cbe4 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Mon, 14 Jan 2019 16:49:38 -0800 Subject: [PATCH 09/29] Update: Thumbnails navigation (#889) Handle the click events on the thumbnail elements in order to navigate to that page of the document --- src/lib/ThumbnailsSidebar.js | 112 +++++++++++++++++++++++++-- src/lib/viewers/doc/DocBaseViewer.js | 11 ++- src/lib/viewers/doc/_docBase.scss | 30 ++++++- 3 files changed, 141 insertions(+), 12 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index f8061d1c7..7800156d0 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -1,6 +1,11 @@ import isFinite from 'lodash/isFinite'; import VirtualScroller from './VirtualScroller'; +const CLASS_BOX_PREVIEW_THUMBNAIL = 'bp-thumbnail'; +const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE = 'bp-thumbnail-image'; +const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED = 'bp-thumbnail-image-loaded'; +const CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED = 'bp-thumbnail-is-selected'; +const CLASS_BOX_PREVIEW_THUMBNAIL_PAGE_NUMBER = 'bp-thumbnail-page-number'; const DEFAULT_THUMBNAILS_SIDEBAR_WIDTH = 150; const THUMBNAIL_WIDTH_MAX = 210; const THUMBNAIL_MARGIN = 15; @@ -12,6 +17,12 @@ class ThumbnailsSidebar { /** @property {number} - The width : height ratio of the pages of the document */ pageRatio; + /** @property {number} - The currently viewed page */ + currentPage; + + /** @property {Array} - The list of currently rendered thumbnail elements */ + currentThumbnails; + /** @property {PDfViewer} - The PDFJS viewer instance */ pdfViewer; @@ -31,11 +42,41 @@ class ThumbnailsSidebar { this.anchorEl = element; this.pdfViewer = pdfViewer; this.thumbnailImageCache = {}; + this.currentThumbnails = []; this.createPlaceholderThumbnail = this.createPlaceholderThumbnail.bind(this); this.requestThumbnailImage = this.requestThumbnailImage.bind(this); this.createThumbnailImage = this.createThumbnailImage.bind(this); this.generateThumbnailImages = this.generateThumbnailImages.bind(this); + this.thumbnailClickHandler = this.thumbnailClickHandler.bind(this); + + this.anchorEl.addEventListener('click', this.thumbnailClickHandler); + } + + /** + * Method to handle the click events in the Thumbnails Sidebar + * + * @param {Event} evt - Mouse click event + * @return {void} + */ + thumbnailClickHandler(evt) { + const { target } = evt; + + // Only care about clicks on the thumbnail element itself. + // The image and page number have pointer-events: none so + // any click should be the thumbnail element itself. + if (target.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL)) { + // Get the page number + const { bpPageNum: pageNumStr } = target.dataset; + const pageNum = Number.parseInt(pageNumStr, 10); + + if (this.onClickHandler) { + this.onClickHandler(pageNum); + } + } + + evt.preventDefault(); + evt.stopImmediatePropagation(); } /** @@ -51,16 +92,29 @@ class ThumbnailsSidebar { this.thumbnailImageCache = null; this.pdfViewer = null; + this.currentThumbnails = []; + this.currentPage = null; + + this.anchorEl.removeEventListener('click', this.thumbnailClickHandler); } /** * Initializes the Thumbnails Sidebar * + * @param {Object} [options] - options for the Thumbnails Sidebar * @return {void} */ - init() { + init(options) { this.virtualScroller = new VirtualScroller(this.anchorEl); + if (options) { + // Click handler for when a thumbnail is clicked + this.onClickHandler = options.onClick; + + // Specify the current page to be selected + this.currentPage = options.currentPage || 1; + } + // Get the first page of the document, and use its dimensions // to set the thumbnails size of the thumbnails sidebar this.pdfViewer.pdfDocument.getPage(1).then((page) => { @@ -102,8 +156,10 @@ class ThumbnailsSidebar { return; } + this.currentThumbnails = items; + items.forEach((thumbnailEl, index) => { - if (thumbnailEl.classList.contains('bp-thumbnail-image-loaded')) { + if (thumbnailEl.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED)) { return; } @@ -120,9 +176,17 @@ class ThumbnailsSidebar { */ createPlaceholderThumbnail(itemIndex) { const thumbnailEl = document.createElement('button'); - thumbnailEl.className = 'bp-thumbnail'; + const pageNum = itemIndex + 1; + + thumbnailEl.className = CLASS_BOX_PREVIEW_THUMBNAIL; thumbnailEl.setAttribute('type', 'button'); - thumbnailEl.appendChild(this.createPageNumber(itemIndex + 1)); + thumbnailEl.dataset.bpPageNum = pageNum; + thumbnailEl.appendChild(this.createPageNumber(pageNum)); + + if (pageNum === this.currentPage) { + thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED); + } + return thumbnailEl; } @@ -137,7 +201,7 @@ class ThumbnailsSidebar { requestAnimationFrame(() => { this.createThumbnailImage(itemIndex).then((imageEl) => { thumbnailEl.appendChild(imageEl); - thumbnailEl.classList.add('bp-thumbnail-image-loaded'); + thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED); }); }); } @@ -170,8 +234,8 @@ class ThumbnailsSidebar { }) .then(() => { const imageEl = document.createElement('img'); + imageEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE); imageEl.src = canvas.toDataURL(); - imageEl.style.maxWidth = '100%'; // Cache this image element for future use this.thumbnailImageCache[itemIndex] = imageEl; @@ -188,10 +252,44 @@ class ThumbnailsSidebar { */ createPageNumber(pageNumber) { const pageNumberEl = document.createElement('div'); - pageNumberEl.className = 'bp-thumbnail-page-number'; + pageNumberEl.className = CLASS_BOX_PREVIEW_THUMBNAIL_PAGE_NUMBER; pageNumberEl.textContent = `${pageNumber}`; return pageNumberEl; } + + /** + * Sets the currently selected page + * + * @param {number} pageNumber - The page number to set to selected + * @return {void} + */ + setCurrentPage(pageNumber) { + const parsedPageNumber = parseInt(pageNumber, 10); + if (!parsedPageNumber || parsedPageNumber < 1 || parsedPageNumber > this.pdfViewer.pagesCount) { + return; + } + + this.currentPage = parsedPageNumber; + + this.applyCurrentPageSelection(); + } + + /** + * Based on current page selection, checks the currently + * visible thumbnails to toggle the appropriate class + * + * @return {void} + */ + applyCurrentPageSelection() { + this.currentThumbnails.forEach((thumbnailEl) => { + const parsedPageNum = Number.parseInt(thumbnailEl.dataset.bpPageNum, 10); + if (parsedPageNum === this.currentPage) { + thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED); + } else { + thumbnailEl.classList.remove(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED); + } + }); + } } export default ThumbnailsSidebar; diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 2d5e2b306..0001f40bd 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -446,6 +446,10 @@ class DocBaseViewer extends BaseViewer { this.pdfViewer.currentPageNumber = parsedPageNumber; this.cachePage(this.pdfViewer.currentPageNumber); + + if (this.thumbnailsSidebar) { + this.thumbnailsSidebar.setCurrentPage(parsedPageNumber); + } } /** @@ -1059,9 +1063,14 @@ class DocBaseViewer extends BaseViewer { } } + /** + * Initialize the Thumbnails Sidebar + * + * @return {void} + */ initThumbnails() { this.thumbnailsSidebar = new ThumbnailsSidebar(this.thumbnailsSidebarEl, this.pdfViewer); - this.thumbnailsSidebar.init(); + this.thumbnailsSidebar.init({ onClick: this.setPage, currentPage: this.pdfViewer.currentPageNumber }); } /** diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index 97d058578..8af6c7838 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -1,36 +1,58 @@ @import '../../boxuiVariables'; +$thumbnail-border-radius: 4px; + .bp { .bp-thumbnails-container { border-right: solid 1px $seesee; display: flex; - flex: 0 0 180px; + flex: 0 0 181px; // Accounts for the 1px border on the right so the remaining width is actually 180 } .bp-thumbnail { align-items: center; background-color: #d8d8d8; border: 0; - border-radius: 4px; + border-radius: $thumbnail-border-radius; display: flex; flex: 1 1 auto; justify-content: center; margin: 0 15px; - overflow: hidden; padding: 0; position: relative; } .bp-thumbnail-page-number { background-color: $black; - border-radius: 4px; + border-radius: $thumbnail-border-radius; bottom: 10px; color: $white; font-size: 11px; line-height: 16px; opacity: .7; padding: 0 5px; + pointer-events: none; + position: absolute; + } + + .bp-thumbnail-image { + border-radius: $thumbnail-border-radius; + pointer-events: none; + width: 100%; + } + + // Applies a border *outside* the thumbnail button itself rather than + // contributing to the width of the button and shrinking the thumbnail + // image + .bp-thumbnail.bp-thumbnail-is-selected::after { + border: 4px solid $box-blue; + border-radius: $thumbnail-border-radius; + bottom: -2px; + content: ''; + left: -2px; position: absolute; + right: -2px; + top: -2px; } } From 66ced4dfbf56c22510cc4eb3769b69bd70468f1e Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 15 Jan 2019 13:57:43 -0800 Subject: [PATCH 10/29] Chore: unit tests for Thumbnail Sidebar clicks (#891) --- src/lib/ThumbnailsSidebar.js | 10 +- src/lib/__tests__/ThumbnailsSidebar-test.js | 113 ++++++++++++++++++++ 2 files changed, 117 insertions(+), 6 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index 7800156d0..72e1bde4c 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -265,13 +265,11 @@ class ThumbnailsSidebar { */ setCurrentPage(pageNumber) { const parsedPageNumber = parseInt(pageNumber, 10); - if (!parsedPageNumber || parsedPageNumber < 1 || parsedPageNumber > this.pdfViewer.pagesCount) { - return; - } - this.currentPage = parsedPageNumber; - - this.applyCurrentPageSelection(); + if (parsedPageNumber >= 1 && parsedPageNumber <= this.pdfViewer.pagesCount) { + this.currentPage = parsedPageNumber; + this.applyCurrentPageSelection(); + } } /** diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index 48b515a1c..f712f0df0 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -221,4 +221,117 @@ describe('ThumbnailsSidebar', () => { }); }); }); + + describe('thumbnailClickHandler()', () => { + let targetEl; + let evt; + + beforeEach(() => { + stubs.onClickHandler = sandbox.stub(); + stubs.preventDefault = sandbox.stub(); + stubs.stopImmediatePropagation = sandbox.stub(); + + targetEl = document.createElement('div'); + targetEl.classList.add('bp-thumbnail'); + targetEl.dataset.bpPageNum = '3'; + + evt = { + target: targetEl, + preventDefault: stubs.preventDefault, + stopImmediatePropagation: stubs.stopImmediatePropagation + }; + + thumbnailsSidebar.onClickHandler = stubs.onClickHandler; + }); + + it('should call the onClickHandler if target is a thumbnail element', () => { + thumbnailsSidebar.thumbnailClickHandler(evt); + + expect(stubs.onClickHandler).to.be.calledWith(3); + expect(stubs.preventDefault).to.be.called; + expect(stubs.stopImmediatePropagation).to.be.called; + }); + + it('should not call the onClickHandler if target is not thumbnail element', () => { + targetEl.classList.remove('bp-thumbnail'); + thumbnailsSidebar.thumbnailClickHandler(evt); + + expect(stubs.onClickHandler).not.to.be.called; + expect(stubs.preventDefault).to.be.called; + expect(stubs.stopImmediatePropagation).to.be.called; + }); + }); + + describe('setCurrentPage()', () => { + beforeEach(() => { + stubs.applyCurrentPageSelection = sandbox.stub(thumbnailsSidebar, 'applyCurrentPageSelection'); + thumbnailsSidebar.pdfViewer = { pagesCount: 10 }; + }); + + const paramaterizedTests = [ + { name: 'pageNumber is undefined', pageNumber: undefined }, + { name: 'pageNumber is less than 1', pageNumber: 0 }, + { name: 'pageNumber is greater than last page', pageNumber: 11 } + ]; + + paramaterizedTests.forEach(({ name, pageNumber }) => { + it(`should do nothing if ${name}`, () => { + thumbnailsSidebar.setCurrentPage(pageNumber); + + expect(thumbnailsSidebar.currentPage).to.be.undefined; + expect(stubs.applyCurrentPageSelection).not.to.be.called; + }); + }); + + it('should set the currentPage and apply current page selection', () => { + thumbnailsSidebar.setCurrentPage(3); + + expect(thumbnailsSidebar.currentPage).to.be.equal(3); + expect(stubs.applyCurrentPageSelection).to.be.called; + }); + }); + + describe('applyCurrentPageSelection()', () => { + let thumbnails; + + beforeEach(() => { + stubs.addClass = sandbox.stub(); + stubs.removeClass = sandbox.stub(); + + // eslint-disable-next-line + const createTestThumbnail = (pageNum) => { + const thumbnail = document.createElement('div'); + thumbnail.dataset.bpPageNum = pageNum; + thumbnail.classList.add = stubs.addClass; + thumbnail.classList.remove = stubs.removeClass; + return thumbnail; + }; + + const thumbnail1 = createTestThumbnail('1'); + const thumbnail2 = createTestThumbnail('2'); + const thumbnail3 = createTestThumbnail('3'); + + thumbnails = [thumbnail1, thumbnail2, thumbnail3]; + + thumbnailsSidebar.currentThumbnails = thumbnails; + }); + + it('should remove the is selected class from all thumbnails', () => { + thumbnailsSidebar.currentPage = 10; + + thumbnailsSidebar.applyCurrentPageSelection(); + + expect(stubs.removeClass).to.be.calledThrice; + expect(stubs.addClass).not.to.be.called; + }); + + it('should remove the is selected class from all thumbnails', () => { + thumbnailsSidebar.currentPage = 2; + + thumbnailsSidebar.applyCurrentPageSelection(); + + expect(stubs.removeClass).to.be.calledTwice; + expect(stubs.addClass).to.be.calledOnce; + }); + }); }); From cfb9b9f0ea766883c643807a4b2b3d5fbb941213 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 15 Jan 2019 16:37:11 -0800 Subject: [PATCH 11/29] Update: Emit metrics for Thumbnail Sidebar usage (#892) --- src/lib/events.js | 7 +++ src/lib/viewers/BaseViewer.js | 23 ++++++++++ src/lib/viewers/doc/DocBaseViewer.js | 45 +++++++++++++++++-- .../doc/__tests__/DocBaseViewer-test.js | 1 + 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index 93353c71b..288d60bda 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -76,3 +76,10 @@ export const USER_DOCUMENT_FIND_EVENTS = { OPEN: 'user_document_find_open', // The user opens the find bar PREVIOUS: 'user_document_find_previous' // The user navigates to the previous find entry }; + +// Events fired when using thumbnail sidebar +export const USER_DOCUMENT_THUMBNAIL_EVENTS = { + CLOSE: 'user_document_thumbnails_close', + NAVIGATE: 'user_document_thumbnails_navigate', + OPEN: 'user_document_thumbnails_open' +}; diff --git a/src/lib/viewers/BaseViewer.js b/src/lib/viewers/BaseViewer.js index 4b1537777..2bad35fda 100644 --- a/src/lib/viewers/BaseViewer.js +++ b/src/lib/viewers/BaseViewer.js @@ -110,6 +110,9 @@ class BaseViewer extends EventEmitter { /** @property {boolean} - Has the viewer retried downloading the content */ hasRetriedContentDownload = false; + /** @property {Object} - Keeps track of which metrics have been emitted already */ + emittedMetrics; + /** * [constructor] * @@ -125,6 +128,8 @@ class BaseViewer extends EventEmitter { this.isMobile = Browser.isMobile(); this.hasTouch = Browser.hasTouch(); + this.emittedMetrics = {}; + // Bind context for callbacks this.resetLoadTimeout = this.resetLoadTimeout.bind(this); this.preventDefault = this.preventDefault.bind(this); @@ -237,6 +242,7 @@ class BaseViewer extends EventEmitter { this.destroyed = true; this.annotatorPromise = null; this.annotatorPromiseResolver = null; + this.emittedMetrics = null; this.emit('destroy'); } @@ -588,12 +594,29 @@ class BaseViewer extends EventEmitter { * @return {void} */ emitMetric(event, data) { + // If this metric has been emitted already and is on the whitelist of metrics + // to be emitted only once per session, then do nothing + if (this.emittedMetrics[event] && this.getMetricsWhitelist().includes(event)) { + return; + } + + // Mark that this metric has been emitted + this.emittedMetrics[event] = true; + super.emit(VIEWER_EVENT.metric, { event, data }); } + /** + * Method which returns the list of metrics to be emitted only once + * @return {Array} - the array of metric names to be emitted only once + */ + getMetricsWhitelist() { + return []; + } + /** * Handles the beginning of a pinch to zoom event on mobile. * Although W3 strongly discourages the prevention of pinch to zoom, diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 0001f40bd..212aea14f 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -40,7 +40,7 @@ import { ICON_THUMBNAILS_TOGGLE } from '../../icons/icons'; import { JS, PRELOAD_JS, CSS } from './docAssets'; -import { ERROR_CODE, VIEWER_EVENT, LOAD_METRIC } from '../../events'; +import { ERROR_CODE, VIEWER_EVENT, LOAD_METRIC, USER_DOCUMENT_THUMBNAIL_EVENTS } from '../../events'; import Timer from '../../Timer'; const CURRENT_PAGE_MAP_KEY = 'doc-current-page-map'; @@ -63,6 +63,12 @@ const MOBILE_MAX_CANVAS_SIZE = 2949120; // ~3MP 1920x1536 const PINCH_PAGE_CLASS = 'pinch-page'; const PINCHING_CLASS = 'pinching'; const PAGES_UNIT_NAME = 'pages'; +// List of metrics to be emitted only once per session +const METRICS_WHITELIST = [ + USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE, + USER_DOCUMENT_THUMBNAIL_EVENTS.NAVIGATE, + USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN +]; class DocBaseViewer extends BaseViewer { //-------------------------------------------------------------------------- @@ -93,6 +99,7 @@ class DocBaseViewer extends BaseViewer { this.pinchToZoomEndHandler = this.pinchToZoomEndHandler.bind(this); this.emitMetric = this.emitMetric.bind(this); this.toggleThumbnails = this.toggleThumbnails.bind(this); + this.onThumbnailClickHandler = this.onThumbnailClickHandler.bind(this); } /** @@ -574,8 +581,8 @@ class DocBaseViewer extends BaseViewer { * @param {Object} event - Event object * @return {void} */ - emitMetric(event) { - super.emitMetric(event.name, event.data); + emitMetric({ name, data }) { + super.emitMetric(name, data); } //-------------------------------------------------------------------------- @@ -1070,7 +1077,21 @@ class DocBaseViewer extends BaseViewer { */ initThumbnails() { this.thumbnailsSidebar = new ThumbnailsSidebar(this.thumbnailsSidebarEl, this.pdfViewer); - this.thumbnailsSidebar.init({ onClick: this.setPage, currentPage: this.pdfViewer.currentPageNumber }); + this.thumbnailsSidebar.init({ + onClick: this.onThumbnailClickHandler, + currentPage: this.pdfViewer.currentPageNumber + }); + } + + /** + * Handles the click of a thumbnail for navigation + * + * @param {number} pageNum - the page number + * @return {void} + */ + onThumbnailClickHandler(pageNum) { + this.emitMetric({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.NAVIGATE, data: pageNum }); + this.setPage(pageNum); } /** @@ -1311,8 +1332,24 @@ class DocBaseViewer extends BaseViewer { this.thumbnailsSidebarEl.classList.toggle(CLASS_HIDDEN); + const { pagesCount } = this.pdfViewer; + const metricName = this.thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN) + ? USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE + : USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN; + this.emitMetric({ name: metricName, data: pagesCount }); + this.resize(); } + + /** + * Overrides the base method + * + * @override + * @return {Array} - the array of metric names to be emitted only once + */ + getMetricsWhitelist() { + return METRICS_WHITELIST; + } } export default DocBaseViewer; diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 2746b5b06..e91d39baf 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -2225,6 +2225,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { it('should toggle the bp-is-hidden class and resize the viewer', () => { const thumbnailsSidebarEl = document.querySelector(SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER); docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; + docBase.pdfViewer = { pagesCount: 10 }; expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.true; docBase.toggleThumbnails(); From 5d67e63f09228289a867456312ca86a7106715ee Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Thu, 17 Jan 2019 11:57:05 -0800 Subject: [PATCH 12/29] Chore: Thumbnail metrics unit tests (#894) --- src/lib/viewers/__tests__/BaseViewer-test.js | 37 +++++++++++++++++++ .../doc/__tests__/DocBaseViewer-test.js | 32 +++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 0a3a2a311..47dffd057 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -1412,4 +1412,41 @@ describe('lib/viewers/BaseViewer', () => { expect(base.containerEl.insertBefore).to.be.called; }); }); + + describe('emitMetric()', () => { + beforeEach(() => { + stubs.emit = sandbox.stub(EventEmitter.prototype, 'emit'); + stubs.getMetricsWhitelist = sandbox.stub(base, 'getMetricsWhitelist'); + }); + + it('should update the emittedMetrics object when called the first time', () => { + base.emittedMetrics = {}; + stubs.getMetricsWhitelist.returns([]); + + base.emitMetric('foo', 'bar'); + + expect(base.emittedMetrics.foo).to.be.true; + expect(stubs.emit).to.be.called; + }); + + it('should be emitted even if not the first time and not whitelisted', () => { + base.emittedMetrics = { foo: true }; + stubs.getMetricsWhitelist.returns([]); + + base.emitMetric('foo', 'bar'); + + expect(base.emittedMetrics.foo).to.be.true; + expect(stubs.emit).to.be.called; + }); + + it('should not do anything if it has been emitted before and is whitelisted', () => { + base.emittedMetrics = { foo: true }; + stubs.getMetricsWhitelist.returns(['foo']); + + base.emitMetric('foo', 'bar'); + + expect(base.emittedMetrics.foo).to.be.true; + expect(stubs.emit).not.to.be.called; + }); + }); }); diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index e91d39baf..c8b5f54f6 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -31,7 +31,7 @@ import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT } from '../../../icons/icons'; -import { VIEWER_EVENT, LOAD_METRIC } from '../../../events'; +import { VIEWER_EVENT, LOAD_METRIC, USER_DOCUMENT_THUMBNAIL_EVENTS } from '../../../events'; import Timer from '../../../Timer'; const LOAD_TIMEOUT_MS = 180000; // 3 min timeout @@ -2212,6 +2212,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { describe('toggleThumbnails()', () => { beforeEach(() => { sandbox.stub(docBase, 'resize'); + sandbox.stub(docBase, 'emitMetric'); }); it('should do nothing if thumbnails sidebar does not exit', () => { @@ -2222,7 +2223,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(docBase.resize).not.to.be.called; }); - it('should toggle the bp-is-hidden class and resize the viewer', () => { + it('should toggle open and resize the viewer', () => { const thumbnailsSidebarEl = document.querySelector(SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER); docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; docBase.pdfViewer = { pagesCount: 10 }; @@ -2232,6 +2233,33 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.false; expect(docBase.resize).to.be.called; + expect(docBase.emitMetric).to.be.calledWith({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN, data: 10 }); + }); + + it('should toggle close and resize the viewer', () => { + const thumbnailsSidebarEl = document.querySelector(SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER); + docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; + thumbnailsSidebarEl.classList.remove(CLASS_HIDDEN); + docBase.pdfViewer = { pagesCount: 10 }; + expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.false; + + docBase.toggleThumbnails(); + + expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.true; + expect(docBase.resize).to.be.called; + expect(docBase.emitMetric).to.be.calledWith({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE, data: 10 }); + }); + }); + + describe('getMetricsWhitelist()', () => { + it('should return the thumbnail sidebar events', () => { + const expWhitelist = [ + USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE, + USER_DOCUMENT_THUMBNAIL_EVENTS.NAVIGATE, + USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN + ]; + + expect(docBase.getMetricsWhitelist()).to.be.eql(expWhitelist); }); }); }); From 2dc31748bbe2dff1244fb1da2919e20aa4d83862 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 23 Jan 2019 13:43:59 -0800 Subject: [PATCH 13/29] Update: Handle non uniform thumbnail sizes (#896) --- src/lib/ThumbnailsSidebar.js | 81 ++++++++++++++++----- src/lib/VirtualScroller.js | 4 +- src/lib/__tests__/ThumbnailsSidebar-test.js | 51 +++++++++++-- src/lib/_boxuiVariables.scss | 1 + src/lib/viewers/doc/_docBase.scss | 15 ++-- 5 files changed, 122 insertions(+), 30 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index 72e1bde4c..d37b62b0e 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -40,14 +40,16 @@ class ThumbnailsSidebar { */ constructor(element, pdfViewer) { this.anchorEl = element; + this.currentThumbnails = []; this.pdfViewer = pdfViewer; this.thumbnailImageCache = {}; - this.currentThumbnails = []; + this.createImageEl = this.createImageEl.bind(this); this.createPlaceholderThumbnail = this.createPlaceholderThumbnail.bind(this); - this.requestThumbnailImage = this.requestThumbnailImage.bind(this); this.createThumbnailImage = this.createThumbnailImage.bind(this); this.generateThumbnailImages = this.generateThumbnailImages.bind(this); + this.getThumbnailDataURL = this.getThumbnailDataURL.bind(this); + this.requestThumbnailImage = this.requestThumbnailImage.bind(this); this.thumbnailClickHandler = this.thumbnailClickHandler.bind(this); this.anchorEl.addEventListener('click', this.thumbnailClickHandler); @@ -68,7 +70,7 @@ class ThumbnailsSidebar { if (target.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL)) { // Get the page number const { bpPageNum: pageNumStr } = target.dataset; - const pageNum = Number.parseInt(pageNumStr, 10); + const pageNum = parseInt(pageNumStr, 10); if (this.onClickHandler) { this.onClickHandler(pageNum); @@ -218,30 +220,75 @@ class ThumbnailsSidebar { return Promise.resolve(this.thumbnailImageCache[itemIndex]); } + return this.getThumbnailDataURL(itemIndex + 1) + .then(this.createImageEl) + .then((imageEl) => { + // Cache this image element for future use + this.thumbnailImageCache[itemIndex] = imageEl; + + return imageEl; + }); + } + + /** + * Given a page number, generates the image data URL for the image of the page + * @param {number} pageNum - The page number of the document + * @return {string} The data URL of the page image + */ + getThumbnailDataURL(pageNum) { const canvas = document.createElement('canvas'); return this.pdfViewer.pdfDocument - .getPage(itemIndex + 1) + .getPage(pageNum) .then((page) => { - const viewport = page.getViewport(1); - canvas.width = THUMBNAIL_WIDTH_MAX; - canvas.height = THUMBNAIL_WIDTH_MAX / this.pageRatio; - const scale = THUMBNAIL_WIDTH_MAX / viewport.width; + const { width, height } = page.getViewport(1); + // Get the current page w:h ratio in case it differs from the first page + const curPageRatio = width / height; + + // Handle the case where the current page's w:h ratio is less than the + // `pageRatio` which means that this page is probably more portrait than + // landscape + if (curPageRatio < this.pageRatio) { + // Set the canvas height to that of the thumbnail max height + canvas.height = THUMBNAIL_WIDTH_MAX / this.pageRatio; + // Find the canvas width based on the curent page ratio + canvas.width = canvas.height * curPageRatio; + } else { + // In case the current page ratio is same as the first page + // or in case it's larger (which means that it's wider), keep + // the width at the max thumbnail width + canvas.width = THUMBNAIL_WIDTH_MAX; + // Find the height based on the current page ratio + canvas.height = THUMBNAIL_WIDTH_MAX / curPageRatio; + } + + // The amount for which to scale down the current page + const { width: canvasWidth } = canvas; + const scale = canvasWidth / width; return page.render({ canvasContext: canvas.getContext('2d'), viewport: page.getViewport(scale) }); }) - .then(() => { - const imageEl = document.createElement('img'); - imageEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE); - imageEl.src = canvas.toDataURL(); + .then(() => canvas.toDataURL()); + } - // Cache this image element for future use - this.thumbnailImageCache[itemIndex] = imageEl; + /** + * Creates the image element + * @param {string} dataUrl - The image data URL for the thumbnail + * @return {HTMLElement} - The image element + */ + createImageEl(dataUrl) { + const imageEl = document.createElement('div'); + imageEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE); + imageEl.style.backgroundImage = `url('${dataUrl}')`; - return imageEl; - }); + // Add the height and width to the image to be the same as the thumbnail + // so that the css `background-image` rules will work + imageEl.style.width = `${DEFAULT_THUMBNAILS_SIDEBAR_WIDTH}px`; + imageEl.style.height = `${DEFAULT_THUMBNAILS_SIDEBAR_WIDTH / this.pageRatio}px`; + + return imageEl; } /** @@ -280,7 +327,7 @@ class ThumbnailsSidebar { */ applyCurrentPageSelection() { this.currentThumbnails.forEach((thumbnailEl) => { - const parsedPageNum = Number.parseInt(thumbnailEl.dataset.bpPageNum, 10); + const parsedPageNum = parseInt(thumbnailEl.dataset.bpPageNum, 10); if (parsedPageNum === this.currentPage) { thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IS_SELECTED); } else { diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index f4d02d32c..6d71cf05d 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -207,8 +207,8 @@ class VirtualScroller { const { firstElementChild, lastElementChild, children } = this.listEl; // Parse the row index from the data-attribute - let curStartOffset = firstElementChild ? Number.parseInt(firstElementChild.dataset.bpVsRowIndex, 10) : -1; - let curEndOffset = lastElementChild ? Number.parseInt(lastElementChild.dataset.bpVsRowIndex, 10) : -1; + let curStartOffset = firstElementChild ? parseInt(firstElementChild.dataset.bpVsRowIndex, 10) : -1; + let curEndOffset = lastElementChild ? parseInt(lastElementChild.dataset.bpVsRowIndex, 10) : -1; // If the data-attribute value is not present default to invalid -1 curStartOffset = isFinite(curStartOffset) ? curStartOffset : -1; diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index f712f0df0..3360da514 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -196,28 +196,67 @@ describe('ThumbnailsSidebar', () => { }); }); - describe('createThumbnailImage()', () => { + describe('createThumbnailImage', () => { + beforeEach(() => { + stubs.getThumbnailDataURL = sandbox + .stub(thumbnailsSidebar, 'getThumbnailDataURL') + .returns(Promise.resolve()); + stubs.createImageEl = sandbox.stub(thumbnailsSidebar, 'createImageEl'); + }); + it('should resolve immediately if the image is in cache', () => { const cachedImage = {}; thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; return thumbnailsSidebar.createThumbnailImage(1).then(() => { - expect(stubs.getPage).not.to.be.called; + expect(stubs.createImageEl).not.to.be.called; }); }); it('should create an image element if not in cache', () => { + const cachedImage = {}; + thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; + stubs.createImageEl.returns(cachedImage); + + return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => { + expect(stubs.createImageEl).to.be.called; + expect(thumbnailsSidebar.thumbnailImageCache[0]).to.be.eql(imageEl); + }); + }); + }); + + describe('getThumbnailDataURL()', () => { + it('should scale canvas the same as the first page if page ratio is the same', () => { const cachedImage = {}; thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; thumbnailsSidebar.pageRatio = 1; - stubs.getViewport.returns({ width: 10, height: 10 }); + // Current page has same ratio + stubs.getViewport.withArgs(1).returns({ width: 10, height: 10 }); stubs.render.returns(Promise.resolve()); - return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => { + const expScale = 21; // Should be THUMBNAIL_WIDTH_MAX(210) / 10 = 21 + + return thumbnailsSidebar.getThumbnailDataURL(1).then(() => { expect(stubs.getPage).to.be.called; - expect(stubs.getViewport).to.be.called; - expect(thumbnailsSidebar.thumbnailImageCache[0]).to.be.eql(imageEl); + expect(stubs.getViewport.withArgs(expScale)).to.be.called; + }); + }); + + it('should handle non-uniform page ratios', () => { + const cachedImage = {}; + thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; + thumbnailsSidebar.pageRatio = 1; + + // Current page has ratio of 0.5 instead of 1 + stubs.getViewport.withArgs(1).returns({ width: 10, height: 20 }); + stubs.render.returns(Promise.resolve()); + + const expScale = 10.5; // Should be 10.5 instead of 21 because the viewport ratio above is 0.5 instead of 1 + + return thumbnailsSidebar.createThumbnailImage(0).then(() => { + expect(stubs.getPage).to.be.called; + expect(stubs.getViewport.withArgs(expScale)).to.be.called; }); }); }); diff --git a/src/lib/_boxuiVariables.scss b/src/lib/_boxuiVariables.scss index 551927a7e..cf79026c7 100644 --- a/src/lib/_boxuiVariables.scss +++ b/src/lib/_boxuiVariables.scss @@ -48,3 +48,4 @@ $tendemob-grey: #64686d !default; $sunset-grey: #464a4f !default; $seventy-sixers: #767676 !default; $approx-mischka-grey: #a3adb9 !default; +$d-eight: #d8d8d8 !default; diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index 8af6c7838..bac6cdeda 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -6,20 +6,21 @@ $thumbnail-border-radius: 4px; .bp-thumbnails-container { border-right: solid 1px $seesee; display: flex; - flex: 0 0 181px; // Accounts for the 1px border on the right so the remaining width is actually 180 + flex: 0 0 201px; // Accounts for the 1px border on the right so the remaining width is actually 180 } .bp-thumbnail { align-items: center; - background-color: #d8d8d8; + background-color: $d-eight; border: 0; border-radius: $thumbnail-border-radius; display: flex; - flex: 1 1 auto; + flex: 1 0 auto; justify-content: center; - margin: 0 15px; + margin: 0 25px; padding: 0; position: relative; + width: 150px; } .bp-thumbnail-page-number { @@ -28,17 +29,21 @@ $thumbnail-border-radius: 4px; bottom: 10px; color: $white; font-size: 11px; + left: 50%; line-height: 16px; opacity: .7; padding: 0 5px; pointer-events: none; position: absolute; + transform: translateX(-50%); } .bp-thumbnail-image { + background-position: center; + background-repeat: no-repeat; + background-size: contain; border-radius: $thumbnail-border-radius; pointer-events: none; - width: 100%; } // Applies a border *outside* the thumbnail button itself rather than From f1417d4d1d3fa13997db0e9e668702b38667686b Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 23 Jan 2019 13:49:39 -0800 Subject: [PATCH 14/29] Update: Emit thumbnails[Open|Close] viewer event (#898) --- src/lib/viewers/doc/DocBaseViewer.js | 17 +++++++++++++---- .../viewers/doc/__tests__/DocBaseViewer-test.js | 5 ++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 212aea14f..09fb1cf64 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -698,7 +698,7 @@ class DocBaseViewer extends BaseViewer { } // Save page and return after resize - const { currentPageNumber } = this.pdfViewer.currentPageNumber; + const { currentPageNumber } = this.pdfViewer; this.pdfViewer.currentScaleValue = this.pdfViewer.currentScaleValue || 'auto'; this.pdfViewer.update(); @@ -1333,10 +1333,19 @@ class DocBaseViewer extends BaseViewer { this.thumbnailsSidebarEl.classList.toggle(CLASS_HIDDEN); const { pagesCount } = this.pdfViewer; - const metricName = this.thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN) - ? USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE - : USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN; + + let metricName; + let eventName; + if (this.thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)) { + metricName = USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE; + eventName = 'thumbnailsClose'; + } else { + metricName = USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN; + eventName = 'thumbnailsOpen'; + } + this.emitMetric({ name: metricName, data: pagesCount }); + this.emit(eventName); this.resize(); } diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index c8b5f54f6..a153c190d 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -2213,9 +2213,10 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { beforeEach(() => { sandbox.stub(docBase, 'resize'); sandbox.stub(docBase, 'emitMetric'); + sandbox.stub(docBase, 'emit'); }); - it('should do nothing if thumbnails sidebar does not exit', () => { + it('should do nothing if thumbnails sidebar does not exist', () => { docBase.thumbnailsSidebarEl = undefined; docBase.toggleThumbnails(); @@ -2234,6 +2235,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.false; expect(docBase.resize).to.be.called; expect(docBase.emitMetric).to.be.calledWith({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN, data: 10 }); + expect(docBase.emit).to.be.calledWith('thumbnailsOpen'); }); it('should toggle close and resize the viewer', () => { @@ -2248,6 +2250,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.true; expect(docBase.resize).to.be.called; expect(docBase.emitMetric).to.be.calledWith({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE, data: 10 }); + expect(docBase.emit).to.be.calledWith('thumbnailsClose'); }); }); From dba419e8a9fe8ae5cb87dcfc570e2ce12d23152f Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Thu, 24 Jan 2019 13:19:44 -0800 Subject: [PATCH 15/29] Update: Change VirtualScroller.renderItems strategy (#900) --- src/lib/VirtualScroller.js | 63 ++++++------- src/lib/__tests__/VirtualScroller-test.js | 110 ++++++++++++---------- 2 files changed, 86 insertions(+), 87 deletions(-) diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 6d71cf05d..67089da2a 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -253,8 +253,8 @@ class VirtualScroller { newEndOffset = this.totalItems - 1; } - // Create a new list element to be swapped out for the existing one - const newListEl = this.createListElement(); + // Creates a document fragment for the new list items to be appended into the list + const fragment = document.createDocumentFragment(); if (curStartOffset <= offset && offset <= curEndOffset) { // Scenario #1: New start offset falls within the current range of items rendered @@ -263,10 +263,12 @@ class VirtualScroller { // |--------------------| // newStartOffset newEndOffset newStartOffset = curEndOffset + 1; - // clone elements from newStartOffset to curEndOffset - this.cloneItems(newListEl, this.listEl, offset - curStartOffset, curEndOffset - curStartOffset); - // then create elements from curEnd + 1 to newEndOffset - this.createItems(newListEl, newStartOffset, newEndOffset); + // Create elements from curEnd + 1 to newEndOffset + this.createItems(fragment, newStartOffset, newEndOffset); + // Delete the elements from curStartOffset to newStartOffset + this.deleteItems(this.listEl, curStartOffset - curStartOffset, offset - curStartOffset); + // Append the document fragment to the listEl + this.listEl.appendChild(fragment); } else if (curStartOffset <= newEndOffset && newEndOffset <= curEndOffset) { // Scenario #2: New end offset falls within the current range of items rendered // |--------------------| @@ -274,66 +276,57 @@ class VirtualScroller { // |--------------------| // newStartOffset newEndOffset - // create elements from newStartOffset to curStart - 1 - this.createItems(newListEl, offset, curStartOffset - 1); - // then clone elements from curStartOffset to newEndOffset - this.cloneItems(newListEl, this.listEl, 0, newEndOffset - curStartOffset); + // Create elements from newStartOffset to curStart - 1 + this.createItems(fragment, offset, curStartOffset - 1); + // Delete the elements from newEndOffset to the end + this.deleteItems(this.listEl, newEndOffset - curStartOffset + 1); + // Insert before the firstElementChild of the listEl + this.listEl.insertBefore(fragment, this.listEl.firstElementChild); } else { // Scenario #3: New range has no overlap with current range of items // |--------------------| // curStartOffset curEndOffset // |--------------------| // newStartOffset newEndOffset - this.createItems(newListEl, newStartOffset, newEndOffset); + this.createItems(fragment, newStartOffset, newEndOffset); + this.listEl.appendChild(fragment); } - - this.scrollingEl.replaceChild(newListEl, this.listEl); - this.listEl = newListEl; } /** - * Clones a subset of the HTMLElements from the oldList to the newList. - * The newList element is modified. + * Creates new HTMLElements appended to the newList * * @param {HTMLElement} newListEl - the new `ol` element - * @param {HTMLElement} oldListEl - the old `ol` element * @param {number} start - start index * @param {number} end - end index * @return {void} */ - cloneItems(newListEl, oldListEl, start, end) { - if (!newListEl || !oldListEl || start < 0 || end < 0) { - return; - } - - const { children } = oldListEl; - - if (!children || start >= children.length || end >= children.length) { + createItems(newListEl, start, end) { + if (!newListEl || start < 0 || end < 0) { return; } for (let i = start; i <= end; i++) { - newListEl.appendChild(children[i].cloneNode(true)); + const newEl = this.renderItem(i); + newListEl.appendChild(newEl); } } /** - * Creates new HTMLElements appended to the newList + * Deletes elements of the 'ol' * - * @param {HTMLElement} newListEl - the new `li` element + * @param {HTMLElement} listEl - the `ol` element * @param {number} start - start index - * @param {number} end - end index + * @param {number} [end] - end index * @return {void} */ - createItems(newListEl, start, end) { - if (!newListEl || start < 0 || end < 0) { + deleteItems(listEl, start, end) { + if (!listEl || start < 0 || end < 0) { return; } - for (let i = start; i <= end; i++) { - const newEl = this.renderItem(i); - newListEl.appendChild(newEl); - } + const listItems = Array.prototype.slice.call(listEl.children, start, end); + listItems.forEach((listItem) => listEl.removeChild(listItem)); } /** diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js index c3d664a2a..6476c97f8 100644 --- a/src/lib/__tests__/VirtualScroller-test.js +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -162,23 +162,20 @@ describe('VirtualScroller', () => { describe('renderItems()', () => { let newListEl; - const curListEl = {}; + let curListEl; beforeEach(() => { - virtualScroller.scrollingEl = { replaceChild: () => {} }; + stubs.appendChild = sandbox.stub(); + stubs.insertBefore = sandbox.stub(); + curListEl = { appendChild: stubs.appendChild, insertBefore: stubs.insertBefore }; + newListEl = {}; virtualScroller.listEl = curListEl; - newListEl = { appendChild: () => {} }; - stubs.createListElement = sandbox.stub(virtualScroller, 'createListElement').returns(newListEl); + stubs.renderItem = sandbox.stub(virtualScroller, 'renderItem'); - stubs.replaceChild = sandbox.stub(virtualScroller.scrollingEl, 'replaceChild'); - stubs.appendChild = sandbox.stub(newListEl, 'appendChild'); stubs.getCurrentListInfo = sandbox.stub(virtualScroller, 'getCurrentListInfo'); - stubs.cloneItems = sandbox.stub(virtualScroller, 'cloneItems'); stubs.createItems = sandbox.stub(virtualScroller, 'createItems'); - }); - - afterEach(() => { - virtualScroller.scrollingEl = null; + stubs.deleteItems = sandbox.stub(virtualScroller, 'deleteItems'); + stubs.createDocumentFragment = sandbox.stub(document, 'createDocumentFragment').returns(newListEl); }); it('should render the whole range of items (no reuse)', () => { @@ -190,9 +187,10 @@ describe('VirtualScroller', () => { }); virtualScroller.renderItems(); - expect(stubs.cloneItems).not.to.be.called; + expect(stubs.deleteItems).not.to.be.called; expect(stubs.createItems).to.be.calledWith(newListEl, 0, 10); - expect(virtualScroller.scrollingEl.replaceChild).to.be.called; + expect(stubs.appendChild).to.be.called; + expect(stubs.insertBefore).not.to.be.called; }); it('should render the remaining items up to totalItems', () => { @@ -204,9 +202,25 @@ describe('VirtualScroller', () => { }); virtualScroller.renderItems(95); - expect(stubs.cloneItems).not.to.be.called; + expect(stubs.deleteItems).not.to.be.called; expect(stubs.createItems).to.be.calledWith(newListEl, 95, 99); - expect(virtualScroller.scrollingEl.replaceChild).to.be.called; + expect(stubs.appendChild).to.be.called; + expect(stubs.insertBefore).not.to.be.called; + }); + + it('should render items above the current list', () => { + virtualScroller.maxRenderedItems = 10; + virtualScroller.totalItems = 100; + stubs.getCurrentListInfo.returns({ + startOffset: 20, + endOffset: 30 + }); + virtualScroller.renderItems(15); + + expect(stubs.deleteItems).to.be.called; + expect(stubs.createItems).to.be.calledWith(newListEl, 15, 19); + expect(stubs.appendChild).not.to.be.called; + expect(stubs.insertBefore).to.be.called; }); }); @@ -333,60 +347,52 @@ describe('VirtualScroller', () => { }); }); - describe('cloneItems()', () => { - let newListEl; + describe('deleteItems()', () => { + let listEl; beforeEach(() => { - stubs.appendChild = sandbox.stub(); - newListEl = { appendChild: stubs.appendChild }; + stubs.removeChild = sandbox.stub(); + listEl = { removeChild: stubs.removeChild }; }); const paramaterizedTests = [ - { name: 'no newListEl provided', newListEl: undefined, start: 1, end: 2 }, - { name: 'no start provided', newListEl, start: undefined, end: 2 }, - { name: 'no end provided', newListEl, start: 1, end: undefined }, - { name: 'start is < 0 provided', newListEl, start: -1, end: 2 }, - { name: 'end is < 0 provided', newListEl, start: 1, end: -1 }, - { name: 'no oldListEl.children', newListEl, start: 1, end: 2 }, - { - name: 'start is greater than oldListEl.children length', - newListEl, - oldListEl: { children: { length: 1 } }, - start: 1, - end: 3 - }, - { - name: 'end is greater than oldListEl.children length', - newListEl, - oldListEl: { children: { length: 1 } }, - start: 0, - end: 1 - } + { name: 'no listEl provided', listEl: undefined, start: 1, end: 2 }, + { name: 'no start provided', listEl, start: undefined, end: 2 }, + { name: 'no end provided', listEl, start: 1, end: undefined }, + { name: 'start is < 0 provided', listEl, start: -1, end: 2 }, + { name: 'end is < 0 provided', listEl, start: 1, end: -1 } ]; paramaterizedTests.forEach((testData) => { it(`should do nothing if ${testData.name}`, () => { - const { newListEl: newList, oldListEl, start, end } = testData; + const { listEl: list, start, end } = testData; - virtualScroller.cloneItems(newList, oldListEl, start, end); + virtualScroller.deleteItems(list, start, end); - expect(stubs.appendChild).not.to.be.called; + expect(stubs.removeChild).not.to.be.called; }); }); - it('should clone over the items specified', () => { - const oldListEl = { - children: [ - { cloneNode: () => {} }, - { cloneNode: () => {} }, - { cloneNode: () => {} }, - { cloneNode: () => {} } - ] + it('should remove the items specified', () => { + const list = { + children: [{}, {}, {}, {}], + removeChild: stubs.removeChild + }; + + virtualScroller.deleteItems(list, 0, 1); + + expect(stubs.removeChild).to.be.calledOnce; + }); + + it('should remove the items specified from start to the end when end is not provided', () => { + const list = { + children: [{}, {}, {}, {}], + removeChild: stubs.removeChild }; - virtualScroller.cloneItems(newListEl, oldListEl, 0, 1); + virtualScroller.deleteItems(list, 2); - expect(stubs.appendChild).to.be.calledTwice; + expect(stubs.removeChild).to.be.calledTwice; }); }); From e9037fd362a7081070cad48161f38d41918b7fbe Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Mon, 28 Jan 2019 10:51:23 -0800 Subject: [PATCH 16/29] Update: Serially render thumbnail images (#901) --- src/lib/ThumbnailsSidebar.js | 62 +++++++++++++++------ src/lib/VirtualScroller.js | 6 +- src/lib/__tests__/ThumbnailsSidebar-test.js | 58 ++++++++++++++----- src/lib/__tests__/VirtualScroller-test.js | 4 +- 4 files changed, 94 insertions(+), 36 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index d37b62b0e..9d02a3c14 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -49,6 +49,7 @@ class ThumbnailsSidebar { this.createThumbnailImage = this.createThumbnailImage.bind(this); this.generateThumbnailImages = this.generateThumbnailImages.bind(this); this.getThumbnailDataURL = this.getThumbnailDataURL.bind(this); + this.renderNextThumbnailImage = this.renderNextThumbnailImage.bind(this); this.requestThumbnailImage = this.requestThumbnailImage.bind(this); this.thumbnailClickHandler = this.thumbnailClickHandler.bind(this); @@ -153,20 +154,29 @@ class ThumbnailsSidebar { * @param {Object} currentListInfo - VirtualScroller info object which contains startOffset, endOffset, and the thumbnail elements * @return {void} */ - generateThumbnailImages({ items, startOffset }) { - if (!isFinite(startOffset) || startOffset < 0) { - return; - } - + generateThumbnailImages({ items }) { this.currentThumbnails = items; - items.forEach((thumbnailEl, index) => { - if (thumbnailEl.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED)) { - return; - } + // Serially renders the thumbnails one by one as needed + this.renderNextThumbnailImage(); + } - this.requestThumbnailImage(index + startOffset, thumbnailEl); - }); + /** + * Requests the next thumbnail image that needs rendering + * + * @return {void} + */ + renderNextThumbnailImage() { + // Iterates over the current thumbnails and requests rendering of the first + // thumbnail it encounters that does not have an image loaded + const nextThumbnailEl = this.currentThumbnails.find( + (thumbnailEl) => !thumbnailEl.classList.contains(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED) + ); + + if (nextThumbnailEl) { + const parsedPageNum = parseInt(nextThumbnailEl.dataset.bpPageNum, 10); + this.requestThumbnailImage(parsedPageNum - 1, nextThumbnailEl); + } } /** @@ -202,8 +212,14 @@ class ThumbnailsSidebar { requestThumbnailImage(itemIndex, thumbnailEl) { requestAnimationFrame(() => { this.createThumbnailImage(itemIndex).then((imageEl) => { - thumbnailEl.appendChild(imageEl); - thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED); + // Promise will resolve with null if create image request was already in progress + if (imageEl) { + thumbnailEl.appendChild(imageEl); + thumbnailEl.classList.add(CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE_LOADED); + } + + // After generating the thumbnail image, render the next one + this.renderNextThumbnailImage(); }); }); } @@ -212,19 +228,29 @@ class ThumbnailsSidebar { * Make a thumbnail image element * * @param {number} itemIndex - the item index for the overall list (0 indexed) - * @return {Promise} - promise reolves with the image HTMLElement + * @return {Promise} - promise reolves with the image HTMLElement or null if generation is in progress */ createThumbnailImage(itemIndex) { - // If this page has already been cached, use it - if (this.thumbnailImageCache[itemIndex]) { - return Promise.resolve(this.thumbnailImageCache[itemIndex]); + const cacheEntry = this.thumbnailImageCache[itemIndex]; + + // If this thumbnail has already been cached, use it + if (cacheEntry && cacheEntry.image) { + return Promise.resolve(cacheEntry.image); + } + + // If this thumbnail has already been requested, resolve with null + if (cacheEntry && cacheEntry.inProgress) { + return Promise.resolve(null); } + // Update the cache entry to be in progress + this.thumbnailImageCache[itemIndex] = { ...cacheEntry, inProgress: true }; + return this.getThumbnailDataURL(itemIndex + 1) .then(this.createImageEl) .then((imageEl) => { // Cache this image element for future use - this.thumbnailImageCache[itemIndex] = imageEl; + this.thumbnailImageCache[itemIndex] = { inProgress: false, image: imageEl }; return imageEl; }); diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 67089da2a..5b152765b 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -289,6 +289,8 @@ class VirtualScroller { // |--------------------| // newStartOffset newEndOffset this.createItems(fragment, newStartOffset, newEndOffset); + // Delete all the current elements (if any) + this.deleteItems(this.listEl); this.listEl.appendChild(fragment); } } @@ -316,11 +318,11 @@ class VirtualScroller { * Deletes elements of the 'ol' * * @param {HTMLElement} listEl - the `ol` element - * @param {number} start - start index + * @param {number} [start] - start index * @param {number} [end] - end index * @return {void} */ - deleteItems(listEl, start, end) { + deleteItems(listEl, start = 0, end) { if (!listEl || start < 0 || end < 0) { return; } diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index 3360da514..c7535643c 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -140,35 +140,53 @@ describe('ThumbnailsSidebar', () => { }); }); - describe('generateThumbnailImage()', () => { + describe('renderNextThumbnailImage()', () => { beforeEach(() => { stubs.requestThumbnailImage = sandbox.stub(thumbnailsSidebar, 'requestThumbnailImage'); }); - it('should do nothing if startOffset is -1', () => { - thumbnailsSidebar.generateThumbnailImages({ items: [], startOffset: -1 }); + // eslint-disable-next-line + const createThumbnailEl = (pageNum, contains) => { + return { + classList: { + contains: () => contains + }, + dataset: { + bpPageNum: pageNum + } + }; + }; + + it('should do nothing there are no current thumbnails', () => { + thumbnailsSidebar.currentThumbnails = []; + thumbnailsSidebar.renderNextThumbnailImage(); expect(stubs.requestThumbnailImage).not.to.be.called; }); it('should not request thumbnail images if thumbnail already contains image loaded class', () => { - stubs.contains = sandbox.stub().returns(true); - - const items = [{ classList: { contains: stubs.contains } }]; + const items = [createThumbnailEl(1, true)]; + thumbnailsSidebar.currentThumbnails = items; - thumbnailsSidebar.generateThumbnailImages({ items, startOffset: 0 }); + thumbnailsSidebar.renderNextThumbnailImage(); expect(stubs.requestThumbnailImage).not.to.be.called; }); it('should request thumbnail images if thumbnail does not already contains image loaded class', () => { - stubs.contains = sandbox.stub().returns(false); + const items = [createThumbnailEl(1, false)]; + thumbnailsSidebar.currentThumbnails = items; + thumbnailsSidebar.renderNextThumbnailImage(); - const items = [{ classList: { contains: stubs.contains } }]; + expect(stubs.requestThumbnailImage).to.be.calledOnce; + }); - thumbnailsSidebar.generateThumbnailImages({ items, startOffset: 0 }); + it('should only request the first thumbnail that does not already contain an image loaded class', () => { + const items = [createThumbnailEl(1, true), createThumbnailEl(2, false), createThumbnailEl(3, false)]; + thumbnailsSidebar.currentThumbnails = items; + thumbnailsSidebar.renderNextThumbnailImage(); - expect(stubs.requestThumbnailImage).to.be.called; + expect(stubs.requestThumbnailImage).to.be.calledOnce; }); }); @@ -206,7 +224,7 @@ describe('ThumbnailsSidebar', () => { it('should resolve immediately if the image is in cache', () => { const cachedImage = {}; - thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; + thumbnailsSidebar.thumbnailImageCache = { 1: { image: cachedImage } }; return thumbnailsSidebar.createThumbnailImage(1).then(() => { expect(stubs.createImageEl).not.to.be.called; @@ -215,12 +233,24 @@ describe('ThumbnailsSidebar', () => { it('should create an image element if not in cache', () => { const cachedImage = {}; - thumbnailsSidebar.thumbnailImageCache = { 1: cachedImage }; + thumbnailsSidebar.thumbnailImageCache = { 1: { image: cachedImage } }; stubs.createImageEl.returns(cachedImage); return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => { expect(stubs.createImageEl).to.be.called; - expect(thumbnailsSidebar.thumbnailImageCache[0]).to.be.eql(imageEl); + expect(thumbnailsSidebar.thumbnailImageCache[0].image).to.be.eql(imageEl); + expect(thumbnailsSidebar.thumbnailImageCache[0].inProgress).to.be.false; + }); + }); + + it('should resolve with null if cache entry inProgress is true', () => { + const cachedImage = {}; + thumbnailsSidebar.thumbnailImageCache = { 0: { inProgress: true } }; + stubs.createImageEl.returns(cachedImage); + + return thumbnailsSidebar.createThumbnailImage(0).then((imageEl) => { + expect(stubs.createImageEl).not.to.be.called; + expect(imageEl).to.be.null; }); }); }); diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js index 6476c97f8..509f73f36 100644 --- a/src/lib/__tests__/VirtualScroller-test.js +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -187,7 +187,7 @@ describe('VirtualScroller', () => { }); virtualScroller.renderItems(); - expect(stubs.deleteItems).not.to.be.called; + expect(stubs.deleteItems).to.be.calledWith(curListEl); expect(stubs.createItems).to.be.calledWith(newListEl, 0, 10); expect(stubs.appendChild).to.be.called; expect(stubs.insertBefore).not.to.be.called; @@ -202,7 +202,7 @@ describe('VirtualScroller', () => { }); virtualScroller.renderItems(95); - expect(stubs.deleteItems).not.to.be.called; + expect(stubs.deleteItems).to.be.calledWith(curListEl); expect(stubs.createItems).to.be.calledWith(newListEl, 95, 99); expect(stubs.appendChild).to.be.called; expect(stubs.insertBefore).not.to.be.called; From 713da29db50e8bf370003a8eaf4522a82e119996 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 1 Feb 2019 14:32:59 -0800 Subject: [PATCH 17/29] Update: Scroll into view (#906) --- src/lib/ThumbnailsSidebar.js | 53 ++++++ src/lib/VirtualScroller.js | 57 ++++++- src/lib/__tests__/ThumbnailsSidebar-test.js | 101 ++++++++++- src/lib/__tests__/VirtualScroller-test.js | 161 +++++++++++++++++- src/lib/viewers/doc/DocBaseViewer.js | 10 +- .../doc/__tests__/DocBaseViewer-test.js | 31 ++-- 6 files changed, 387 insertions(+), 26 deletions(-) diff --git a/src/lib/ThumbnailsSidebar.js b/src/lib/ThumbnailsSidebar.js index 9d02a3c14..7ddf32d7b 100644 --- a/src/lib/ThumbnailsSidebar.js +++ b/src/lib/ThumbnailsSidebar.js @@ -1,5 +1,6 @@ import isFinite from 'lodash/isFinite'; import VirtualScroller from './VirtualScroller'; +import { CLASS_HIDDEN } from './constants'; const CLASS_BOX_PREVIEW_THUMBNAIL = 'bp-thumbnail'; const CLASS_BOX_PREVIEW_THUMBNAIL_IMAGE = 'bp-thumbnail-image'; @@ -137,6 +138,7 @@ class ThumbnailsSidebar { const scaledViewport = page.getViewport(this.scale); this.virtualScroller.init({ + initialRowIndex: this.currentPage - 1, totalItems: this.pdfViewer.pagesCount, itemHeight: scaledViewport.height, containerHeight: this.anchorEl.parentNode.clientHeight, @@ -342,6 +344,7 @@ class ThumbnailsSidebar { if (parsedPageNumber >= 1 && parsedPageNumber <= this.pdfViewer.pagesCount) { this.currentPage = parsedPageNumber; this.applyCurrentPageSelection(); + this.virtualScroller.scrollIntoView(parsedPageNumber - 1); } } @@ -361,6 +364,56 @@ class ThumbnailsSidebar { } }); } + + /** + * Toggles the thumbnails sidebar + * @return {void} + */ + toggle() { + if (!this.anchorEl) { + return; + } + + if (!this.isOpen()) { + this.toggleOpen(); + } else { + this.toggleClose(); + } + } + + /** + * Returns whether the sidebar is open or not + * @return {boolean} true if the sidebar is open, false if not + */ + isOpen() { + return this.anchorEl && !this.anchorEl.classList.contains(CLASS_HIDDEN); + } + + /** + * Toggles the sidebar open. This will scroll the current page into view + * @return {void} + */ + toggleOpen() { + if (!this.anchorEl) { + return; + } + + this.anchorEl.classList.remove(CLASS_HIDDEN); + + this.virtualScroller.scrollIntoView(this.currentPage - 1); + } + + /** + * Toggles the sidebar closed + * @return {void} + */ + toggleClose() { + if (!this.anchorEl) { + return; + } + + this.anchorEl.classList.add(CLASS_HIDDEN); + } } export default ThumbnailsSidebar; diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 5b152765b..60a67b013 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -56,10 +56,12 @@ class VirtualScroller { this.previousScrollTop = 0; this.createListElement = this.createListElement.bind(this); + this.isVisible = this.isVisible.bind(this); this.onScrollEndHandler = this.onScrollEndHandler.bind(this); this.onScrollHandler = this.onScrollHandler.bind(this); this.getCurrentListInfo = this.getCurrentListInfo.bind(this); this.renderItems = this.renderItems.bind(this); + this.scrollIntoView = this.scrollIntoView.bind(this); this.debouncedOnScrollEndHandler = debounce(this.onScrollEndHandler, DEBOUNCE_SCROLL_THRESHOLD); this.throttledOnScrollHandler = throttle(this.onScrollHandler, THROTTLE_SCROLL_THRESHOLD); @@ -110,7 +112,8 @@ class VirtualScroller { this.scrollingEl.appendChild(this.listEl); this.anchorEl.appendChild(this.scrollingEl); - this.renderItems(); + // If initialRowIndex is < the first window into the list, then just render from the first item + this.renderItems(config.initialRowIndex < this.maxRenderedItems ? 0 : config.initialRowIndex); this.bindDOMListeners(); @@ -245,7 +248,10 @@ class VirtualScroller { return; } - let newStartOffset = offset; + // If specified offset is in the last window into the list then + // render that last window instead of starting at that offset + const lastWindowOffset = this.totalItems - this.maxRenderedItems; + let newStartOffset = offset > lastWindowOffset ? lastWindowOffset : offset; let newEndOffset = offset + this.maxRenderedItems; // If the default count of items to render exceeds the totalItems count // then just render up to the end @@ -372,6 +378,53 @@ class VirtualScroller { newListEl.style.height = `${this.totalItems * (this.itemHeight + this.margin) + this.margin}px`; return newListEl; } + + /** + * Scrolls the provided row index into view. + * @param {number} rowIndex - the index of the row in the overall list + * @return {void} + */ + scrollIntoView(rowIndex) { + if (!this.scrollingEl || rowIndex < 0 || rowIndex >= this.totalItems) { + return; + } + + // See if the list item indexed by `rowIndex` is already present + const foundItem = Array.prototype.slice.call(this.listEl.children).find((listItem) => { + const { bpVsRowIndex } = listItem.dataset; + const parsedRowIndex = parseInt(bpVsRowIndex, 10); + return parsedRowIndex === rowIndex; + }); + + if (foundItem) { + // If it is already present and visible, do nothing, but if not visible + // then scroll it into view + if (!this.isVisible(foundItem)) { + foundItem.scrollIntoView(); + } + } else { + // If it is not present, then adjust the scrollTop so that the list item + // will get rendered. + const topPosition = (this.itemHeight + this.margin) * rowIndex; + this.scrollingEl.scrollTop = topPosition; + } + } + + /** + * Checks to see whether the provided list item element is currently visible + * @param {HTMLElement} listItemEl - the list item elment + * @return {boolean} Returns true if the list item is visible, false otherwise + */ + isVisible(listItemEl) { + if (!this.scrollingEl || !listItemEl) { + return false; + } + + const { scrollTop } = this.scrollingEl; + const { offsetTop } = listItemEl; + + return scrollTop <= offsetTop && offsetTop <= scrollTop + this.containerHeight; + } } export default VirtualScroller; diff --git a/src/lib/__tests__/ThumbnailsSidebar-test.js b/src/lib/__tests__/ThumbnailsSidebar-test.js index c7535643c..7c464a413 100644 --- a/src/lib/__tests__/ThumbnailsSidebar-test.js +++ b/src/lib/__tests__/ThumbnailsSidebar-test.js @@ -1,6 +1,7 @@ /* eslint-disable no-unused-expressions */ import ThumbnailsSidebar from '../ThumbnailsSidebar'; import VirtualScroller from '../VirtualScroller'; +import { CLASS_HIDDEN } from '../constants'; const sandbox = sinon.sandbox.create(); @@ -11,6 +12,7 @@ describe('ThumbnailsSidebar', () => { let page; let virtualScroller; let pagePromise; + let anchorEl; before(() => fixture.setBase('src/lib')); @@ -31,9 +33,11 @@ describe('ThumbnailsSidebar', () => { stubs.getPage = sandbox.stub().returns(pagePromise); stubs.vsInit = sandbox.stub(VirtualScroller.prototype, 'init'); stubs.vsDestroy = sandbox.stub(VirtualScroller.prototype, 'destroy'); + stubs.vsScrollIntoView = sandbox.stub(VirtualScroller.prototype, 'scrollIntoView'); virtualScroller = { - destroy: stubs.vsDestroy + destroy: stubs.vsDestroy, + scrollIntoView: stubs.vsScrollIntoView }; pdfViewer = { @@ -42,7 +46,9 @@ describe('ThumbnailsSidebar', () => { } }; - thumbnailsSidebar = new ThumbnailsSidebar(document.getElementById('test-thumbnails-sidebar'), pdfViewer); + anchorEl = document.getElementById('test-thumbnails-sidebar'); + + thumbnailsSidebar = new ThumbnailsSidebar(anchorEl, pdfViewer); }); afterEach(() => { @@ -335,6 +341,7 @@ describe('ThumbnailsSidebar', () => { beforeEach(() => { stubs.applyCurrentPageSelection = sandbox.stub(thumbnailsSidebar, 'applyCurrentPageSelection'); thumbnailsSidebar.pdfViewer = { pagesCount: 10 }; + thumbnailsSidebar.virtualScroller = virtualScroller; }); const paramaterizedTests = [ @@ -357,6 +364,7 @@ describe('ThumbnailsSidebar', () => { expect(thumbnailsSidebar.currentPage).to.be.equal(3); expect(stubs.applyCurrentPageSelection).to.be.called; + expect(stubs.vsScrollIntoView).to.be.calledWith(2); }); }); @@ -403,4 +411,93 @@ describe('ThumbnailsSidebar', () => { expect(stubs.addClass).to.be.calledOnce; }); }); + + describe('toggle()', () => { + beforeEach(() => { + stubs.isOpen = sandbox.stub(thumbnailsSidebar, 'isOpen'); + stubs.toggleOpen = sandbox.stub(thumbnailsSidebar, 'toggleOpen'); + stubs.toggleClose = sandbox.stub(thumbnailsSidebar, 'toggleClose'); + }); + + it('should do nothing if there is no anchorEl', () => { + thumbnailsSidebar.anchorEl = null; + + thumbnailsSidebar.toggle(); + + expect(stubs.isOpen).not.to.be.called; + expect(stubs.toggleOpen).not.to.be.called; + expect(stubs.toggleClose).not.to.be.called; + + thumbnailsSidebar.anchorEl = anchorEl; + }); + + it('should toggle open if it was closed', () => { + stubs.isOpen.returns(false); + + thumbnailsSidebar.toggle(); + + expect(stubs.isOpen).to.be.called; + expect(stubs.toggleOpen).to.be.called; + expect(stubs.toggleClose).not.to.be.called; + }); + + it('should toggle closed if it was open', () => { + stubs.isOpen.returns(true); + + thumbnailsSidebar.toggle(); + + expect(stubs.isOpen).to.be.called; + expect(stubs.toggleOpen).not.to.be.called; + expect(stubs.toggleClose).to.be.called; + }); + }); + + describe('toggleOpen()', () => { + beforeEach(() => { + stubs.removeClass = sandbox.stub(thumbnailsSidebar.anchorEl.classList, 'remove'); + thumbnailsSidebar.virtualScroller = virtualScroller; + }); + + it('should do nothing if there is no anchorEl', () => { + thumbnailsSidebar.anchorEl = null; + + thumbnailsSidebar.toggleOpen(); + + expect(stubs.removeClass).not.to.be.called; + expect(stubs.vsScrollIntoView).not.to.be.called; + + thumbnailsSidebar.anchorEl = anchorEl; + }); + + it('should remove the hidden class and scroll the page into view', () => { + thumbnailsSidebar.currentPage = 3; + + thumbnailsSidebar.toggleOpen(); + + expect(stubs.removeClass).to.be.calledWith(CLASS_HIDDEN); + expect(stubs.vsScrollIntoView).to.be.calledWith(2); + }); + }); + + describe('toggleClose()', () => { + beforeEach(() => { + stubs.addClass = sandbox.stub(thumbnailsSidebar.anchorEl.classList, 'add'); + }); + + it('should do nothing if there is no anchorEl', () => { + thumbnailsSidebar.anchorEl = null; + + thumbnailsSidebar.toggleClose(); + + expect(stubs.addClass).not.to.be.called; + + thumbnailsSidebar.anchorEl = anchorEl; + }); + + it('should add the hidden class', () => { + thumbnailsSidebar.toggleClose(); + + expect(stubs.addClass).to.be.calledWith(CLASS_HIDDEN); + }); + }); }); diff --git a/src/lib/__tests__/VirtualScroller-test.js b/src/lib/__tests__/VirtualScroller-test.js index 509f73f36..aec3973d4 100644 --- a/src/lib/__tests__/VirtualScroller-test.js +++ b/src/lib/__tests__/VirtualScroller-test.js @@ -97,6 +97,36 @@ describe('VirtualScroller', () => { expect(stubs.onInitHandler).to.be.calledWith(mockListInfo); }); + + it('should call renderItems with the provided initialRowIndex', () => { + stubs.renderItemFn = sandbox.stub(); + stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems'); + + virtualScroller.init({ + totalItems: 10, + itemHeight: 100, + containerHeight: 500, + renderItemFn: stubs.renderItemFn, + initialRowIndex: 50 + }); + + expect(stubs.renderItems).to.be.calledWith(50); + }); + + it('should call renderItems with 0 if initialRowIndex falls within first window', () => { + stubs.renderItemFn = sandbox.stub(); + stubs.renderItems = sandbox.stub(virtualScroller, 'renderItems'); + + virtualScroller.init({ + totalItems: 10, + itemHeight: 100, + containerHeight: 500, + renderItemFn: stubs.renderItemFn, + initialRowIndex: 2 + }); + + expect(stubs.renderItems).to.be.calledWith(0); + }); }); describe('validateRequiredConfig()', () => { @@ -170,6 +200,8 @@ describe('VirtualScroller', () => { curListEl = { appendChild: stubs.appendChild, insertBefore: stubs.insertBefore }; newListEl = {}; virtualScroller.listEl = curListEl; + virtualScroller.maxRenderedItems = 10; + virtualScroller.totalItems = 100; stubs.renderItem = sandbox.stub(virtualScroller, 'renderItem'); stubs.getCurrentListInfo = sandbox.stub(virtualScroller, 'getCurrentListInfo'); @@ -179,8 +211,6 @@ describe('VirtualScroller', () => { }); it('should render the whole range of items (no reuse)', () => { - virtualScroller.maxRenderedItems = 10; - virtualScroller.totalItems = 100; stubs.getCurrentListInfo.returns({ startOffset: -1, endOffset: -1 @@ -193,9 +223,7 @@ describe('VirtualScroller', () => { expect(stubs.insertBefore).not.to.be.called; }); - it('should render the remaining items up to totalItems', () => { - virtualScroller.maxRenderedItems = 10; - virtualScroller.totalItems = 100; + it('should render the last window into the list', () => { stubs.getCurrentListInfo.returns({ startOffset: -1, endOffset: -1 @@ -203,14 +231,12 @@ describe('VirtualScroller', () => { virtualScroller.renderItems(95); expect(stubs.deleteItems).to.be.calledWith(curListEl); - expect(stubs.createItems).to.be.calledWith(newListEl, 95, 99); + expect(stubs.createItems).to.be.calledWith(newListEl, 90, 99); expect(stubs.appendChild).to.be.called; expect(stubs.insertBefore).not.to.be.called; }); it('should render items above the current list', () => { - virtualScroller.maxRenderedItems = 10; - virtualScroller.totalItems = 100; stubs.getCurrentListInfo.returns({ startOffset: 20, endOffset: 30 @@ -430,4 +456,123 @@ describe('VirtualScroller', () => { expect(stubs.appendChild).to.be.calledThrice; }); }); + + describe('scrollIntoView()', () => { + let scrollingEl; + let listEl; + + beforeEach(() => { + scrollingEl = { remove: () => {} }; + + virtualScroller.totalItems = 10; + virtualScroller.itemHeight = 10; + virtualScroller.margin = 0; + virtualScroller.scrollingEl = scrollingEl; + + stubs.isVisible = sandbox.stub(virtualScroller, 'isVisible'); + stubs.scrollIntoView = sandbox.stub(); + + listEl = { + children: [ + { dataset: { bpVsRowIndex: 0 }, scrollIntoView: stubs.scrollIntoView }, + { dataset: { bpVsRowIndex: 1 }, scrollIntoView: stubs.scrollIntoView }, + { dataset: { bpVsRowIndex: 2 }, scrollIntoView: stubs.scrollIntoView } + ] + }; + }); + + it('should do nothing if scrollingEl is falsy', () => { + virtualScroller.scrollingEl = undefined; + + virtualScroller.scrollIntoView(1); + + expect(stubs.isVisible).not.to.be.called; + expect(scrollingEl.scrollTop).to.be.undefined; + }); + + it('should do nothing if rowIndex is < 0', () => { + virtualScroller.scrollIntoView(-1); + + expect(stubs.isVisible).not.to.be.called; + expect(scrollingEl.scrollTop).to.be.undefined; + }); + + it('should do nothing if rowIndex is = totalItems', () => { + virtualScroller.scrollIntoView(10); + + expect(stubs.isVisible).not.to.be.called; + expect(scrollingEl.scrollTop).to.be.undefined; + }); + + it('should do nothing if rowIndex is > totalItems', () => { + virtualScroller.scrollIntoView(11); + + expect(stubs.isVisible).not.to.be.called; + expect(scrollingEl.scrollTop).to.be.undefined; + }); + + it('should set the scroll top if item is not found', () => { + virtualScroller.listEl = listEl; + + virtualScroller.scrollIntoView(8); + + expect(stubs.isVisible).not.to.be.called; + expect(stubs.scrollIntoView).not.to.be.called; + expect(scrollingEl.scrollTop).not.to.be.undefined; + }); + + it('should scroll item into view if found but not visible', () => { + virtualScroller.listEl = listEl; + stubs.isVisible.returns(false); + + virtualScroller.scrollIntoView(1); + + expect(stubs.isVisible).to.be.called; + expect(stubs.scrollIntoView).to.be.called; + expect(scrollingEl.scrollTop).to.be.undefined; + }); + + it('should not scroll if item is found and visible', () => { + virtualScroller.listEl = listEl; + stubs.isVisible.returns(true); + + virtualScroller.scrollIntoView(1); + + expect(stubs.isVisible).to.be.called; + expect(stubs.scrollIntoView).not.to.be.called; + expect(scrollingEl.scrollTop).to.be.undefined; + }); + }); + + describe('isVisible()', () => { + let scrollingEl; + + beforeEach(() => { + scrollingEl = { scrollTop: 100, remove: () => {} }; + virtualScroller.scrollingEl = scrollingEl; + virtualScroller.containerHeight = 100; + }); + + it('should return false if scrollingEl is falsy', () => { + virtualScroller.scrollingEl = false; + + expect(virtualScroller.isVisible({})).to.be.false; + }); + + it('should return false if listItemEl is falsy', () => { + expect(virtualScroller.isVisible()).to.be.false; + }); + + it('should return false if the offsetTop of listItemEl is < scrollTop', () => { + expect(virtualScroller.isVisible({ offsetTop: 50 })).to.be.false; + }); + + it('should return false if the offsetTop of listItemEl is > scrollTop + containerHeight', () => { + expect(virtualScroller.isVisible({ offsetTop: 201 })).to.be.false; + }); + + it('should return true if the offsetTop of listItemEl is >= scrollTop && <= scrollTop + containerHeight', () => { + expect(virtualScroller.isVisible({ offsetTop: 101 })).to.be.true; + }); + }); }); diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 09fb1cf64..92bad4d63 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -1134,6 +1134,10 @@ class DocBaseViewer extends BaseViewer { const { pageNumber } = event; this.pageControls.updateCurrentPage(pageNumber); + if (this.thumbnailsSidebar) { + this.thumbnailsSidebar.setCurrentPage(pageNumber); + } + // We only set cache the current page if 'pagechange' was fired after // preview is loaded - this filters out pagechange events fired by // the viewer's initialization @@ -1326,17 +1330,17 @@ class DocBaseViewer extends BaseViewer { * @return {void} */ toggleThumbnails() { - if (!this.thumbnailsSidebarEl) { + if (!this.thumbnailsSidebar) { return; } - this.thumbnailsSidebarEl.classList.toggle(CLASS_HIDDEN); + this.thumbnailsSidebar.toggle(); const { pagesCount } = this.pdfViewer; let metricName; let eventName; - if (this.thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)) { + if (!this.thumbnailsSidebar.isOpen()) { metricName = USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE; eventName = 'thumbnailsClose'; } else { diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index a153c190d..a356d4b45 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -19,7 +19,6 @@ import { STATUS_SUCCESS, QUERY_PARAM_ENCODING, ENCODING_TYPES, - SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER, SELECTOR_BOX_PREVIEW_CONTENT, CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER } from '../../../constants'; @@ -2210,14 +2209,25 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }); describe('toggleThumbnails()', () => { + let thumbnailsSidebar; + beforeEach(() => { sandbox.stub(docBase, 'resize'); sandbox.stub(docBase, 'emitMetric'); sandbox.stub(docBase, 'emit'); + + stubs.toggleSidebar = sandbox.stub(); + stubs.isSidebarOpen = sandbox.stub(); + + thumbnailsSidebar = { + toggle: stubs.toggleSidebar, + isOpen: stubs.isSidebarOpen, + destroy: () => {} + }; }); it('should do nothing if thumbnails sidebar does not exist', () => { - docBase.thumbnailsSidebarEl = undefined; + docBase.thumbnailsSidebar = undefined; docBase.toggleThumbnails(); @@ -2225,29 +2235,28 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }); it('should toggle open and resize the viewer', () => { - const thumbnailsSidebarEl = document.querySelector(SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER); - docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; + docBase.thumbnailsSidebar = thumbnailsSidebar; docBase.pdfViewer = { pagesCount: 10 }; - expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.true; + stubs.isSidebarOpen.returns(true); docBase.toggleThumbnails(); - expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.false; + expect(stubs.toggleSidebar).to.be.called; + expect(stubs.isSidebarOpen).to.be.called; expect(docBase.resize).to.be.called; expect(docBase.emitMetric).to.be.calledWith({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.OPEN, data: 10 }); expect(docBase.emit).to.be.calledWith('thumbnailsOpen'); }); it('should toggle close and resize the viewer', () => { - const thumbnailsSidebarEl = document.querySelector(SELECTOR_BOX_PREVIEW_THUMBNAILS_CONTAINER); - docBase.thumbnailsSidebarEl = thumbnailsSidebarEl; - thumbnailsSidebarEl.classList.remove(CLASS_HIDDEN); + docBase.thumbnailsSidebar = thumbnailsSidebar; docBase.pdfViewer = { pagesCount: 10 }; - expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.false; + stubs.isSidebarOpen.returns(false); docBase.toggleThumbnails(); - expect(thumbnailsSidebarEl.classList.contains(CLASS_HIDDEN)).to.be.true; + expect(stubs.toggleSidebar).to.be.called; + expect(stubs.isSidebarOpen).to.be.called; expect(docBase.resize).to.be.called; expect(docBase.emitMetric).to.be.calledWith({ name: USER_DOCUMENT_THUMBNAIL_EVENTS.CLOSE, data: 10 }); expect(docBase.emit).to.be.calledWith('thumbnailsClose'); From f07961bfc34eadd7e538cf7371fdc691c2634eee Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Fri, 1 Feb 2019 15:14:57 -0800 Subject: [PATCH 18/29] Chore: fixing BaseViewer-test.js after rebase --- src/lib/viewers/__tests__/BaseViewer-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/viewers/__tests__/BaseViewer-test.js b/src/lib/viewers/__tests__/BaseViewer-test.js index 47dffd057..5543eb08f 100644 --- a/src/lib/viewers/__tests__/BaseViewer-test.js +++ b/src/lib/viewers/__tests__/BaseViewer-test.js @@ -8,6 +8,7 @@ import DownloadReachability from '../../DownloadReachability'; import fullscreen from '../../Fullscreen'; import * as util from '../../util'; import * as icons from '../../icons/icons'; +import * as constants from '../../constants'; import { VIEWER_EVENT, LOAD_METRIC, ERROR_CODE } from '../../events'; import Timer from '../../Timer'; From 240ad385bdecbbbecc07a5f29e1058ad8c2c73da Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Tue, 5 Feb 2019 18:29:33 -0800 Subject: [PATCH 19/29] Chore: adding e2e tests for Thumbnails (#915) --- src/index.html | 8 +- src/lib/VirtualScroller.js | 2 +- src/lib/shell.html | 2 +- src/lib/viewers/doc/DocBaseViewer.js | 1 + .../integration/document/Controls.e2e.test.js | 13 +-- .../document/Thumbnails.e2e.test.js | 99 +++++++++++++++++++ test/integration/sanity/Sanity.e2e.test.js | 3 +- test/support/commands.js | 16 +-- 8 files changed, 125 insertions(+), 19 deletions(-) create mode 100644 test/integration/document/Thumbnails.e2e.test.js diff --git a/src/index.html b/src/index.html index b1f912fe3..85eb465ff 100644 --- a/src/index.html +++ b/src/index.html @@ -57,6 +57,10 @@ + +
+ +
@@ -77,9 +81,6 @@ // Cache it in local storage localStorage.setItem(selector, value) - - // Attempt to load Preview - loadPreview(); } function loadPreview(options) { @@ -102,6 +103,7 @@ // Try to load all properties from storage on page load setProperty('token'); setProperty('fileid'); + loadPreview(); diff --git a/src/lib/VirtualScroller.js b/src/lib/VirtualScroller.js index 60a67b013..117549030 100644 --- a/src/lib/VirtualScroller.js +++ b/src/lib/VirtualScroller.js @@ -250,7 +250,7 @@ class VirtualScroller { // If specified offset is in the last window into the list then // render that last window instead of starting at that offset - const lastWindowOffset = this.totalItems - this.maxRenderedItems; + const lastWindowOffset = Math.max(0, this.totalItems - this.maxRenderedItems); let newStartOffset = offset > lastWindowOffset ? lastWindowOffset : offset; let newEndOffset = offset + this.maxRenderedItems; // If the default count of items to render exceeds the totalItems count diff --git a/src/lib/shell.html b/src/lib/shell.html index ea11e09a4..7f0d58abc 100644 --- a/src/lib/shell.html +++ b/src/lib/shell.html @@ -50,7 +50,7 @@ -
+
-
+
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+ +
+
+
- -
-
-
-
-