From e75171c995f950a29432f2a0a2ee02d1b21e1f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 19:17:52 -0400 Subject: [PATCH] Forms: an option list stays inside the window; media ratios are checked in one place The floating list under a Select, Combobox or MultiSelect was at least its trigger's width with no ceiling, so a long choice ran it past the window and its rows never ellipsized. It now stops at the window's width less the window margin on each side, so gpui snaps it inside. The camera preview and the model viewer check their ratio through primitives::checked_ratio. --- AGENTS.md | 2 +- TASKS.md | 1 + src/forms/options.rs | 2 ++ src/forms/tests/choices.rs | 29 +++++++++++++++++++++++++++++ src/media/camera.rs | 5 ++--- src/media/model.rs | 5 ++--- 6 files changed, 37 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c9a44d8..c146040 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,7 +65,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - The macOS open panel runs out of process too. Escape does not reach it; the harness sends it `cancel:`. - gpui's `test-support` swaps its executor: `block` fails on a real dispatcher. It stays behind this crate's `test-support` feature, never on for the gallery. - A tracked `FocusHandle` is a Tab stop only when built with `.tab_stop(true)`. `tab_index` on the div reaches only handles gpui makes itself. `primitives::tab_stop` keeps one per id. -- Floating lists go below their anchor, or above when only above has room (`forms::options::float`). gpui's own switch keeps the anchor point, so a flipped list would cover its trigger. +- Floating lists go below their anchor, or above when only above has room (`forms::options::float`). gpui's own switch keeps the anchor point, so a flipped list would cover its trigger. An option list is at least its trigger's width and at most the window's less `window_margin` each side, so gpui can snap it inside and its rows end in an ellipsis; wider, gpui pins it left and the rest runs off. - A trigger that opens a list goes through `forms::select::listing`: toggle, keys, blur and popup live there, for Select, TabOverflowMenu and Breadcrumb alike. - A palette goes through `navigation::palette::Palette`: query field, grouped rows, cursor, keys and focus live there. CommandPalette, QuickOpen, QuickSwitcher, SearchPalette and QuickLauncher only build rows. - A menu goes through `menus::menu`: rows, submenus, keys, focus and presses outside live there, for DropdownMenu, OverflowMenu, SplitButton, ContextMenu, SearchableMenu and MenuBar. A press on the host's own box is the host's to handle, so a trigger toggles and a right click moves a context menu without a close and reopen in one event. diff --git a/TASKS.md b/TASKS.md index 01c0a1a..5a4269c 100644 --- a/TASKS.md +++ b/TASKS.md @@ -316,3 +316,4 @@ Per-component lines live in `tasks/`. Tags there: | T26c | 3 | PASS | | | T26d | 1 | FAIL | the playlist's keys started from the first row, not the song under way; the waveform's bar fit sat untested in paint | | T26d | 2 | PASS | | +| T26e | 1 | FAIL | an option list had no width limit, so a long device name ran it past the window | diff --git a/src/forms/options.rs b/src/forms/options.rs index 4361da1..3a3bbdb 100644 --- a/src/forms/options.rs +++ b/src/forms/options.rs @@ -286,10 +286,12 @@ impl Popup<'_> { }) }); let list = surface(self.id, cx) + .debug_selector(|| "option-list".into()) .min_w(theme.tooltip_max_width()) .when(anchor.size.width > Pixels::ZERO, |list| { list.min_w(anchor.size.width) }) + .max_w(window.viewport_size().width - theme.window_margin() * 2.0) .max_h(theme.list_max_height()) .overflow_y_scroll() .when_some(self.scroll, |list, handle| list.track_scroll(handle)) diff --git a/src/forms/tests/choices.rs b/src/forms/tests/choices.rs index 4b8e72b..0e544ab 100644 --- a/src/forms/tests/choices.rs +++ b/src/forms/tests/choices.rs @@ -446,3 +446,32 @@ fn an_open_select_with_nothing_to_pick_fails(cx: &mut TestAppContext) { setup(cx); cx.add_window_view(|_, _| Empty(false)); } + +/// A select whose one choice runs long. +struct Named; + +impl Render for Named { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let long = Choice::new("long", "A very long microphone name that runs past the box"); + div() + .w(px(200.0)) + .child(Select::new("long", [long, Choice::new("b", "B")])) + } +} + +#[gpui::test] +fn a_long_choice_keeps_its_list_inside_the_window(cx: &mut TestAppContext) { + setup(cx); + let (_, cx) = cx.add_window_view(|_, _| Named); + cx.simulate_resize(gpui::size(px(280.0), px(600.0))); + cx.run_until_parked(); + cx.update(|window, _| window.focus_next()); + cx.simulate_keystrokes("enter"); + release("enter", cx); + cx.run_until_parked(); + let list = cx.debug_bounds("option-list").expect("the list opens"); + assert!( + list.left() >= px(0.0) && list.right() <= px(280.0), + "{list:?}" + ); +} diff --git a/src/media/camera.rs b/src/media/camera.rs index 53e8004..cf0b480 100644 --- a/src/media/camera.rs +++ b/src/media/camera.rs @@ -7,7 +7,7 @@ use gpui::{ use crate::{ forms::{Choice, OnValue, Select}, - primitives::{Icon, IconName, framed}, + primitives::{Icon, IconName, checked_ratio, framed}, theme::{ActiveTheme, IconSize, Radius, TextSize}, typography::Ellipsis, }; @@ -24,10 +24,9 @@ pub struct CameraPreview { impl CameraPreview { /// `ratio` is the camera's width over its height. pub fn new(id: impl Into, ratio: f32) -> Self { - assert!(ratio > 0.0, "a camera's shape of {ratio}"); Self { id: id.into(), - ratio, + ratio: checked_ratio(ratio), frame: None, name: None, } diff --git a/src/media/model.rs b/src/media/model.rs index 488fd95..37fc3fa 100644 --- a/src/media/model.rs +++ b/src/media/model.rs @@ -11,7 +11,7 @@ use crate::{ layout::seeded::use_seeded, motion::Spinner, overlays::media_button, - primitives::{FocusRing, IconName, framed, tab_stop}, + primitives::{FocusRing, IconName, checked_ratio, framed, tab_stop}, theme::{ActiveTheme, ControlSize, Radius}, }; @@ -70,7 +70,6 @@ pub struct ModelViewer { impl ModelViewer { /// `ratio` is the view's width over its height. pub fn new(id: impl Into, ratio: f32, orbit: Orbit) -> Self { - assert!(ratio > 0.0, "a view's shape of {ratio}"); assert!( (NEAR..=FAR).contains(&orbit.distance), "a distance of {}", @@ -78,7 +77,7 @@ impl ModelViewer { ); Self { id: id.into(), - ratio, + ratio: checked_ratio(ratio), frame: None, orbit, on_orbit: None,