From f9f511e9907c28f0f9c199f68b1d7df716797eab Mon Sep 17 00:00:00 2001 From: Jeremy Press Date: Tue, 29 Aug 2017 12:53:45 -0700 Subject: [PATCH] Chore: Refactor pageControls and scroll handling for multi page images (#321) * Chore: Refactor pageControls and scroll handling for multi page images * Chore: fix conflicts, tests, and small changes --- src/lib/Controls.scss | 1 + src/lib/PageControls.js | 170 +++++++---- src/lib/__tests__/PageControls-test.js | 137 ++++++--- src/lib/util.js | 26 ++ src/lib/viewers/doc/DocBaseViewer.js | 24 +- src/lib/viewers/doc/DocumentViewer.js | 3 +- src/lib/viewers/doc/PresentationViewer.js | 3 +- .../doc/__tests__/DocBaseViewer-test.js | 12 +- .../doc/__tests__/DocumentViewer-test.js | 7 +- .../doc/__tests__/PresentationViewer-test.js | 8 +- src/lib/viewers/image/MultiImageViewer.js | 119 +++----- .../image/__tests__/MultiImageViewer-test.js | 277 ++++++++++++++---- 12 files changed, 542 insertions(+), 245 deletions(-) diff --git a/src/lib/Controls.scss b/src/lib/Controls.scss index 5216ce2cc..967adff7f 100644 --- a/src/lib/Controls.scss +++ b/src/lib/Controls.scss @@ -92,6 +92,7 @@ margin: 0; padding: 0; touch-action: manipulation; + user-select: none; vertical-align: middle; } diff --git a/src/lib/PageControls.js b/src/lib/PageControls.js index 1886e8b6f..1f8ea1913 100644 --- a/src/lib/PageControls.js +++ b/src/lib/PageControls.js @@ -22,51 +22,71 @@ const pageNumTemplate = ` `.replace(/>\s*<'); class PageControls extends EventEmitter { + /** @property {Controls} - Controls object */ + controls; + + /** @property {HTMLElement} - Controls element */ + controlsEl; + + /** @property {HTMLElement} - File content element */ + contentEl; + + /** @property {HTMLElement} - Total pages element */ + totalPagesEl; + + /** @property {HTMLElement} - Current page element */ + currentPageEl; + + /** @property {HTMLElement} - Page number input element */ + pageNumInputEl; /** * [constructor] * * @param {HTMLElement} controls - Viewer controls - * @param {Function} previousPage - Previous page handler - * @param {Function} nextPage - Next page handler + * @param {HTMLElement} contentEl - The content element of the file + * @return {Controls} Instance of controls */ - constructor(controls, previousPage, nextPage) { + constructor(controls, contentEl) { super(); this.controls = controls; this.controlsEl = controls.controlsEl; - this.currentPageEl = controls.currentPageEl; - this.pageNumInputEl = controls.pageNumInputEl; - this.previousPage = previousPage; - this.nextPage = nextPage; + + this.contentEl = contentEl; + + this.pageNumInputBlurHandler = this.pageNumInputBlurHandler.bind(this); + this.pageNumInputKeydownHandler = this.pageNumInputKeydownHandler.bind(this); + this.setPreviousPage = this.setPreviousPage.bind(this); + this.showPageNumInput = this.showPageNumInput.bind(this); + this.setNextPage = this.setNextPage.bind(this); } /** - * Adds controls and initializes page number selector. + * Add the page controls * - * @private - * @param {number} pagesCount - Total number of page + * @param {number} currentPageNumber - Current page number + * @param {number} pagesCount - Number of total pages * @return {void} */ - init(pagesCount) { - // Add controls - this.controls.add(__('previous_page'), this.previousPage, `bp-previous-page-icon ${PREV_PAGE}`, ICON_DROP_UP); - this.controls.add(__('enter_page_num'), this.showPageNumInput.bind(this), PAGE_NUM, pageNumTemplate); - this.controls.add(__('next_page'), this.nextPage, `bp-next-page-icon ${NEXT_PAGE}`, ICON_DROP_DOWN); + add(currentPageNumber, pagesCount) { + this.controls.add( + __('previous_page'), + this.setPreviousPage, + `bp-previous-page-icon ${PREV_PAGE}`, + ICON_DROP_UP + ); + this.controls.add(__('enter_page_num'), this.showPageNumInput, PAGE_NUM, pageNumTemplate); + this.controls.add(__('next_page'), this.setNextPage, `bp-next-page-icon ${NEXT_PAGE}`, ICON_DROP_DOWN); - // Initialize page number selector const pageNumEl = this.controlsEl.querySelector(`.${PAGE_NUM}`); - this.pagesCount = pagesCount; - - // Update total page number - const totalPageEl = pageNumEl.querySelector(`.${CONTROLS_TOTAL_PAGES}`); - totalPageEl.textContent = pagesCount; - - // Keep reference to page number input and current page elements + this.totalPagesEl = pageNumEl.querySelector(`.${CONTROLS_TOTAL_PAGES}`); + this.totalPagesEl.textContent = pagesCount; + this.currentPageEl = pageNumEl.querySelector(`.${CONTROLS_CURRENT_PAGE}`); + this.currentPageEl.textContent = currentPageNumber; this.pageNumInputEl = pageNumEl.querySelector(`.${CONTROLS_PAGE_NUM_INPUT_CLASS}`); - this.pageNumInputEl.setAttribute('max', pagesCount); - this.currentPageEl = pageNumEl.querySelector(`.${CONTROLS_CURRENT_PAGE}`); + this.checkPaginationButtons(); } /** @@ -79,13 +99,13 @@ class PageControls extends EventEmitter { // show the input box with the current page number selected within it this.controlsEl.classList.add(SHOW_PAGE_NUM_INPUT_CLASS); - this.pageNumInputEl.value = this.currentPageEl.textContent; + this.pageNumInputEl.value = this.getCurrentPageNumber(); this.pageNumInputEl.focus(); this.pageNumInputEl.select(); // finish input when input is blurred or enter key is pressed - this.pageNumInputEl.addEventListener('blur', this.pageNumInputBlurHandler.bind(this)); - this.pageNumInputEl.addEventListener('keydown', this.pageNumInputKeydownHandler.bind(this)); + this.pageNumInputEl.addEventListener('blur', this.pageNumInputBlurHandler); + this.pageNumInputEl.addEventListener('keydown', this.pageNumInputKeydownHandler); } /** @@ -104,11 +124,9 @@ class PageControls extends EventEmitter { * Disables or enables previous/next pagination buttons depending on * current page number. * - * @param {number} currentPageNum - Current page number - * @param {number} pagesCount - Total number of page * @return {void} */ - checkPaginationButtons(currentPageNum, pagesCount) { + checkPaginationButtons() { const pageNumButtonEl = this.controlsEl.querySelector(`.${PAGE_NUM}`); const previousPageButtonEl = this.controlsEl.querySelector(`.${PREV_PAGE}`); const nextPageButtonEl = this.controlsEl.querySelector(`.${NEXT_PAGE}`); @@ -118,7 +136,7 @@ class PageControls extends EventEmitter { // Disable page number selector if there is only one page or less if (pageNumButtonEl) { - if (pagesCount <= 1 || isSafariFullscreen) { + if (this.getTotalPages() <= 1 || isSafariFullscreen) { pageNumButtonEl.disabled = true; } else { pageNumButtonEl.disabled = false; @@ -127,7 +145,7 @@ class PageControls extends EventEmitter { // Disable previous page if on first page, otherwise enable if (previousPageButtonEl) { - if (currentPageNum === 1) { + if (this.getCurrentPageNumber() === 1) { previousPageButtonEl.disabled = true; } else { previousPageButtonEl.disabled = false; @@ -136,7 +154,7 @@ class PageControls extends EventEmitter { // Disable next page if on last page, otherwise enable if (nextPageButtonEl) { - if (currentPageNum === pagesCount) { + if (this.getCurrentPageNumber() === this.getTotalPages()) { nextPageButtonEl.disabled = true; } else { nextPageButtonEl.disabled = false; @@ -147,30 +165,71 @@ class PageControls extends EventEmitter { /** * Update page number in page control widget. * - * @private - * @param {number} pageNum - Number of page to update to + * @public + * @param {number} pageNumber - Number of page to update to * @return {void} */ - updateCurrentPage(pageNum) { - let truePageNum = pageNum; - - // refine the page number to fall within bounds - if (pageNum > this.pagesCount) { - truePageNum = this.pagesCount; - } else if (pageNum < 1) { - truePageNum = 1; - } - + updateCurrentPage(pageNumber) { if (this.pageNumInputEl) { - this.pageNumInputEl.value = truePageNum; + this.pageNumInputEl.value = pageNumber; } if (this.currentPageEl) { - this.currentPageEl.textContent = truePageNum; + this.setCurrentPageNumber(pageNumber); } - this.currentPageNumber = truePageNum; - this.checkPaginationButtons(this.currentPageNumber, this.pagesCount); + this.checkPaginationButtons(); + } + + /** + * Emits a message to the viewer to decrement the current page. + * + * @private + * @return {void} + */ + setPreviousPage() { + this.emit('pagechange', this.getCurrentPageNumber() - 1); + } + + /** + * Emits a message to the viewer to increment the current page. + * + * @private + * @return {void} + */ + setNextPage() { + this.emit('pagechange', this.getCurrentPageNumber() + 1); + } + + /** + * Gets the page number for the file. + * + * @private + * @return {number} Number of pages + */ + getCurrentPageNumber() { + return parseInt(this.currentPageEl.textContent, 10); + } + + /** + * Sets the number of pages for the file. + * + * @private + * @param {number} pageNumber - Number to set + * @return {number} Number of pages + */ + setCurrentPageNumber(pageNumber) { + this.currentPageEl.textContent = pageNumber; + } + + /** + * Gets the number of pages for the file. + * + * @private + * @return {number} Number of pages + */ + getTotalPages() { + return parseInt(this.totalPagesEl.textContent, 10); } /** @@ -182,10 +241,10 @@ class PageControls extends EventEmitter { */ pageNumInputBlurHandler(event) { const target = event.target; - const pageNum = parseInt(target.value, 10); + const pageNumber = parseInt(target.value, 10); - if (!isNaN(pageNum)) { - this.emit('setpage', pageNum); + if (!isNaN(pageNumber)) { + this.emit('pagechange', pageNumber); } this.hidePageNumInput(); @@ -204,12 +263,13 @@ class PageControls extends EventEmitter { switch (key) { case 'Enter': case 'Tab': + this.contentEl.focus(); // The keycode of the 'next' key on Android Chrome is 9, which maps to 'Tab'. - // this.docEl.focus(); // We normally trigger the blur handler by blurring the input // field, but this doesn't work for IE in fullscreen. For IE, // we blur the page behind the controls - this unfortunately // is an IE-only solution that doesn't work with other browsers + if (Browser.getName() !== 'Explorer') { event.target.blur(); } @@ -220,7 +280,7 @@ class PageControls extends EventEmitter { case 'Escape': this.hidePageNumInput(); - // this.docEl.focus(); + this.contentEl.focus(); event.stopPropagation(); event.preventDefault(); diff --git a/src/lib/__tests__/PageControls-test.js b/src/lib/__tests__/PageControls-test.js index 84b4152fc..5367f7158 100644 --- a/src/lib/__tests__/PageControls-test.js +++ b/src/lib/__tests__/PageControls-test.js @@ -54,20 +54,36 @@ describe('lib/PageControls', () => { }); }); - describe('init()', () => { - it('should initialize the page number selector', () => { - const pagesCount = '5'; + describe('add()', () => { + beforeEach(() => { + stubs.currentPageNumber = 1; + stubs.pagesCount = 10; + stubs.add = sandbox.spy(pageControls.controls, 'add'); + stubs.checkPaginationButtons = sandbox.stub(pageControls, 'checkPaginationButtons'); + }); + + it('should add the page number controls', () => { + pageControls.add(stubs.currentPageNumber, stubs.pagesCount); + + expect(stubs.add).to.be.calledThrice; + }); + + it('should initialize the number of total pages', () => { + pageControls.add(stubs.currentPageNumber, stubs.pagesCount); - pageControls.init(pagesCount); expect(pageControls.controls.buttonRefs.length).equals(3); - expect(pageControls.pagesCount).equals(pagesCount); - expect(pageControls.currentPageEl).to.not.be.undefined; + expect(parseInt(pageControls.totalPagesEl.textContent, 10)).to.equal(stubs.pagesCount); + }); - const totalPageEl = pageControls.controlsEl.querySelector(`.${CONTROLS_TOTAL_PAGES}`); - expect(totalPageEl).to.have.text(pagesCount); + it('should initialize the current page number', () => { + pageControls.add(stubs.currentPageNumber, stubs.pagesCount); - const pageNumInputEl = pageControls.controlsEl.querySelector(`.${CONTROLS_PAGE_NUM_INPUT_CLASS}`); - expect(pageNumInputEl).to.have.attr('max', pagesCount); + expect(parseInt(pageControls.currentPageEl.textContent, 10)).to.equal(stubs.currentPageNumber); + }); + + it('should check the pagination buttons', () => { + pageControls.add(stubs.currentPageNumber, stubs.pagesCount); + expect(stubs.checkPaginationButtons).to.be.called; }); }); @@ -105,7 +121,7 @@ describe('lib/PageControls', () => { describe('checkPaginationButtons()', () => { beforeEach(() => { - pageControls.init(); + pageControls.add(1, 10); stubs.pageNumButtonEl = pageControls.controlsEl.querySelector(`.${PAGE_NUM}`); stubs.previousPageButtonEl = pageControls.controlsEl.querySelector(`.${PREV_PAGE}`); stubs.nextPageButtonEl = pageControls.controlsEl.querySelector(`.${NEXT_PAGE}`); @@ -118,34 +134,41 @@ describe('lib/PageControls', () => { pageControls.checkPaginationButtons(); expect(stubs.pageNumButtonEl.disabled).to.equal(true); - pageControls.checkPaginationButtons(1, 6); - expect(stubs.pageNumButtonEl.disabled).to.equal(true); - - stubs.fullscreen.returns('false'); stubs.browser.returns('Chrome'); - pageControls.checkPaginationButtons(1, 6); + pageControls.checkPaginationButtons(); + expect(stubs.pageNumButtonEl.disabled).to.equal(false); + + stubs.browser.returns('Safari'); + stubs.fullscreen.returns(false); + pageControls.checkPaginationButtons(); expect(stubs.pageNumButtonEl.disabled).to.equal(false); + + pageControls.totalPagesEl.textContent = '1'; + pageControls.checkPaginationButtons(); + expect(stubs.pageNumButtonEl.disabled).to.equal(true); }); it('should disable/enable previous page button el based on current page', () => { - pageControls.checkPaginationButtons(1, 5); + pageControls.checkPaginationButtons(); expect(stubs.previousPageButtonEl.disabled).to.equal(true); - pageControls.checkPaginationButtons(20, 20); + pageControls.setCurrentPageNumber(3); + pageControls.checkPaginationButtons(); expect(stubs.previousPageButtonEl.disabled).to.equal(false); }); it('should disable/enable next page button el based on current page', () => { - pageControls.checkPaginationButtons(20, 20); - expect(stubs.nextPageButtonEl.disabled).to.equal(true); - - pageControls.checkPaginationButtons(1, 20); + pageControls.checkPaginationButtons(); expect(stubs.nextPageButtonEl.disabled).to.equal(false); + + pageControls.setCurrentPageNumber(10); + pageControls.checkPaginationButtons(); + expect(stubs.nextPageButtonEl.disabled).to.equal(true); }); }); describe('updateCurrentPage()', () => { - it('should only update the page to a valid value', () => { + it('should update the page to a value', () => { pageControls.pagesCount = 10; pageControls.pageNumInputEl = { value: 1, @@ -153,20 +176,63 @@ describe('lib/PageControls', () => { }; const checkPaginationButtonsStub = sandbox.stub(pageControls, 'checkPaginationButtons'); - pageControls.updateCurrentPage(-5); - expect(checkPaginationButtonsStub).to.be.called; - expect(pageControls.pageNumInputEl.value).to.equal(1); - - pageControls.updateCurrentPage(25); - expect(checkPaginationButtonsStub).to.be.called; - expect(pageControls.pageNumInputEl.value).to.equal(10); - pageControls.updateCurrentPage(7); expect(checkPaginationButtonsStub).to.be.called; expect(pageControls.pageNumInputEl.value).to.equal(7); }); }); + describe('setPreviousPage()', () => { + it('should emit the page change event for the previous page', () => { + stubs.emit = sandbox.stub(pageControls, 'emit'); + stubs.getCurrentPageNumber = sandbox.stub(pageControls, 'getCurrentPageNumber').returns(3); + + pageControls.setPreviousPage(); + expect(stubs.emit).to.be.calledWith('pagechange', 2); + }); + }); + + describe('setNextPage()', () => { + it('should emit the page change event for the next page', () => { + stubs.emit = sandbox.stub(pageControls, 'emit'); + stubs.getCurrentPageNumber = sandbox.stub(pageControls, 'getCurrentPageNumber').returns(3); + + pageControls.setNextPage(); + expect(stubs.emit).to.be.calledWith('pagechange', 4); + }); + }); + + describe('currentPageNumber', () => { + beforeEach(() => { + pageControls.currentPageEl = { + textContent: '1' + } + }); + + describe('getCurrentPageNumber()', () => { + it('should return the correct page number', () => { + const currPageNum = pageControls.getCurrentPageNumber(); + expect(currPageNum).to.equal(1); + }); + }); + + describe('setCurrentPageNumber()', () => { + it('should set the correct value', () => { + pageControls.setCurrentPageNumber(3); + const currPageNum = pageControls.getCurrentPageNumber(); + expect(currPageNum).to.equal(3); + }); + }); + }) + + + describe('getTotalPages()', () => { + it('should return the total number of pages', () => { + pageControls.add(1 , 10); + expect(pageControls.getTotalPages()).to.equal(10); + }); + }); + describe('pageNumInputBlurHandler()', () => { beforeEach(() => { stubs.event = { @@ -180,7 +246,7 @@ describe('lib/PageControls', () => { it('should hide the page number input and set the page if given valid input', () => { pageControls.pageNumInputBlurHandler(stubs.event); - expect(stubs.emit).to.be.calledWith('setpage', stubs.event.target.value); + expect(stubs.emit).to.be.calledWith('pagechange', stubs.event.target.value); expect(stubs.hidePageNumInputStub).to.be.called; }); @@ -203,13 +269,17 @@ describe('lib/PageControls', () => { blur: sandbox.stub() } }; + pageControls.contentEl = { + focus: sandbox.stub() + } stubs.browser = sandbox.stub(Browser, 'getName').returns('Explorer'); stubs.hidePageNumInput = sandbox.stub(pageControls, 'hidePageNumInput'); }); - it('should focus the doc element if IE and stop default actions on \'enter\'', () => { + it('should focus the doc element and stop default actions on \'enter\'', () => { pageControls.pageNumInputKeydownHandler(stubs.event); expect(stubs.browser).to.be.called; + expect(pageControls.contentEl.focus).to.be.called; expect(stubs.event.stopPropagation).to.be.called; expect(stubs.event.preventDefault).to.be.called; }); @@ -229,6 +299,7 @@ describe('lib/PageControls', () => { pageControls.pageNumInputKeydownHandler(stubs.event); expect(stubs.hidePageNumInput).to.be.called; + expect(pageControls.contentEl.focus).to.be.called; expect(stubs.event.stopPropagation).to.be.called; expect(stubs.event.preventDefault).to.be.called; }); diff --git a/src/lib/util.js b/src/lib/util.js index 49b97c3ab..aaad9b3ba 100644 --- a/src/lib/util.js +++ b/src/lib/util.js @@ -721,3 +721,29 @@ export function removeActivationListener(element, handler) { element.removeEventListener('click', handler); element.removeEventListener('keydown', handler); } + +/** + * Update the page number based on scroll position. Only increment if + * wrapper is scrolled down past at least half of the current page element. + * Only decrement page if wrapper is scrolled up past at least half of the + * previous page element + * + * @public + * @param {number} currentPageNum - The current page + * @param {HTMLElement} currentPageEl - The current page element + * @param {HTMLElement} wrapperEl - the content wrapper element + * @return {number} the resulting page number + */ +export function pageNumberFromScroll(currentPageNum, currentPageEl, wrapperEl) { + const currentScrollTop = wrapperEl.scrollTop; + const currentScrollBottom = wrapperEl.scrollTop + wrapperEl.offsetHeight; + const currentPageMiddleY = currentPageEl.offsetTop + currentPageEl.clientHeight / 2; + + if (currentScrollTop > currentPageMiddleY) { + return currentPageNum + 1; + } else if (currentScrollBottom < currentPageMiddleY) { + return currentPageNum - 1; + } + + return currentPageNum; +} diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index a96419b36..03109df44 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -87,6 +87,10 @@ class DocBaseViewer extends BaseViewer { // Clean up print blob this.printBlob = null; + if (this.pageControls) { + this.pageControls.removeListener('pagechange', this.setPage); + } + if (this.controls && typeof this.controls.destroy === 'function') { this.controls.destroy(); } @@ -302,15 +306,15 @@ class DocBaseViewer extends BaseViewer { /** * Go to specified page * - * @param {number} pageNum - Page to navigate to + * @param {number} pageNumber - Page to navigate to * @return {void} */ - setPage(pageNum) { - if (pageNum < 1 || pageNum > this.pdfViewer.pagesCount) { + setPage(pageNumber) { + if (pageNumber < 1 || pageNumber > this.pdfViewer.pagesCount) { return; } - this.pdfViewer.currentPageNumber = pageNum; + this.pdfViewer.currentPageNumber = pageNumber; this.cachePage(this.pdfViewer.currentPageNumber); } @@ -696,7 +700,8 @@ class DocBaseViewer extends BaseViewer { */ loadUI() { this.controls = new Controls(this.containerEl); - this.pageControls = new PageControls(this.controls, this.previousPage, this.nextPage); + this.pageControls = new PageControls(this.controls, this.docEl); + this.pageControls.addListener('pagechange', this.setPage); this.bindControlListeners(); } @@ -787,7 +792,6 @@ class DocBaseViewer extends BaseViewer { this.pdfViewer.currentScaleValue = 'auto'; this.loadUI(); - this.pageControls.checkPaginationButtons(this.pdfViewer.currentPageNumber, this.pdfViewer.pagesCount); // Set current page to previously opened page or first page this.setPage(this.getCachedPage()); @@ -846,17 +850,17 @@ class DocBaseViewer extends BaseViewer { * @return {void} */ pagechangeHandler(event) { - const pageNum = event.pageNumber; - this.pageControls.updateCurrentPage(pageNum); + const pageNumber = event.pageNumber; + this.pageControls.updateCurrentPage(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 if (this.loaded) { - this.cachePage(pageNum); + this.cachePage(pageNumber); } - this.emit('pagefocus', pageNum); + this.emit('pagefocus', pageNumber); } /** diff --git a/src/lib/viewers/doc/DocumentViewer.js b/src/lib/viewers/doc/DocumentViewer.js index 21537f4bb..4c6366320 100644 --- a/src/lib/viewers/doc/DocumentViewer.js +++ b/src/lib/viewers/doc/DocumentViewer.js @@ -100,8 +100,7 @@ class DocumentViewer extends DocBaseViewer { 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.init(this.pdfViewer.pagesCount); - this.pageControls.addListener('setpage', this.setPage); + this.pageControls.add(this.pdfViewer.currentPageNumber, this.pdfViewer.pagesCount); this.controls.add( __('enter_fullscreen'), diff --git a/src/lib/viewers/doc/PresentationViewer.js b/src/lib/viewers/doc/PresentationViewer.js index 0f6bdfa2c..59e4438fe 100644 --- a/src/lib/viewers/doc/PresentationViewer.js +++ b/src/lib/viewers/doc/PresentationViewer.js @@ -190,8 +190,7 @@ class PresentationViewer extends DocBaseViewer { 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.init(this.pdfViewer.pagesCount); - this.pageControls.addListener('setpage', this.setPage); + this.pageControls.add(this.pdfViewer.currentPageNumber, this.pdfViewer.pagesCount); this.controls.add( __('enter_fullscreen'), diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 0608a041b..5c49891f5 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -1039,6 +1039,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { docBase.loadUI(); expect(bindControlListenersStub).to.be.called; expect(docBase.controls instanceof Controls).to.be.true; + expect(docBase.pageControls instanceof PageControls).to.be.true; + expect(docBase.pageControls.contentEl).to.equal(docBase.docEl); }); }); @@ -1142,11 +1144,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { stubs.getCachedPage = sandbox.stub(docBase, 'getCachedPage'); stubs.emit = sandbox.stub(docBase, 'emit'); stubs.setupPages = sandbox.stub(docBase, 'setupPageIds'); - - docBase.pageControls = { - checkPaginationButtons: sandbox.stub() - }; - stubs.checkPaginationButtons = docBase.pageControls.checkPaginationButtons; }); it('should load UI, check the pagination buttons, set the page, and make document scrollable', () => { @@ -1156,11 +1153,9 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { docBase.pagesinitHandler(); expect(stubs.loadUI).to.be.called; - expect(stubs.checkPaginationButtons).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.checkPaginationButtons).to.be.called; }); it('should broadcast that the preview is loaded if it hasn\'t already', () => { @@ -1217,7 +1212,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { pageCount: 1 }; docBase.pageControls = { - updateCurrentPage: sandbox.stub() + updateCurrentPage: sandbox.stub(), + removeListener: sandbox.stub() }; stubs.updateCurrentPage = docBase.pageControls.updateCurrentPage; }); diff --git a/src/lib/viewers/doc/__tests__/DocumentViewer-test.js b/src/lib/viewers/doc/__tests__/DocumentViewer-test.js index a457a7ae0..c252d9f77 100644 --- a/src/lib/viewers/doc/__tests__/DocumentViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocumentViewer-test.js @@ -160,8 +160,8 @@ describe('lib/viewers/doc/DocumentViewer', () => { }; doc.pageControls = { - init: sandbox.stub(), - addListener: sandbox.stub() + add: sandbox.stub(), + removeListener: sandbox.stub() }; }); @@ -175,8 +175,7 @@ describe('lib/viewers/doc/DocumentViewer', () => { ); expect(doc.controls.add).to.be.calledWith(__('zoom_in'), doc.zoomIn, 'bp-doc-zoom-in-icon', ICON_ZOOM_IN); - expect(doc.pageControls.init).to.be.called; - expect(doc.pageControls.addListener).to.be.calledWith('setpage', sinon.match.func); + expect(doc.pageControls.add).to.be.called; expect(doc.controls.add).to.be.calledWith( __('enter_fullscreen'), diff --git a/src/lib/viewers/doc/__tests__/PresentationViewer-test.js b/src/lib/viewers/doc/__tests__/PresentationViewer-test.js index 8a572a2d8..489091bfc 100644 --- a/src/lib/viewers/doc/__tests__/PresentationViewer-test.js +++ b/src/lib/viewers/doc/__tests__/PresentationViewer-test.js @@ -302,12 +302,13 @@ describe('lib/viewers/doc/PresentationViewer', () => { beforeEach(() => { presentation.pdfViewer = { pagesCount: 4, + currentPageNumber: 1, cleanup: sandbox.stub() }; presentation.pageControls = { - init: sandbox.stub(), - addListener: sandbox.stub() + add: sandbox.stub(), + removeListener: sandbox.stub() }; }); @@ -326,8 +327,7 @@ describe('lib/viewers/doc/PresentationViewer', () => { ICON_ZOOM_IN ); - expect(presentation.pageControls.init).to.be.called; - expect(presentation.pageControls.addListener).to.be.calledWith('setpage', sinon.match.func); + expect(presentation.pageControls.add).to.be.calledWith(1, 4); expect(presentation.controls.add).to.be.calledWith( __('enter_fullscreen'), diff --git a/src/lib/viewers/image/MultiImageViewer.js b/src/lib/viewers/image/MultiImageViewer.js index 741619a09..79870d648 100644 --- a/src/lib/viewers/image/MultiImageViewer.js +++ b/src/lib/viewers/image/MultiImageViewer.js @@ -4,6 +4,7 @@ import PageControls from '../../PageControls'; import './MultiImage.scss'; import { ICON_FILE_IMAGE, ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT } from '../../icons/icons'; import { CLASS_INVISIBLE } from '../../constants'; +import { pageNumberFromScroll } from '../../util'; const PADDING_BUFFER = 100; const CSS_CLASS_IMAGE = 'bp-images'; @@ -46,6 +47,10 @@ class MultiImageViewer extends ImageBaseViewer { }); } + if (this.pageControls) { + this.pageControls.removeListener('pagechange', this.setPage); + } + super.destroy(); } @@ -194,21 +199,19 @@ class MultiImageViewer extends ImageBaseViewer { */ loadUI() { super.loadUI(); - this.pageControls.checkPaginationButtons(this.currentPageNumber, this.pagesCount); + this.pageControls = new PageControls(this.controls, this.wrapperEl); + this.bindPageControlListeners(); } /** - * Binds listeners for document controls. Overridden. + * Binds listeners for the page and the rest of the document controls. * * @protected * @return {void} */ - bindControlListeners() { - super.bindControlListeners(); - - this.pageControls = new PageControls(this.controls, this.previousPage, this.nextPage); - this.pageControls.init(this.pagesCount); - this.pageControls.addListener('setpage', this.setPage); + bindPageControlListeners() { + this.pageControls.add(this.currentPageNumber, this.pagesCount); + this.pageControls.addListener('pagechange', this.setPage); this.controls.add( __('enter_fullscreen'), @@ -268,107 +271,79 @@ class MultiImageViewer extends ImageBaseViewer { /** * Go to specified page * - * @param {number} pageNum - Page to navigate to + * @param {number} pageNumber - Page to navigate to * @return {void} */ - setPage(pageNum) { - if (pageNum < 1 || pageNum > this.pagesCount) { + setPage(pageNumber) { + if (!this.isValidPageChange(pageNumber)) { return; } - this.currentPageNumber = pageNum; - this.singleImageEls[pageNum - 1].scrollIntoView(); + this.singleImageEls[pageNumber - 1].scrollIntoView(); + this.updateCurrentPage(pageNumber); } /** - * Handles scroll event in the wrapper element + * Updates the current page * - * @private + * @param {number} pageNumber - Page to set * @return {void} */ - scrollHandler() { - if (this.scrollCheckHandler) { + + updateCurrentPage(pageNumber) { + if (!this.isValidPageChange(pageNumber)) { return; } - if (!this.scrollState) { - const currentPageEl = this.singleImageEls[this.currentPageNumber - 1]; - this.scrollState = { - down: false, - lastY: currentPageEl.scrollTop - }; - } + this.currentPageNumber = pageNumber; + this.pageControls.updateCurrentPage(pageNumber); - const imageScrollHandler = this.isSingleImageElScrolled.bind(this); - this.scrollCheckHandler = window.requestAnimationFrame(imageScrollHandler); + this.emit('pagefocus', { + pageNumber + }); } /** - * Updates page number if the single image has been scrolled past + * Determines if the requested page change is valid * * @private + * @param {number} pageNumber - Requested page number * @return {void} */ - isSingleImageElScrolled() { - this.scrollCheckHandler = null; - const currentY = this.wrapperEl.scrollTop; - const lastY = this.scrollState.lastY; - - if (currentY !== lastY) { - this.scrollState.isScrollingDown = currentY > lastY; - } - this.scrollState.lastY = currentY; - this.updatePageChange(); + isValidPageChange(pageNumber) { + return pageNumber >= 1 && pageNumber <= this.pagesCount && pageNumber !== this.currentPageNumber; } /** - * Updates page number in the page controls + * Handles scroll event in the wrapper element * * @private - * @param {number} pageNum - Page just navigated to * @return {void} */ - pagechangeHandler(pageNum) { - this.currentPageNumber = pageNum; - this.pageControls.updateCurrentPage(pageNum); - this.emit('pagefocus', this.currentPageNumber); + scrollHandler() { + if (this.scrollCheckHandler) { + return; + } + + const imageScrollHandler = this.handlePageChangeFromScroll; + this.scrollCheckHandler = window.requestAnimationFrame(imageScrollHandler); } /** - * Update the page number based on scroll direction. Only increment if - * wrapper is scrolled down past at least half of the current page element. - * Only decrement page if wrapper is scrolled up past at least half of the - * previous page element + * Handles page changes due to scrolling * * @private * @return {void} */ - updatePageChange() { - let pageNum = this.currentPageNumber; - const currentPageEl = this.singleImageEls[this.currentPageNumber - 1]; - const wrapperScrollOffset = this.scrollState.lastY; - const currentPageMiddleY = currentPageEl.offsetTop + currentPageEl.clientHeight / 2; - const isScrolledToBottom = wrapperScrollOffset + this.wrapperEl.clientHeight >= this.wrapperEl.scrollHeight; - - if ( - this.scrollState.isScrollingDown && - currentPageEl.nextSibling && - (wrapperScrollOffset > currentPageMiddleY || isScrolledToBottom) - ) { - // Increment page - const nextPage = currentPageEl.nextSibling; - pageNum = parseInt(nextPage.dataset.pageNumber, 10); - } else if (!this.scrollState.isScrollingDown && currentPageEl.previousSibling) { - const prevPage = currentPageEl.previousSibling; - const prevPageMiddleY = prevPage.offsetTop + prevPage.clientHeight / 2; - - // Decrement page - if (prevPageMiddleY > wrapperScrollOffset) { - pageNum = parseInt(prevPage.dataset.pageNumber, 10); - } - } + handlePageChangeFromScroll() { + const pageChange = pageNumberFromScroll( + this.currentPageNumber, + this.singleImageEls[this.currentPageNumber - 1], + this.wrapperEl + ); - this.pagechangeHandler(pageNum); + this.updateCurrentPage(pageChange); + this.scrollCheckHandler = null; } } diff --git a/src/lib/viewers/image/__tests__/MultiImageViewer-test.js b/src/lib/viewers/image/__tests__/MultiImageViewer-test.js index 3889e756c..ebfffffe3 100644 --- a/src/lib/viewers/image/__tests__/MultiImageViewer-test.js +++ b/src/lib/viewers/image/__tests__/MultiImageViewer-test.js @@ -1,8 +1,11 @@ /* eslint-disable no-unused-expressions */ import MultiImageViewer from '../MultiImageViewer'; +import PageControls from '../../../PageControls'; import fullscreen from '../../../Fullscreen'; import BaseViewer from '../../BaseViewer'; import Browser from '../../../Browser'; +import * as util from '../../../util'; +import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT } from '../../../icons/icons'; const CLASS_INVISIBLE = 'bp-is-invisible'; @@ -148,6 +151,34 @@ describe('lib/viewers/image/MultiImageViewer', () => { }); }); + describe('constructImageUrls()', () => { + it('should remove both the new and old form of asset path', () => { + const firstURL = 'file/100/content/1.png'; + const result = multiImage.constructImageUrls('file/100/content/{page}.png'); + + expect(result[0]).to.equal(firstURL); + + multiImage.options = { + viewerAsset: '{asset_path}', + viewer: { + ASSET: '{page}.png' + }, + representation: { + metadata: { + pages: 3 + } + } + }; + const result2 = multiImage.constructImageUrls('file/100/content/{asset_path}'); + expect(result2[0]).to.equal(firstURL); + }); + + it('should create a URL for each page', () => { + const result = multiImage.constructImageUrls('file/100/content/{page}.png'); + expect(result.length).to.equal(3); + }); + }); + describe('setupImageEls()', () => { beforeEach(() => { multiImage.setup(); @@ -192,34 +223,6 @@ describe('lib/viewers/image/MultiImageViewer', () => { }); }); - describe('constructImageUrls()', () => { - it('should remove both the new and old form of asset path', () => { - const firstURL = 'file/100/content/1.png'; - const result = multiImage.constructImageUrls('file/100/content/{page}.png'); - - expect(result[0]).to.equal(firstURL); - - multiImage.options = { - viewerAsset: '{asset_path}', - viewer: { - ASSET: '{page}.png' - }, - representation: { - metadata: { - pages: 3 - } - } - }; - const result2 = multiImage.constructImageUrls('file/100/content/{asset_path}'); - expect(result2[0]).to.equal(firstURL); - }); - - it('should create a URL for each page', () => { - const result = multiImage.constructImageUrls('file/100/content/{page}.png'); - expect(result.length).to.equal(3); - }); - }); - describe('updatePannability()', () => { beforeEach(() => { stubs.updateCursor = sandbox.stub(multiImage, 'updateCursor'); @@ -291,16 +294,67 @@ describe('lib/viewers/image/MultiImageViewer', () => { }); }); + describe('setScale()', () => { + it('should set the scale relative to the size of the first image dimensions', () => { + multiImage.singleImageEls = [ + { + naturalWidth: 1024, + naturalHeight: 1024 + }, + { + src: 'www.NotTheRightImage.net' + } + ]; + sandbox.stub(multiImage, 'emit'); + + multiImage.setScale(512, 512); + expect(multiImage.emit).to.be.calledWith('scale', { scale: 0.5 }); + }); + }); + + describe('loadUI()', () => { - it('should create controls and add control buttons for fullscreen', () => { - multiImage.setup(); + it('should create page controls and bind the page control listeners', () => { + stubs.bindPageControlListeners = sandbox.stub(multiImage, 'bindPageControlListeners') + multiImage.loadUI(); + expect(multiImage.pageControls instanceof PageControls).to.be.true; + expect(multiImage.pageControls.contentEl).to.equal(multiImage.wrapperEl); + expect(stubs.bindPageControlListeners).to.be.called; + }); + }); + + describe('bindPageControlListeners', () => { + beforeEach(() => { + multiImage.currentPageNumber = 1; + multiImage.pagesCount = 10; + multiImage.pageControls = { + add: sandbox.stub(), + addListener: sandbox.stub() + } + + multiImage.controls = { + add: sandbox.stub() + } + }); - expect(multiImage.controls).to.not.be.undefined; - expect(multiImage.controls.buttonRefs.length).to.equal(7); + it('should add the page controls and bind the pagechange listener', () => { + multiImage.bindPageControlListeners(); + + expect(multiImage.pageControls.add).to.be.calledWith(multiImage.currentPageNumber, multiImage.pagesCount); + expect(multiImage.pageControls.addListener).to.be.calledWith('pagechange', multiImage.setPage); }); + + it('should finish binding the document controls', () => { + multiImage.bindPageControlListeners(); + + expect(multiImage.controls.add).to.be.calledWith(__('enter_fullscreen'), multiImage.toggleFullscreen, 'bp-enter-fullscreen-icon', ICON_FULLSCREEN_IN); + expect(multiImage.controls.add).to.be.calledWith(__('exit_fullscreen'), multiImage.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); + }); + }); + describe('bindImageListeners', () => { beforeEach(() => { multiImage.singleImageEls = { @@ -347,40 +401,153 @@ describe('lib/viewers/image/MultiImageViewer', () => { }); }); - describe('setScale()', () => { - it('should set the scale relative to the size of the first image dimensions', () => { - multiImage.singleImageEls = [ - { - naturalWidth: 1024, - naturalHeight: 1024 - }, - { - src: 'www.NotTheRightImage.net' - } - ]; - sandbox.stub(multiImage, 'emit'); - - multiImage.setScale(512, 512); - expect(multiImage.emit).to.be.calledWith('scale', { scale: 0.5 }); - }); - }); - describe('setPage()', () => { - it('should scroll to the current page', () => { + beforeEach(() => { multiImage.singleImageEls = { 1: stubs.singleImageEl, 2: stubs.singleImageEl, 3: stubs.singleImageEl }; sandbox.stub(multiImage, 'emit'); + stubs.isValidPageChange = sandbox.stub(multiImage, 'isValidPageChange'); + stubs.updateCurrentPage = sandbox.stub(multiImage, 'updateCurrentPage') + }) + + it('should do nothing if the page change is invalid', () => { + multiImage.setPage(-2); + expect(multiImage.singleImageEls[2].scrollIntoView).to.not.be.called; + }); + + it('should scroll the set page into view', () => { + stubs.isValidPageChange.returns(true); multiImage.setPage(2); - expect(multiImage.currentPageNumber).equals(2); + expect(stubs.singleImageEl.scrollIntoView).to.be.called; }); - it('should not do anything if setting an invalid page', () => { - multiImage.setPage(-1); - expect(multiImage.currentPageNumber).to.be.undefined; + it('should update the current page number', () => { + stubs.isValidPageChange.returns(true); + + multiImage.setPage(2); + expect(stubs.updateCurrentPage).to.be.calledWith(2); + }); + }); + + describe('updateCurrentPage()', () => { + beforeEach(() => { + stubs.isValidPageChange = sandbox.stub(multiImage, 'isValidPageChange'); + multiImage.pageControls = { + updateCurrentPage: sandbox.stub() + } + + stubs.emit = sandbox.stub(multiImage, 'emit'); + multiImage.currentPageNumber = 1 + }); + + it('should do nothing if the requested page change is invalid', () => { + stubs.isValidPageChange.returns(false); + + multiImage.updateCurrentPage(3); + expect(multiImage.currentPageNumber).to.equal(1); + }); + + it('should set the current page number and update the page controls', () => { + stubs.isValidPageChange.returns(true); + + multiImage.updateCurrentPage(3); + expect(multiImage.currentPageNumber).to.equal(3); + expect(multiImage.pageControls.updateCurrentPage).to.be.calledWith(3); + }); + + it('should emit the pagefocus event', () => { + stubs.isValidPageChange.returns(true); + + multiImage.updateCurrentPage(3); + expect(stubs.emit).to.be.calledWith('pagefocus', { pageNumber: 3 }); + }); + }); + + describe('isValidPageChange()', () => { + beforeEach(() => { + multiImage.pagesCount = 10; + multiImage.currentPageNumber = 3; + }); + + it('should return false if the page number is less thatn one', () => { + const result = multiImage.isValidPageChange(0); + expect(result).to.be.false; + }); + + it('should return false if the page number is greater than the number of pages', () => { + const result = multiImage.isValidPageChange(11); + expect(result).to.be.false; + }); + + it('should return false if the page number is the same as the current page number', () => { + let result = multiImage.isValidPageChange(3); + expect(result).to.be.false; + }); + + it('should return true if the page number is in the range of valid pages', () => { + let result = multiImage.isValidPageChange(10); + expect(result).to.be.true; + + result = multiImage.isValidPageChange(1); + expect(result).to.be.true; + + result = multiImage.isValidPageChange(5); + expect(result).to.be.true; + }); + }); + + describe('scrollHandler()', () => { + beforeEach(() => { + stubs.requestAnimationFrame = sandbox.stub(window, 'requestAnimationFrame'); + }) + + it('should do nothing if the scroll check handler already exists', () => { + multiImage.scrollCheckHandler = true; + + multiImage.scrollHandler(); + expect(stubs.requestAnimationFrame).to.not.be.called; + }); + + it('should reqeust an animation frame to handle page changes from scroll', () => { + multiImage.scrollCheckHandler = undefined; + + multiImage.scrollHandler(); + expect(stubs.requestAnimationFrame).to.be.calledWith(multiImage.handlePageChangeFromScroll); + }); + }); + + describe('handlePageChangeFromScroll()', () => { + beforeEach(() => { + stubs.pageNumberFromScroll = sandbox.stub(util, 'pageNumberFromScroll').returns(1); + stubs.updateCurrentPage = sandbox.stub(multiImage, 'updateCurrentPage'); + multiImage.currentPageNumber = 1; + multiImage.singleImageEls = [document.createElement('div')]; + stubs.singleImageEls = multiImage.singleImageEls; + + multiImage.wrapperEl = document.createElement('div'); + stubs.wrapperEl = multiImage.wrapperEl; + + }) + + it('should determine the current page number based on scroll', () => { + multiImage.handlePageChangeFromScroll(); + expect(stubs.pageNumberFromScroll).to.be.calledWith(1, stubs.singleImageEls[0], stubs.wrapperEl); + }); + + it('should attempt to update the current page number', () => { + multiImage.handlePageChangeFromScroll(); + expect(stubs.updateCurrentPage).to.be.called; + }); + + it('reset the scroll check handler', () => { + multiImage.scrollCheckHandler = true; + + multiImage.handlePageChangeFromScroll(); + expect(multiImage.scrollCheckHandler).to.equal(null); }); }); });