From 82416d4b1937d46f3983fece9b0f44358677668a Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Fri, 9 Feb 2018 14:25:16 +0100
Subject: [PATCH] Refactor `PDFThumbnailViewer.scrollThumbnailIntoView` to
avoid unnecessary 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.
---
web/pdf_thumbnail_view.js | 6 ------
web/pdf_thumbnail_viewer.js | 38 ++++++++++++++++++++++++++-----------
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js
index 4b995313f..6463f44e1 100644
--- a/web/pdf_thumbnail_view.js
+++ b/web/pdf_thumbnail_view.js
@@ -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;
diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js
index a84c72a48..14d29708f 100644
--- a/web/pdf_thumbnail_viewer.js
+++ b/web/pdf_thumbnail_viewer.js
@@ -19,6 +19,7 @@ import {
import { PDFThumbnailView } from './pdf_thumbnail_view';
const THUMBNAIL_SCROLL_MARGIN = -19;
+const THUMBNAIL_SELECTED_CLASS = 'selected';
/**
* @typedef {Object} PDFThumbnailViewerOptions
@@ -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;
}
- let thumbnail = document.querySelector(
- 'div.thumbnail[data-page-number="' + page + '"]');
- if (thumbnail) {
- thumbnail.classList.add('selected');
+ const thumbnailView = this._thumbnails[pageNumber - 1];
+
+ if (!thumbnailView) {
+ console.error('scrollThumbnailIntoView: Invalid "pageNumber" parameter.');
+ return;
+ }
+
+ 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;
@@ -86,11 +95,11 @@ 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;
@@ -98,9 +107,11 @@ class PDFThumbnailViewer {
});
}
if (shouldScroll) {
- scrollIntoView(thumbnail, { top: THUMBNAIL_SCROLL_MARGIN, });
+ scrollIntoView(thumbnailView.div, { top: THUMBNAIL_SCROLL_MARGIN, });
}
}
+
+ this._currentPageNumber = pageNumber;
}
get pagesRotation() {
@@ -133,6 +144,7 @@ class PDFThumbnailViewer {
*/
_resetView() {
this._thumbnails = [];
+ this._currentPageNumber = 1;
this._pageLabels = null;
this._pagesRotation = 0;
this._pagesRequests = [];
@@ -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);
});