Skip to content

Commit

Permalink
Refactor PDFThumbnailViewer.scrollThumbnailIntoView to avoid unnece…
Browse files Browse the repository at this point in the history
…ssary DOM look-ups

The code responsible for highlighting/scrolling thumbnails is quite old, and parts of it hasn't really changed much over time.
In particular, the `scrollThumbnailIntoView` method is currently using `document.querySelector` in order to find both the new/current thumbnail element. This seems totally unnessary, since we can easily keep track of the current thumbnail (similar to the "regular" viewer) and thus avoid unnecessary DOM look-ups.

Furthermore, note how the `PDFThumbnailView` constructor is currently highlighting the *first* thumbnail. This is yet another leftover from older viewer code, which we can now remove and instead take care of in `PDFThumbnailViewer.setDocument`.
Given that `PDFThumbnailView` does not, nor should it, have any concept of which thumbnail is currently selected, this change also improves the general structure of this code a tiny bit.
  • Loading branch information
Snuffleupagus committed Feb 9, 2018
1 parent 2529362 commit 82416d4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 17 deletions.
6 changes: 0 additions & 6 deletions web/pdf_thumbnail_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,6 @@ class PDFThumbnailView {
div.setAttribute('data-page-number', this.id);
this.div = div;

if (id === 1) {
// Highlight the thumbnail of the first page when no page number is
// specified (or exists in cache) when the document is loaded.
div.classList.add('selected');
}

let ring = document.createElement('div');
ring.className = 'thumbnailSelectionRing';
let borderAdjustment = 2 * THUMBNAIL_CANVAS_BORDER_WIDTH;
Expand Down
38 changes: 27 additions & 11 deletions web/pdf_thumbnail_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { PDFThumbnailView } from './pdf_thumbnail_view';

const THUMBNAIL_SCROLL_MARGIN = -19;
const THUMBNAIL_SELECTED_CLASS = 'selected';

/**
* @typedef {Object} PDFThumbnailViewerOptions
Expand Down Expand Up @@ -66,15 +67,23 @@ class PDFThumbnailViewer {
return getVisibleElements(this.container, this._thumbnails);
}

scrollThumbnailIntoView(page) {
let selected = document.querySelector('.thumbnail.selected');
if (selected) {
selected.classList.remove('selected');
scrollThumbnailIntoView(pageNumber) {
if (!this.pdfDocument) {
return;
}
const thumbnailView = this._thumbnails[pageNumber - 1];

if (!thumbnailView) {
console.error('scrollThumbnailIntoView: Invalid "pageNumber" parameter.');
return;
}
let thumbnail = document.querySelector(
'div.thumbnail[data-page-number="' + page + '"]');
if (thumbnail) {
thumbnail.classList.add('selected');

if (pageNumber !== this._currentPageNumber) {
const prevThumbnailView = this._thumbnails[this._currentPageNumber - 1];
// Remove the highlight from the previous thumbnail...
prevThumbnailView.div.classList.remove(THUMBNAIL_SELECTED_CLASS);
// ... and add the highlight to the new thumbnail.
thumbnailView.div.classList.add(THUMBNAIL_SELECTED_CLASS);
}
let visibleThumbs = this._getVisibleThumbs();
let numVisibleThumbs = visibleThumbs.views.length;
Expand All @@ -86,21 +95,23 @@ class PDFThumbnailViewer {
let last = (numVisibleThumbs > 1 ? visibleThumbs.last.id : first);

let shouldScroll = false;
if (page <= first || page >= last) {
if (pageNumber <= first || pageNumber >= last) {
shouldScroll = true;
} else {
visibleThumbs.views.some(function(view) {
if (view.id !== page) {
if (view.id !== pageNumber) {
return false;
}
shouldScroll = view.percent < 100;
return true;
});
}
if (shouldScroll) {
scrollIntoView(thumbnail, { top: THUMBNAIL_SCROLL_MARGIN, });
scrollIntoView(thumbnailView.div, { top: THUMBNAIL_SCROLL_MARGIN, });
}
}

this._currentPageNumber = pageNumber;
}

get pagesRotation() {
Expand Down Expand Up @@ -133,6 +144,7 @@ class PDFThumbnailViewer {
*/
_resetView() {
this._thumbnails = [];
this._currentPageNumber = 1;
this._pageLabels = null;
this._pagesRotation = 0;
this._pagesRequests = [];
Expand Down Expand Up @@ -167,6 +179,10 @@ class PDFThumbnailViewer {
});
this._thumbnails.push(thumbnail);
}

// Ensure that the current thumbnail is always highlighted on load.
const thumbnailView = this._thumbnails[this._currentPageNumber - 1];
thumbnailView.div.classList.add(THUMBNAIL_SELECTED_CLASS);
}).catch((reason) => {
console.error('Unable to initialize thumbnail viewer', reason);
});
Expand Down

0 comments on commit 82416d4

Please sign in to comment.