Finn Evers
82a7aca5a6
Closes https://github.com/zed-industries/zed/issues/23386 This PR updates the scrollbar-component to account for padding present in the parent container. Since the linked issue was opened, https://github.com/zed-industries/zed/pull/25288 improved the behaviour so that the scrollbar does allow scrolling the entire container, however the scrollbar thumb still does not go the entire way to the bottom. This can be seen here: https://github.com/user-attachments/assets/89204355-e6b8-428b-9fa9-bb614051b6fa This happens because during layouting of the scrollbar, padding of the parent container is not taken into account. The scrollbar thumb size is calculated as if no padding was present. With this change, padding is now included in the calculation, which resolves the issue: https://github.com/user-attachments/assets/1d4c62e0-4555-4332-a9ab-4e114684b4b3 The change here is to store the calculated content size during prepaint _including_ padding and use this for layouting the scrollbar. This ensures that the actual scroll max and the content size are always in sync. Furthermore, the existing `TODO`-comment is also resolved, as we now no longer look at the size of the last child but the actual parent size instead. This also removes an existing panic of the scrollbar-component in cases where the content size was 0, which was previously not accounted for (this never happened in practice so far, for example because of the padding added here: https://github.com/zed-industries/zed/blob/43712285bfa8aa75bac1ca8a7eb5146dd36239be/crates/editor/src/hover_popover.rs#L802-L809 which prevented the container size from ever being 0). --- Lastly, as I was wiring through the changes of the `content_size` I noticed that some code was duplicated during the initial layouting as well as in the click handlers. I refactored this in the second commit to use `along` where possible as well as computing the new click offset in one closure which can be passed to both event listeners. As always, should any of these changes not be wanted, feel free to let me know and I will revert these. Looking forward to your feedback 😄 Release Notes: - Fixed scrollbars sometimes not scrolling all the way to the bottom.
Design notes:
This crate is split into two conceptual halves:
- The terminal.rs file and the src/mappings/ folder, these contain the code for interacting with Alacritty and maintaining the pty event loop. Some behavior in this file is constrained by terminal protocols and standards. The Zed init function is also placed here.
- Everything else. These other files integrate the
Terminalstruct created in terminal.rs into the rest of GPUI. The main entry point for GPUI is the terminal_view.rs file and the modal.rs file.
ttys are created externally, and so can fail in unexpected ways. However, GPUI currently does not have an API for models than can fail to instantiate. TerminalBuilder solves this by using Rust's type system to split tty instantiation into a 2 step process: first attempt to create the file handles with TerminalBuilder::new(), check the result, then call TerminalBuilder::subscribe(cx) from within a model context.
The TerminalView struct abstracts over failed and successful terminals, passing focus through to the associated view and allowing clients to build a terminal without worrying about errors.
#Input
There are currently many distinct paths for getting keystrokes to the terminal:
-
Terminal specific characters and bindings. Things like ctrl-a mapping to ASCII control character 1, ANSI escape codes associated with the function keys, etc. These are caught with a raw key-down handler in the element and are processed immediately. This is done with the
try_keystroke()method on Terminal -
GPU Action handlers. GPUI clobbers a few vital keys by adding bindings to them in the global context. These keys are synthesized and then dispatched through the same
try_keystroke()API as the above mappings -
IME text. When the special character mappings fail, we pass the keystroke back to GPUI to hand it to the IME system. This comes back to us in the
View::replace_text_in_range()method, and we then send that to the terminal directly, bypassingtry_keystroke(). -
Pasted text has a separate pathway.
Generally, there's a distinction between 'keystrokes that need to be mapped' and 'strings which need to be written'. I've attempted to unify these under the '.try_keystroke()' API and the .input() API (which try_keystroke uses) so we have consistent input handling across the terminal