Skip to content

Conversation

@cutelisp
Copy link
Contributor

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.

@usfbih8u
Copy link
Contributor

Also, when scrolling beyond the visible content, the scrollbar begins to disappear.

I think this is because it is covered by the status line. If you disable it with set statusline off, it works (at least based on my tests).

Can the calculation of the visualLineNum() be avoided? At least in updateDisplayInfo(), maybe something like this:

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.

@cutelisp
Copy link
Contributor Author

I think this is because it is covered by the status line. If you disable it with set statusline off, it works (at least based on my tests).

Ye, if you disable statusline you can see 1 char of the scrollbar. Is that intended? Looks so off

Can the calculation of the visualLineNum() be avoided?

Made some improvements to the logic, but probably there's room for improvement.

Could this be resolved by taking the status line into account? The "resolution" of the scrollbar is limited to the level of individual lines.

I'm sorry but I didn't understand what you meant here 😞. "Could this be resolved" wdym by "this"?

Thanks for the feedback

@usfbih8u
Copy link
Contributor

Ye, if you disable statusline you can see 1 char of the scrollbar. Is that intended? Looks so off

I don't think so; to me, it is a bug.

Maybe it doesn't even take into account the infobar line either (set infobar off). I disabled both, and the scrollbar does not reach the last line, so the infobar is respected even when it is disabled.

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'm sorry but I didn't understand what you meant here 😞. "Could this be resolved" wdym by "this"?

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.

@cutelisp
Copy link
Contributor Author

leaving the logic mostly as it was but solving the statusline issue, obviously.

This PR doesn't really aim there. When I said:
"Also, when scrolling beyond the visible content, the scrollbar begins to disappear"

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.

@usfbih8u
Copy link
Contributor

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.

@cutelisp
Copy link
Contributor Author

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)?

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.

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 the best way I found to achieve the behavior I wanted. As I mentioned, there’s probably room for improvement or a better approach.

That was my point the other day; except for the scrollbar disappearing in the status line, the current behavior seems mostly fine.

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.

@usfbih8u
Copy link
Contributor

It also doesn't account softwrapping so you can have one single line with 10k rows and the scrollbar won't show up.

Didn't notice that 🙃, haha.

I did not notice the scrollbar disappearing until this PR 😅.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants