Zoom out: maintain scroll position #61465
Draft
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
ellatrix
added
[Type] Bug
|
Size Change: +87 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
scruffian
force-pushed
the
try/zoom-out-maintain-scroll
branch
from
af73d49 to
3f1831a
Compare
|
@stokesman do you think #63390 will resolve the Issue this PR seeks to fix? |
|
Yes, I think so. |
2 tasks
draganescu
force-pushed
the
try/zoom-out-maintain-scroll
branch
from
3f1831a to
bf676b2
Compare
|
I've refreshed this.The way this PR proposes to fix the issue makes perfect sense but it doesn't work yet. The scroll calculation seems to be off and also the effect is ran once even after the scroll is set. Now we also have a hook |
Thank you @draganescu 👍
How does it propose to fix it? It isn't clear from the description. Would you be able to update that with any knowledge you gleaned from the code?
That's a shame. Do you think this route is the best option for fixing this bug. To me it's a pretty important Issue to get fixed. |
|
@getdave updated the description.
It makes sense - we have the correct scroll position when the user initiates the exit, we just need to apply it to the new situation. Another option is to simply scroll into view the current selection. |
|
It seems like this also needs to take the frame spacing into account in its scale calculation much like #65998. |
Perhaps my findings while testing are related to this not perfect, but I noticed that the zoom doesn't always display the correct section. My selected block was in frame only half of the time. |
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.
What?
When toggling zoom out, we should maintain the scroll position to the currently visible content.
It's not perfect, but better than trunk.
Closes #65884
Why?
Because it's disorienting be scrolled to the top when zoomed in, given the intent of the double click is to edit the section which was just in view before double click,
How?
Try to compute scroll position after the zoom out is exited getting the previous scale and scroll and applying them to the new scale.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
zoom-in-maintain-scroll.mp4