Skip to content

Commit

Permalink
For consistency with other viewer state, remove and deprecate setting…
Browse files Browse the repository at this point in the history
… Scroll/Spread modes in the `BaseViewer` constructor

Since other viewer state, such as the current page/scale/rotation[1], are not available as `BaseViewer` constructor options, this makes the Scroll/Spread modes stand out quite a bit. Hence it probably makes sense to remove/deprecate this, to avoid inconsistent/confusing state.

---

[1] These properties are *purposely* not available in the constructor, since attempting to set them before a document is loaded has number of issues; please refer to mozilla#8539 (comment) for additional details.
  • Loading branch information
Snuffleupagus committed Jun 29, 2018
1 parent 4c09213 commit a26848e
Showing 1 changed file with 20 additions and 13 deletions.
33 changes: 20 additions & 13 deletions web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,6 @@ const SpreadMode = {
* size in total pixels, i.e. width * height. Use -1 for no limit.
* The default value is 4096 * 4096 (16 mega-pixels).
* @property {IL10n} l10n - Localization service.
* @property {number} scrollMode - (optional) The direction in which the
* document pages should be laid out within the scrolling container. The
* constants from {ScrollMode} should be used. The default value is
* `ScrollMode.VERTICAL`.
* @property {number} spreadMode - (optional) If not `SpreadMode.NONE`, groups
* pages into spreads, starting with odd- or even-numbered pages. The
* constants from {SpreadMode} should be used. The default value is
* `SpreadMode.NONE`.
*/

function PDFPageViewBuffer(size) {
Expand Down Expand Up @@ -162,8 +154,6 @@ class BaseViewer {
this.useOnlyCssZoom = options.useOnlyCssZoom || false;
this.maxCanvasPixels = options.maxCanvasPixels;
this.l10n = options.l10n || NullL10n;
this._scrollMode = options.scrollMode || ScrollMode.VERTICAL;
this._spreadMode = options.spreadMode || SpreadMode.NONE;

this.defaultRenderingQueue = !options.renderingQueue;
if (this.defaultRenderingQueue) {
Expand All @@ -181,8 +171,17 @@ class BaseViewer {
if (this.removePageBorders) {
this.viewer.classList.add('removePageBorders');
}
if (this._scrollMode !== ScrollMode.VERTICAL) {
this._updateScrollMode();

if ((typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) &&
('scrollMode' in options || 'spreadMode' in options)) {
console.error(`The ${this._name} constructor options ` +
'`scrollMode`/`spreadMode` are deprecated, use the setters instead.');
if (options.scrollMode !== undefined) {
this.scrollMode = options.scrollMode;
}
if (options.spreadMode !== undefined) {
this.spreadMode = options.spreadMode;
}
}
}

Expand Down Expand Up @@ -524,9 +523,13 @@ class BaseViewer {
this._pagesRotation = 0;
this._pagesRequests = [];
this._pageViewsReady = false;
this._scrollMode = ScrollMode.VERTICAL;
this._spreadMode = SpreadMode.NONE;

// Remove the pages from the DOM.
// Remove the pages from the DOM...
this.viewer.textContent = '';
// ... and reset the Scroll mode CSS class(es) afterwards.
this._updateScrollMode();
}

_scrollUpdate() {
Expand Down Expand Up @@ -1076,6 +1079,8 @@ class BaseViewer {
}

setScrollMode(mode) {
console.error(`${this._name}.setScrollMode() is deprecated, ` +
`use the ${this._name}.scrollMode setter instead.`);
this.scrollMode = mode;
}

Expand Down Expand Up @@ -1140,6 +1145,8 @@ class BaseViewer {
}

setSpreadMode(mode) {
console.error(`${this._name}.setSpreadMode() is deprecated, ` +
`use the ${this._name}.spreadMode setter instead.`);
this.spreadMode = mode;
}
}
Expand Down

0 comments on commit a26848e

Please sign in to comment.