Reveal closer to top (center body, not signature) #80727
Conversation
wkornewald
force-pushed
the
reveal-closer-to-top
branch
from
a4b380f to
17cf5a2
Compare
There was a problem hiding this comment.
This cannot be implemented by changing the behavior of VerticalRevealType.Center and VerticalRevealType.CenterIfOutsideViewport. These are well established and used by various places inside VS Code, and by extensions, e.g. the vim extension makes use of the revealLine command which accepts as argument "at": "center".
This could be implemented by adding a new kind of VerticalRevealType in the editor and adopting this new kind for the desired features (go to definition, etc.)
wkornewald
force-pushed
the
reveal-closer-to-top
branch
2 times, most recently
from
28113cb to
ab3692a
Compare
|
This needs some testing whether the UX really is better in all cases. I had to make quite a few changes and I removed the Only the history navigation is different because JetBrains remembers the scroll position of each history entry. If you resize the window it always scrolls like with go to definition. I think VS Code should remember the scroll position, too, but I didn't look into that as part of this PR. Very small viewports (e.g. when pressing SHIFT+F12) needed some tuning, so you can see enough preceding lines. The code ensures a minimum scroll position of 100px from the top. What do you think? |
|
@wkornewald I am sorry, but I cannot accept a PR that changes the world, because I cannot estimate its impact. That was really the point of my first review. Can you please make a change that is limited to go to definition or outline? Pick one, and let's tweak that one. If we receive positive feedback (I can also take the change to VSCode's UX call), we can then expand it to the entire universe of reveal calls... but I cannot accept such a huge PR because I cannot take responsibility for such a large impact change. |
wkornewald
force-pushed
the
reveal-closer-to-top
branch
from
ab3692a to
ada2e60
Compare
This scrolls the target closer to the top, but leaves sufficient room for e.g. class/function comments, while showing more of the source below the target (i.e.. more of the class/function body).
wkornewald
force-pushed
the
reveal-closer-to-top
branch
from
ada2e60 to
00ef6f1
Compare
|
@alexandrudima I see, then let's give outline a try. The change is split into one commit for the infrastructure and one for the actual outline behavior change. I hope that's easier to review and simplifies testing individual aspects. |
|
Thank you! |
|
I have created #90068 to track the further adoption of this reveal type for other actions. |
|
Thank you, that's great news. |
This improves the jump to definition and outline UX esp. for long functions/classes.
Fixes #80726 (see issue description for reasoning behind this PR).