Skip to content

Commit

Permalink
Merge pull request mozilla#9832 from Snuffleupagus/scrollMode-fixes
Browse files Browse the repository at this point in the history 
Fix a number of regressions/inefficiencies introduced by adding Scroll/Spread modes to the viewer (PR 9208 follow-up)
  • Loading branch information
@timvandermeij
timvandermeij authored Jun 23, 2018
2 parents 7d4149c + 549272a commit f853042
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 48 deletions.
53 changes: 24 additions & 29 deletions web/base_viewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ class BaseViewer {
if (this.removePageBorders) {
this.viewer.classList.add('removePageBorders');
}
this._updateScrollModeClasses();
if (this.scrollMode !== ScrollMode.VERTICAL) {
this._updateScrollModeClasses();
}
}

get pagesCount() {
Expand Down Expand Up @@ -595,11 +597,11 @@ class BaseViewer {
if (!currentPage) {
return;
}
let hPadding = (this.isInPresentationMode || this.removePageBorders) ?
0 : SCROLLBAR_PADDING;
let vPadding = (this.isInPresentationMode || this.removePageBorders) ?
0 : VERTICAL_PADDING;
if (this.scrollMode === ScrollMode.HORIZONTAL) {
const noPadding = (this.isInPresentationMode || this.removePageBorders);
let hPadding = noPadding ? 0 : SCROLLBAR_PADDING;
let vPadding = noPadding ? 0 : VERTICAL_PADDING;

if (!noPadding && this._isScrollModeHorizontal) {
const temp = hPadding;
hPadding = vPadding;
vPadding = temp;
Expand Down Expand Up @@ -832,6 +834,10 @@ class BaseViewer {
this.container.focus();
}

get _isScrollModeHorizontal() {
throw new Error('Not implemented: _isScrollModeHorizontal');
}

get isInPresentationMode() {
return this.presentationModeState === PresentationModeState.FULLSCREEN;
}
Expand Down Expand Up @@ -904,8 +910,8 @@ class BaseViewer {

forceRendering(currentlyVisiblePages) {
let visiblePages = currentlyVisiblePages || this._getVisiblePages();
let scrollAhead = this.scrollMode === ScrollMode.HORIZONTAL ?
this.scroll.right : this.scroll.down;
let scrollAhead = (this._isScrollModeHorizontal ?
this.scroll.right : this.scroll.down);
let pageView = this.renderingQueue.getHighestPriority(visiblePages,
this._pages,
scrollAhead);
Expand Down Expand Up @@ -1018,37 +1024,26 @@ class BaseViewer {
}

setScrollMode(mode) {
if (mode !== this.scrollMode) {
this.scrollMode = mode;
this._updateScrollModeClasses();
this.eventBus.dispatch('scrollmodechanged', { mode, });
const pageNumber = this._currentPageNumber;
// Non-numeric scale modes can be sensitive to the scroll orientation.
// Call this before re-scrolling to the current page, to ensure that any
// changes in scale don't move the current page.
if (isNaN(this._currentScaleValue)) {
this._setScale(this._currentScaleValue, this.isInPresentationMode);
}
this.scrollPageIntoView({ pageNumber, });
this.update();
if (!Number.isInteger(mode) || !Object.values(ScrollMode).includes(mode)) {
throw new Error(`Invalid scroll mode: ${mode}`);
}
this.scrollMode = mode;
}

_updateScrollModeClasses() {
const mode = this.scrollMode, { classList, } = this.viewer;
classList.toggle('scrollHorizontal', mode === ScrollMode.HORIZONTAL);
classList.toggle('scrollWrapped', mode === ScrollMode.WRAPPED);
// No-op in the base class.
}

setSpreadMode(mode) {
if (mode !== this.spreadMode) {
this.spreadMode = mode;
this.eventBus.dispatch('spreadmodechanged', { mode, });
this._regroupSpreads();
if (!Number.isInteger(mode) || !Object.values(SpreadMode).includes(mode)) {
throw new Error(`Invalid spread mode: ${mode}`);
}
this.spreadMode = mode;
}

_regroupSpreads() {}
_regroupSpreads() {
// No-op in the base class.
}
}

export {
Expand Down
5 changes: 5 additions & 0 deletions web/pdf_single_page_viewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ class PDFSinglePageViewer extends BaseViewer {
location: this._location,
});
}

get _isScrollModeHorizontal() {
// The Scroll/Spread modes are never used in `PDFSinglePageViewer`.
return shadow(this, '_isScrollModeHorizontal', false);
}
}

export {
Expand Down
72 changes: 65 additions & 7 deletions web/pdf_viewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class PDFViewer extends BaseViewer {
}

_scrollIntoView({ pageDiv, pageSpot = null, }) {
if (!pageSpot) {
if (!pageSpot && !this.isInPresentationMode) {
const left = pageDiv.offsetLeft + pageDiv.clientLeft;
const right = left + pageDiv.clientWidth;
const { scrollLeft, clientWidth, } = this.container;
Expand Down Expand Up @@ -87,14 +87,72 @@ class PDFViewer extends BaseViewer {
});
}

get _isScrollModeHorizontal() {
// Used to ensure that pre-rendering of the next/previous page works
// correctly, since Scroll/Spread modes are ignored in Presentation Mode.
return (this.isInPresentationMode ?
false : this.scrollMode === ScrollMode.HORIZONTAL);
}

setScrollMode(mode) {
if (mode === this.scrollMode) {
return;
}
super.setScrollMode(mode);

this.eventBus.dispatch('scrollmodechanged', { mode, });
this._updateScrollModeClasses();

if (!this.pdfDocument) {
return;
}
const pageNumber = this._currentPageNumber;
// Non-numeric scale modes can be sensitive to the scroll orientation.
// Call this before re-scrolling to the current page, to ensure that any
// changes in scale don't move the current page.
if (isNaN(this._currentScaleValue)) {
this._setScale(this._currentScaleValue, true);
}
this.scrollPageIntoView({ pageNumber, });
this.update();
}

_updateScrollModeClasses() {
const { scrollMode, viewer, } = this;

if (scrollMode === ScrollMode.HORIZONTAL) {
viewer.classList.add('scrollHorizontal');
} else {
viewer.classList.remove('scrollHorizontal');
}
if (scrollMode === ScrollMode.WRAPPED) {
viewer.classList.add('scrollWrapped');
} else {
viewer.classList.remove('scrollWrapped');
}
}

setSpreadMode(mode) {
if (mode === this.spreadMode) {
return;
}
super.setSpreadMode(mode);

this.eventBus.dispatch('spreadmodechanged', { mode, });
this._regroupSpreads();
}

_regroupSpreads() {
const container = this._setDocumentViewerElement, pages = this._pages;
while (container.firstChild) {
container.firstChild.remove();
if (!this.pdfDocument) {
return;
}
const viewer = this.viewer, pages = this._pages;
// Temporarily remove all the pages from the DOM.
viewer.textContent = '';

if (this.spreadMode === SpreadMode.NONE) {
for (let i = 0, iMax = pages.length; i < iMax; ++i) {
container.appendChild(pages[i].div);
viewer.appendChild(pages[i].div);
}
} else {
const parity = this.spreadMode - 1;
Expand All @@ -103,10 +161,10 @@ class PDFViewer extends BaseViewer {
if (spread === null) {
spread = document.createElement('div');
spread.className = 'spread';
container.appendChild(spread);
viewer.appendChild(spread);
} else if (i % 2 === parity) {
spread = spread.cloneNode(false);
container.appendChild(spread);
viewer.appendChild(spread);
}
spread.appendChild(pages[i].div);
}
Expand Down
42 changes: 30 additions & 12 deletions web/secondary_toolbar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,23 +190,41 @@ class SecondaryToolbar {

_bindScrollModeListener(buttons) {
this.eventBus.on('scrollmodechanged', function(evt) {
buttons.scrollVerticalButton.classList.toggle('toggled',
evt.mode === ScrollMode.VERTICAL);
buttons.scrollHorizontalButton.classList.toggle('toggled',
evt.mode === ScrollMode.HORIZONTAL);
buttons.scrollWrappedButton.classList.toggle('toggled',
evt.mode === ScrollMode.WRAPPED);
buttons.scrollVerticalButton.classList.remove('toggled');
buttons.scrollHorizontalButton.classList.remove('toggled');
buttons.scrollWrappedButton.classList.remove('toggled');

switch (evt.mode) {
case ScrollMode.VERTICAL:
buttons.scrollVerticalButton.classList.add('toggled');
break;
case ScrollMode.HORIZONTAL:
buttons.scrollHorizontalButton.classList.add('toggled');
break;
case ScrollMode.WRAPPED:
buttons.scrollWrappedButton.classList.add('toggled');
break;
}
});
}

_bindSpreadModeListener(buttons) {
this.eventBus.on('spreadmodechanged', function(evt) {
buttons.spreadNoneButton.classList.toggle('toggled',
evt.mode === SpreadMode.NONE);
buttons.spreadOddButton.classList.toggle('toggled',
evt.mode === SpreadMode.ODD);
buttons.spreadEvenButton.classList.toggle('toggled',
evt.mode === SpreadMode.EVEN);
buttons.spreadNoneButton.classList.remove('toggled');
buttons.spreadOddButton.classList.remove('toggled');
buttons.spreadEvenButton.classList.remove('toggled');

switch (evt.mode) {
case SpreadMode.NONE:
buttons.spreadNoneButton.classList.add('toggled');
break;
case SpreadMode.ODD:
buttons.spreadOddButton.classList.add('toggled');
break;
case SpreadMode.EVEN:
buttons.spreadEvenButton.classList.add('toggled');
break;
}
});
}

Expand Down

0 comments on commit f853042

Please sign in to comment.