-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Change scrollbar behaviour #3748
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
base: master
Are you sure you want to change the base?
Conversation
I think this is because it is covered by the status line. If you disable it with Can the calculation of the func (w *BufWindow) isScrollable() bool {
// This assumes current behavior
return w.Buf.LinesNum() > 1 || (w.Buf.LinesNum() == 1 && w.getRowCount(0) > 1)
}I don’t know if all those calculations are worth the effort. Could this be resolved by taking the status line into account? The "resolution" of the scrollbar is limited to the level of individual lines. |
Ye, if you disable statusline you can see 1 char of the scrollbar. Is that intended? Looks so off
Made some improvements to the logic, but probably there's room for improvement.
I'm sorry but I didn't understand what you meant here 😞. "Could this be resolved" wdym by "this"? Thanks for the feedback |
I don't think so; to me, it is a bug. Maybe it doesn't even take into account the infobar line either ( I kept testing, and sometimes the scrollbar reaches the end line; something is wrong. It seems to have something to do with the wrapping, probably. But you have more information gathered than I do. I did not notice the scrollbar disappearing until this PR 😅.
I was referring to the scrollbar disappearing and avoiding calculations (for loop) if possible — seeing what I saw, maybe it is not possible; I don’t know — leaving the logic mostly as it was but solving the statusline issue, obviously. This was a first glance observation; the maintainers are the ones who decide if it is okay or not. |
This PR doesn't really aim there. When I said: I was just describing the actual scrollbar behavior to contrast to what I am proposing - a scrollbar that reflects scrolling 1:1. Leaving the logic as it and "solving the statusline issue" won't achieve what I described. |
|
What do you mean exactly by "a scrollbar that provides a true 1:1 representation of the scrollable area"? What is the visual change (I did not try it, so I don't know how it looks)? My main concern is the for loop. Is that for loop worth the calculations when the lines are wrapped? What happens with that for loop when huge files are open? That function is called for all the splits of the current tab, in the main loop. That was my point the other day; except for the scrollbar disappearing in the status line, the current behavior seems mostly fine. But again, maintainers have the last word. |
I mean a scrollbar that appears when the buffer is scrollable — currently, it only shows up when there are more lines than the buffer height, even though you can scroll as long as there's more than one (visual) line. A scrollbar that accounts all the scrollable area - Nowadays it accounts the amount of lines but you can actually scroll past it. It also doesn't account softwrapping so you can have one single line with 10k rows and the scrollbar won't show up. Maybe it's easier if you see it working, if you cba to build this pr take a look of vscode scrollbar.
That was the best way I found to achieve the behavior I wanted. As I mentioned, there’s probably room for improvement or a better approach.
Maybe we just expect different things from a scrollbar 😄 — in my opinion, if you're scrolling and the scrollbar isn’t visible, something feels off. |
Didn't notice that 🙃, haha.
It seems that i don't look at the scrollbar at all. Now, seeing what you are referring to, it is unavoidable, so it looks (without testing) 👌 to me. |
Currently, the scrollbar does not accurately reflect the scrolling behavior.
It only appears when the number of visual lines exceeds the buffer height, even though scrolling is possible whenever there's more than one line. Also, when scrolling beyond the visible content, the scrollbar begins to disappear.
This PR tries to make a scrollbar that provides a true 1:1 representation of the scrollable area.