Add various improvements to scroll Thumbnails into view #2509

Merged
merged 1 commit into from Jan 18, 2013

Projects

None yet

4 participants

@Snuffleupagus
Mozilla member

This PR fixes a number of inconsistencies with regards to scrolling thumbnails into view:

  1. If no page is specified on document load, no thumbnail will be highlighted when the sidebar is opened.
  2. When inputting a new pageNumber, the thumbnail won't scroll into view. (Issue #2492)
  3. When scrolling the document with the outline view visible and then switching back to the thumbnail view, the thumbnail view doesn't update properly.

To make sure that these changes won't affect performance on document load and/or page switches, I've tested this patch with the Profiler in Firebug. As far as I can tell, there doesn't seem to be any performance regressions cause by this patch.

@yurydelendik

/botio-windows preview

@pdfjsbot

From: Bot.io (Windows)


Received

Command cmd_preview from @yurydelendik received. Current queue size: 1

Live output at: http://107.22.172.223:8877/9f1e8308d1ad8f2/output.txt

@Snuffleupagus
Mozilla member

New version uploaded with added logic to prevent the thumbnails from being scrolled unnecessarily, if the thumbnail view is already visible when the user clicks on the "Show Thumbnails" button.

@brendandahl brendandahl commented on an outdated diff Jan 10, 2013
web/viewer.js
@@ -842,6 +845,14 @@ var PDFView = {
return currentPageNumber;
},
+ set previousPage(val) {
@brendandahl
brendandahl Jan 10, 2013

No need for getter/setter here, let's just use a property on the object.

@brendandahl brendandahl commented on the diff Jan 10, 2013
web/viewer.js
@@ -1477,12 +1488,21 @@ var PDFView = {
switch (view) {
case 'thumbs':
+ var wasOutlineViewVisible = thumbsView.classList.contains('hidden');
@brendandahl
brendandahl Jan 10, 2013

Move down below to above where this is first used.

@Snuffleupagus
Snuffleupagus Jan 10, 2013

I might be a little slow right now, but I just don't understand how that would work.
If it's moved down, how would you know if the outline view was visible when it has already been hidden!?

@brendandahl
brendandahl Jan 10, 2013

Yeah, disregard that comment.

@brendandahl brendandahl commented on an outdated diff Jan 10, 2013
web/viewer.js
@@ -2212,6 +2232,10 @@ var ThumbnailView = function thumbnailView(container, pdfPage, id) {
var div = this.el = document.createElement('div');
div.id = 'thumbnailContainer' + id;
div.className = 'thumbnail';
+ // Highlight the thumbnail of the first page when no page number is specified
+ // (or exists in cache) when the document is loaded.
+ if (id == 1)
@Snuffleupagus
Mozilla member

@brendandahl The new version should hopefully address your comments, thanks for your help!

@brendandahl

/botio-windows lint

@pdfjsbot

From: Bot.io (Windows)


Received

Command cmd_lint from @brendandahl received. Current queue size: 0

Live output at: http://107.22.172.223:8877/6917f3f20ccae0a/output.txt

@pdfjsbot

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/6917f3f20ccae0a/output.txt

Total script time: 1.22 mins

  • Lint: Passed
@brendandahl brendandahl merged commit 71b1022 into mozilla:master Jan 18, 2013
@brendandahl

Thanks for the fixes!

@Snuffleupagus Snuffleupagus deleted the Snuffleupagus:tweak-thumbnail-scrolling branch Jan 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment