-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
Keep current scroll position when zooming the document #3513
Keep current scroll position when zooming the document #3513
Conversation
/botio-windows preview |
From: Bot.io (Windows)ReceivedCommand cmd_preview from @brendandahl received. Current queue size: 0 Live output at: http://107.22.172.223:8877/7cf813b415a8d66/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/7cf813b415a8d66/output.txt Total script time: 0.33 mins Published |
I don't seem to be seeing any of the issues as before. Squash, then I'll merge. |
Great, that is a relief. |
Entering and exiting full screen I was still getting stuck at the end of the document. This seems to fix that: - this.parseScale(this.presentationModeArgs.previousScale, true);
- this.page = this.page;
+ var currentPage = this.pages[this.page - 1];
+ this.parseScale(this.presentationModeArgs.previousScale, true, true);
+ currentPage.scrollIntoView(); |
The same affect could also be had by moving the this.isPresentationMode = false; below the parse scale, but that seems kind of hacky. |
@brendandahl I've updated I also found that there where issues with scrolling the initial position of the document into view when reloading the page, if the user had specified a hash (i.e. Finally, I want to apologise for all the problems that this patch has caused! I obviously wouldn't have submitted it if it didn't work in my testing. (Also, I still cannot understand why I've been unable to reproduce even one of the issues you found!) If this is OK, I'll squash the commits. /botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/4b5bdfc244b8ad7/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/4b5bdfc244b8ad7/output.txt Total script time: 0.28 mins Published |
…oom-v2 Keep current scroll position when zooming the document
@brendandahl Thanks for merging this, and please except my apologies for the inconvenience this patch caused you! |
@Snuffleupagus it was no inconvenience, thanks for the feature! |
@brendandahl I'm really sorry, but I think we may need to back out this PR.
So to sum up the above: I don't really understand why some of these things happen and even the ones that I do sort of get, I'm not sure how to fix properly. |
Backout of #3513 (except formatting)
@Snuffleupagus I hope you'll be able fix the issues and re-open this later, as it is great functionality. :) |
@timvandermeij Considering that I now know that this PR wasn't the sole culprit causing these issues, I'm willing to take a stab at implementing this again. But for this to happen, issue #3545 needs to be fixed first. Otherwise this PR would just be a waste of time, not to mention a great source of frustration ;-) |
This PR is an attempt to rewrite #3446 from the ground up, to hopefully be able to pinpoint and fix the bugs in that implementation.
Fixes #1521 and replaces #3446.
/cc @brendandahl