Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove PDFThumbnailViewer_ensureThumbnailVisible #7040

Merged
merged 1 commit into from
Feb 28, 2016
Merged

Remove PDFThumbnailViewer_ensureThumbnailVisible #7040

merged 1 commit into from
Feb 28, 2016

Conversation

Snuffleupagus
Copy link
Collaborator

Functionality wise, ensureThumbnailVisible is essentially just a shorthand for scrollThumbnailIntoView. (And note that PDFViewer doesn't implement a ensurePageVisible method.)

The only remaining usage of PDFThumbnailViewer_ensureThumbnailVisible is inside PDFPresentationMode, which introduces an otherwise unnecessary PDFThumbnailViewer dependency there.

We're already relying on the presentationmodechanged event, in various files, to track the state of Presentation Mode. Thus we can simply listen for that event in PDFSidebar too, and update the thumbnails if necessary.

Functionality wise, `ensureThumbnailVisible` is essentially just a shorthand for `scrollThumbnailIntoView`. (And note that `PDFViewer` doesn't implement a `ensurePageVisible` method.)

The only remaining usage of `PDFThumbnailViewer_ensureThumbnailVisible` is inside `PDFPresentationMode`, which introduces an otherwise unnecessary `PDFThumbnailViewer` dependency there.

We're already relying on the `presentationmodechanged` event, in various files, to track the state of Presentation Mode. Thus we can simply listen for that event in `PDFSidebar` too, and update the thumbnails if necessary.
@Snuffleupagus
Copy link
Collaborator Author

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/453cdcd3bde48d6/output.txt

timvandermeij added a commit that referenced this pull request Feb 28, 2016
…_ensureThumbnailVisible

Remove `PDFThumbnailViewer_ensureThumbnailVisible`
@timvandermeij timvandermeij merged commit 4d9a3d4 into mozilla:master Feb 28, 2016
@timvandermeij
Copy link
Contributor

Looks good, thank you!

@Snuffleupagus Snuffleupagus deleted the remove-PDFThumbnailViewer_ensureThumbnailVisible branch February 29, 2016 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants