Also update the browser history when the user *manually* change pages using the pageNumber-input (PR 12493 follow-up) #12559
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/1b56d669dce1e16/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/df3fe667fb5c68f/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/1b56d669dce1e16/output.txt Total script time: 3.43 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/df3fe667fb5c68f/output.txt Total script time: 4.90 mins
|
…'s no active PDF document All of these methods will, in one way or another, cause e.g. scrolling or zooming to occur and consequently they don't really make sense unless there's an active PDF document. Especially since all of these methods end up calling into a `BaseViewer`-instance, which already contains similar early returns in essentially all of it's methods and setters.
… using the pageNumber-input (PR 12493 follow-up) This patch addresses a review comment, which pointed out that we should *also* handle the pageNumber-input, from PR 12493. Given that a user *manually* changing pages using the pageNumber-input, on the toolbar, could be regarded as a pretty strong indication of user-intent w.r.t. navigation in the document, hence I suppose that updating the browser history in this case as well probably won't hurt.
Snuffleupagus
force-pushed
the
goToPage-labels
branch
from
2973c49
to
6e83d8c
Compare
Since we're now free to use e.g. optional chaining everywhere *except* for the worker, we can thus simplify this code a bit.
Snuffleupagus
force-pushed
the
goToPage-labels
branch
from
6e83d8c
to
322b107
Compare
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/9cf4765ec7df9ea/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/9cf4765ec7df9ea/output.txt Total script time: 4.07 mins Published |
timvandermeij
approved these changes
Nov 3, 2020
|
Looks good; thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch addresses a review comment, which pointed out that we should also handle the pageNumber-input, from PR #12493 (comment).
Given that a user manually changing pages using the pageNumber-input, on the toolbar, could be regarded as a pretty strong indication of user-intent w.r.t. navigation in the document, hence I suppose that updating the browser history in this case as well probably won't hurt.