Skip to content

Add a new Page scrolling mode (issue 2638, 8952, 10907) #14112

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

Merged
merged 2 commits into from
Oct 15, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Oct 8, 2021

This implements a new Page scrolling mode, essentially bringing (and extending) the functionality from PDFSinglePageViewer into the regular PDFViewer-class. Compared to PDFSinglePageViewer, which as its name suggests will only display one page at a time, in the PDFViewer-implementation this new Page scrolling mode also support spreadModes properly (somewhat similar to e.g. Adobe Reader).

Given the size and scope of these changes, I've tried to focus on implementing the basic functionality. Hence there's room for further clean-up and/or improvements, including e.g. simplifying the CSS/JS related to PresentationMode and implementing easier page-switching with the mouse-wheel/arrow-keys.

Fixes #2638
Fixes #8952
Fixes #10907

@Snuffleupagus Snuffleupagus force-pushed the ScrollMode-PAGE branch 14 times, most recently from 5e239e2 to c346f10 Compare October 9, 2021 20:57
@Snuffleupagus Snuffleupagus marked this pull request as ready for review October 9, 2021 21:01
@Snuffleupagus Snuffleupagus marked this pull request as draft October 9, 2021 21:58
@Snuffleupagus Snuffleupagus force-pushed the ScrollMode-PAGE branch 2 times, most recently from fa6f3ec to e4cf1e2 Compare October 10, 2021 10:27
@Snuffleupagus Snuffleupagus marked this pull request as ready for review October 10, 2021 10:36
@mozilla mozilla deleted a comment from pdfjsbot Oct 10, 2021
@mozilla mozilla deleted a comment from pdfjsbot Oct 10, 2021
@SamyCookie
Copy link

SamyCookie commented Oct 11, 2021

Thanks @Snuffleupagus, I think it was one of the last big missing UX features in this project. Happy to see this coming !

@mozilla mozilla deleted a comment from pdfjsbot Oct 11, 2021
@mozilla mozilla deleted a comment from pdfjsbot Oct 11, 2021
@Snuffleupagus Snuffleupagus force-pushed the ScrollMode-PAGE branch 3 times, most recently from 14bb51a to 2cbc758 Compare October 12, 2021 09:48
This implements a new Page scrolling mode, essentially bringing (and extending) the functionality from `PDFSinglePageViewer` into the regular `PDFViewer`-class. Compared to `PDFSinglePageViewer`, which as its name suggests will only display one page at a time, in the `PDFViewer`-implementation this new Page scrolling mode also support spreadModes properly (somewhat similar to e.g. Adobe Reader).

Given the size and scope of these changes, I've tried to focus on implementing the basic functionality. Hence there's room for further clean-up and/or improvements, including e.g. simplifying the CSS/JS related to PresentationMode and implementing easier page-switching with the mouse-wheel/arrow-keys.
With the previous commit, both of the `PDFViewer` and `PDFSinglePageViewer` clases are now small/simple enough that it no longer seems necessary to keep them in separate files.
@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/fe02baa63f4b2a2/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/fe02baa63f4b2a2/output.txt

Total script time: 4.36 mins

Published

@timvandermeij timvandermeij merged commit e504e81 into mozilla:master Oct 15, 2021
@timvandermeij
Copy link
Contributor

Nice implementation! It helped that you limited the scope here because it was indeed quite some code to review carefully.

@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij Thank you so much for reviewing this; I know it cannot have been easy!
You probably wouldn't have wanted to see my first drafts though, but in the end I was really quite surprised at the (relatively) small net increase in code-size here :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants