Repository navigation
fix(ui): host the touch sheet on gpui-base Sheet - #565
Merged
Merged
Conversation
The bottom sheet rebuilt what gpui_base::Sheet owns: the overlay press, Escape, and it had no focus trap. It now supplies only the scrim and the surface. OutsideDismissal keeps the release of the dismissing press from reaching what the sheet uncovered, which base's press-time dismissal cannot do; it is armed from the dismissal instead of the press, so the sheet arms it from Sheet::request_close, and a new press disarms it so an Escape costs nothing.
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
1 participant
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.
Changed behaviour
Popover::bottom_sheetrebuilt by hand whatgpui_base::Sheetowns — the overlay press, Escape through its ownkey_context("Popover"), outside-dismissal — and had no focus trap. It now handsSheetthe scrim and the surface and keeps only presentation: the Presence fade, the grabber, the title row, the scrollable content area. Escape, overlay dismissal and the focus trap come from base.OutsideDismissalstays, with its gap named at the site: base dismisses on the outside press, and GPUI then delivers that press's release (and the click it completes) to whatever the popup uncovered — confirmed by removing the guard from the old code, which fails the same test. The guard is now armed from the dismissal rather than from the press event, so the sheet arms it fromSheet::request_close, and it is disarmed by the next press so a dismissal by Escape or by choosing an item swallows nothing. Popover and menus keep consuming the press itself, with a comment naming base's pass-through default they depart from.Per Principle 7 as extended in #564.
Tests
all_popovers_consume_dismissal_release_even_after_redraw(existing) covers the release guard for sheet, popover and dropdown.a_dismissal_without_a_press_leaves_the_next_click_alone: Escape closes the sheet and the next click lands — the contract the arm-from-dismissal design must keep.tall_sheet_scrolls_below_navigation_and_above_keyboard, the pickers' sheet geometry tests andshell::tests::compact*assert the unchanged sheet/scrim bounds and that Escape reaches the sheet.Checks run
cargo fmt --all --check,cargo clippy --workspace --all-targets --locked -- -D warningscargo nextest run -p tcode-ui --locked— 346 passedNot verified visually: the computer-use tools on this machine lack Accessibility/Screen Recording permission, so I could not open the sheet in the phone example. Geometry is asserted by the tests above; the surface's styling code is unchanged.