Repository navigation
scroll: Scroll content past a scrollable's maximum size - #3613
Open
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
Open
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
An element with a maximum size along the axis it scrolls does not scroll: under
v_flex().max_h(px(320.)).overflow_y_scrollbar(), the rows past 320px are clipped and cannot be reached. Both copies ofScrollable, in gpui-component and in gpui-shell, have the code involved; this changes both.Scrollablewraps the element. The wrapper copies its size, minimum size and maximum size, and the element becomes the content of a scroll area inside the wrapper. Two things keep such an element from scrolling:size_full(), 100% of the wrapper, does not hold it to the wrapper's maximum size. In a column whose height follows its content, placed in a row, the area comes out as tall as the inside of that column (140px for a 100px maximum between two 20px blocks), and the wrapper clips it. With the default GPUI the area already gets the wrapper's 100px; the two use different Taffy versions (0.14 and 0.13). The wrapper is now a flex container along the scrolled axis, so the area is a flex item and shrinks to the wrapper's size with either. Where the wrapper's size is fixed or comes from its parent, as withflex_1()orsize_full(), the area keeps the size it had.ScrollBouncetakes the layout of the area it wraps, andScrollableMaskis absolutely positioned, so neither becomes a second flex item. With both axes (overflow_scrollbar()) the wrapper is a column, so only the area's height is held to the wrapper's maximum this way.Found in a form whose fields sit under
max_h(320).overflow_y_scrollbar()inside a card.How to Test
max_height_scrollable_scrolls_to_its_last_row(gpui-component) andscrollable_with_a_max_height_scrolls_what_overflows_it(gpui-shell) lay out the column described above and scroll it to its last row.max_width_scrollable_scrolls_what_overflows_itandscrollable_with_a_max_width_scrolls_what_overflows_itscroll a row with a maximum width. Without the first change all four fail. Without the flex wrapper the two max-height tests fail with GPUI Fast (gpui-shell uses it by default): the last row ends 140px below the first row's top instead of 100px. With the default GPUI they pass either way. The gpui-shell flex-item test now also scrolls to its last row; it passes before and after the change.AI-assisted: written with Claude Code and reviewed by me before submission.
Checklist
cargo runfor story tests related to the changes. (Not run; the tests above cover the layout.)