Add scrollbar
To address the need for a visual indicator, I implemented a non-interactive scrollbar with 3 modes, selectable via config:
- Never (default): This disables the scrollbar.
- Fading: Shows the scrollbar after the view changed (e.g. by scrolling, adding new text, or resizing the window) and fades it out after a short time. Does not reserve space for the scrollbar, but draws it semi-transparent over the view area.
- Always: Reserves space for the scrollbar. The scrollbar is permanently shown, unless scrolling is impossible, because there is no content out of view.
The mode and further configurations are documented in the manpage.
Of the different features discussed in #775, this PR only addresses the need for a visual indicator, not for new ways to navigate the scrollback. If there is interest to merge such a change, I would be willing to implement dragging functionality to the scrollbar.
Fading mode
https://i.imgur.com/FYyMJtc.mp4
Always mode
https://i.imgur.com/BcUkKq9.mp4
I haven't looked very deep into the implementation nor that I tested, but I have a few questions based on the quick glance:
- Have you considered making it similar to the way search bar works in terms of sizing? For example maybe it can simply be configured in terms of the width of the cell and say, that minimum height will be clamped by the height of the cell?
- How this will interact with the
Vimode, which also has an indicator, but a bit different? - Have you thought on defining a fixed animation or reusing the ones we have for the visual bell (maybe only subset of them)? I think that logic could be reused a lot, since visual bell and scrollbar are animating the same basic object (rectangle with alpha). I don't think that you can configure scrollbar animations in software in general, and less config options is better.
- One concern is how selection should work with the
Fadingoption you showed, since if this scrollbar can be dragged with the mouse, it'll conflict with the selection andVimode cursor. I tend to think thatAlwaysshowed option could be better? Or maybe fading could be done for theAlwayscursor and you'll just have a constant offset on the right and you'll get a scrollbar visible once you move your mouse or something?
Without mouse drag I'm pretty sure it's not that helpful, since mouse users want to fast scroll the terminal sometimes. Though, I don't think that it'll be that complicated to add, given that with Always mode it's relatively simple.
I'm in general +1 on adding this feature, because it has practical use and given that it's relatively simple, since you just offset and draw rectangle, which is basically what search does already, but you offset from the right side instead, and do similar offsetting in resize methods.
I'd wait for @chrisduerr to comment on that matter, but I know that the amount of config options should be cut, and that without a mouse/touch drag, it'll be hard to sell this feature. On mobile devices scrollbar will help a lot, since you can select and scroll with the scrollbar at the same time basically, and scrolling without kinetic scroll, etc is painfully slow there.
To answer some of the questions:
-
I did not consider fixing the size and margins by cell sizes, but that would be a way to reduce config options while still respecting the user's preferences for sizing.
-
In
VImode, the indicator is drawn over the cell area and is therefore handled the same way as the console text. InFadingmode, the scrollbar is drawn over it. InAlwaysmode, where the scrollbar reserves space, the scrollbar appear to the right of the indicator. -
Since I never used the visual bell, I wasn't aware it does has similar logic. I will look into whether the animation logic can be shared or reused. Though I wouldn't want to use the bell's
durationconfig for scrollbar fading, as that would require users of a fading scrollbar, to also use the visual bell. -
For the
Alwaysmode, there would be no conflict between scrollbar and selection. ForFadingmode, I planned to have a click on the scrollbar trigger scrollbar dragging and no selection. To start a selection at a cell, that is shadowed by the scrollbar, you would have to wait until the scrollbar is faded out.
I would wait for directions before with implementing mouse dragging, because one (old) comment from @chrisduerr under #775 was: A scrollback that can be dragged around is definitely out of scope of this project. The main reason why this issue is still open, because an indicatior to show position in the scrollback buffer is not completely out of question. . I agree, that support for mouse dragging shouldn't be too complicated.
I personally think a non-interactive scrollbar is already a usage improvement compared to no scrollbar, as it provides more intuitive orientation than the line numbers in the VI indicator, though I agree, that a draggable scrollbar would be a much bigger improvement for mouse users.
This is super cool!
Have you considered making it similar to the way search bar works in terms of sizing? For example maybe it can simply be configured in terms of the width of the cell and say, that minimum height will be clamped by the height of the cell?
This was one of the first things I noticed. As it stands the scrollbar itself gets so small that it is somewhat lost in the window rounding on macOS. I wouldn't even be surprised if the scrollbar minimum height was two cells tall TBH.
How this will interact with the Vi mode, which also has an indicator, but a bit different?
I'm not sure I mind having the two indications on screen at once, since one is the well known scrollbar, while the other tells you exactly what line the cursor is on. However, I'd be curious if these features could be unified somewhat?
On a very small note, and perhaps this is just my bias, but I think the default fadeout durations should be a bit faster. Both the delay before fade and the fade themselves.
Digging a bit deeper into the current config options here's some more thoughts:
Setting the min_height to 25 seems decent to me, but I still like the idea of removing this config option altogether and just using a sane default. I'm starting to thinks that it shouldn't be related to the cell height though. Think about it, on a web browser, you can scale the content, but it doesn't change the scrollbar size itself.
Additionally, there's a small bug here I think. The actual position of the scrollbar appears to be the middle of the bar, however, it should be the bottom of the scrollbar when at the bottom, and the top of the scrollbar when at the top. This way the scrollbar is always completely inside the display. Just a little bit more math required to offset the bar needed.
Is there any reason to not just disable the scrollbar altogether when the opacity is 0.0? Should this even be configurable? I get that color probably needs to be configurable, unless there's a smart way to choose an inverse of the background color, but 0.5 seems like a totally sane value for the fading mode here to me.
Finally, I'll add that the current interaction between window.padding and scrollbar.margin seems a bit funky to me. I feel like it's sort of weird to have the scrollbar essentially outside of the window, if you think of this like the box model. But frankly, I don't see a good reason for this to be configurable. Once the scrollbar is tall enough and clamped to the screen (see my last comment), then the margin.y becomes less critical and potentially confusing, since it may lead you to think there's more to scroll than there is. margin.x being set to 0 seems like a perfectly fine setting to me.
Additionally, there's a small bug here I think. The actual position of the scrollbar appears to be the middle of the bar, however, it should be the bottom of the scrollbar when at the bottom, and the top of the scrollbar when at the top. This way the scrollbar is always completely inside the display. Just a little bit more math required to offset the bar needed.
Thanks for testing. It should be fixed now on the wip-branch.
The height should likely be based on the amount of lines in scrollback and just have a minimum hieght of a cell height/or two.
@kchibisov I was just referring to the min height.
If anything though, it should be based on the window height, not the cell height. I think I'd find it strange for the height of the scrollbar to change for any reason other than the amount of content in the scrollback changing. It's also simpler to make it a constant amount.
I have updated several aspects:
Reduced config options
The x-margin was replaced by reusing the window padding as margin between text and scrollbar. The y-margin was removed. The scrollbar width and minimum height is now one cell wide and two cells high. Having the height based on cell size can be unintuitive, as the scrollbar gets larger if you increase text size. I tried to use a value based on window height instead, but couldn't find a reasonable value for both large and small screens. Basing height on cell size should be very safe in that regard, as every window, whether on a large screen or a mobile device, must have a well visible and not too large cell size.
Mouse interactibity
The scrollbar is now draggable by mouse and touch events.
Others
Scrollbar doesn't correctly indicate no scrollback, it is hidden rather than being the full height of the terminal (this matches browsers, but I'm not sure it makes sense for "always", vte always shows the scrollbar)
Scrollbar is no longer hidden if it would take up the whole screen height in Always mode, so that mode gets closer to other terminal emulators, while Fading mode stays closer to browser behavior.
Don't use let else
Let-else is now written out, as it like before rust 1.65.
No framerate limit on fading?
While waiting for the actual fading, the scrollbar is no longer redrawn. Instead a redraw is scheduled with a timer. For the animation itself (by default for 0.4 seconds), there is no throttling. The animation directly requests a redraw similar to the visual bell.
Display offset should not be stored in two locations, in fact it should be able to compute all necessary data from just the render state, rather than having extra scroll state
To know, whether the scrollbar must be redrawn (reset fading and add damage rect), there needs to be some tracking whether the view area changed since last redraw. As far as I know this isn't done yet, so I opted to have the scrollbar keep track of the changes relevant to it. Instead of storing display_offset and total_lines, the time of last view change could also be stored in Term and be updated on Term::scroll_display, Term::resize, and others. But I consider that a more error-prone implementation, as there are multiple actions (scroll, add new line, resize window) requiring a scrollbar redraw, and all would need to be aware of the this tracking. With the current implementation, the scrollbar knows itself, whether the state changed in a way that requires redrawing, which reduces the need for other existing or future parts of alacritty to be aware of the scrollbar.
@flash-freezing-lava are you waiting on re-review? Though, I don't remember if you can re-request it.
@kchibisov The request for your review should still be open.
Small thing I noticed while giving this another look today.
Live reloading the config doesn't seem to work when the scrollbar was initially not present.
Also, if we're modeling the behavior of the cursor after macOS at all (which isn't always the worst thing to do), we shouldn't change it to CursorIcon::RowResize since we're not really resizing anything.
I was somewhat surprised that the cursor didn't change to a Pointer on hover, but thinking more about it, it makes sense. I can't test how other scrollbars work on Windows and Linux systems right now, but I suspect they are the same as macOS in this respect.
More tiny UI/UX things.
Not sure if this is the same issue as I'm having with the visual bell on macOS, but the scrollbar doesn't seem to go away in the fading mode until another action triggers a redraw.
Perhaps we can kill two birds with one stone in this fix?
Last thing for now.
How would we feel about always drawing the top and bottom of the scrollbar on the absolute top and bottom of the window instead of inside a row of the grid? This ways A) it's more clear when you are at the top or bottom and B) the scrollbar doesn't jump around as much when vertically resizing.
This is generally more consistent with how other applications layout the scrollbar and it's controlling content. The scrollbar isn't inside of the content it scrolls.
will we get this feature?
I’m watching this, happy to test as needed.