From f022f08947d0c368a6b99f7db8650c3ed4000cb4 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Mon, 9 Nov 2026 13:36:26 +0700 Subject: [PATCH 1/5] Add component audit report --- docs/component-audit.md | 183 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 docs/component-audit.md diff --git a/docs/component-audit.md b/docs/component-audit.md new file mode 100644 index 0000000..9667a60 --- /dev/null +++ b/docs/component-audit.md @@ -0,0 +1,183 @@ +# Component audit — gpui-native-kit + +Full audit of every component ahead of migrating `signed` onto this kit. Scope: all 7 crates (~21k LOC) — `crates/core` (30 components), `crates/data`, `crates/editor`, `crates/layout`, plus the foundation crates `crates/theme`, `crates/icons`, `crates/utils`, the facade, and every `Cargo.toml` feature graph. + +Audit criteria, in priority order: no over-engineering, justified function arguments only, no memory leaks, best performance. + +## Method + +Seven parallel read-only audits (one per component group) plus a manual audit of the foundation crates. Every gpui-behavior claim was verified against the pinned zed checkout (`504a7f2`, per `Cargo.lock`); every "unused param" claim was backed by workspace-wide greps (`crates/`, `examples/`, `docs/`, `tests/`). All 15 major findings were re-verified by reading the cited lines. No code was modified; static audit only, no tests run. + +- Date: 2026-11-09 +- gpui rev: `504a7f2` (zed checkout, per `Cargo.lock`) +- Revision audited: pre-`signed` migration state + +--- + +## Verdict per criterion + +| Criterion | Verdict | +|---|---| +| **Memory leaks** | Near-clean. Zero `.detach()` on component tasks/subscriptions anywhere in `crates/` (grep-verified); tasks stored in fields, self-references via `WeakEntity`, undo history bounded (100 entries), icon cache bounded, keyed element state pruned by gpui per frame. **One real leak:** `AnchorBoundsRegistry` in utils (finding 11), plus one stuck-state bug in `resize_handle` (finding 10). | +| **Over-engineering** | A handful of cases: dead defensive machinery (dialog/command_palette bounds plumbing), drifted duplicate painter (scroll_area), pass-through wrappers (markdown), single-use indirection, one speculative persistence API (resizable). | +| **Function arguments** | ~20 removal candidates found, all with zero-call-site evidence (list below). Feature graphs and module gating are clean (verified per-file against actual imports). | +| **Performance** | Three systemic hot-path issues (registry clone churn, uncapped caret blink, per-frame O(N) scan in diff_view) plus a recurring `debug_selector` pattern that allocates every frame in release builds across 7+ sites. | + +--- + +## Major findings (15, all verified) + +### A. Shared keyed state across sibling instances — root cause: un-namespaced keys + +gpui scopes `use_keyed_state` by the element-id path; a `RenderOnce` renders under its **type name**, so two instances of the same component under the same ancestor chain resolve to the *same* keyed state. Existing tests never catch this class because every test window mounts exactly one instance. + +| # | Location | Bug | Fix | +|---|---|---|---| +| 1 | `crates/core/src/menu.rs:385,395-396` | `"menu-was-open"` / `"menu-scroll"` / `"menu-selection"` shared by all `Menu`s in a window → a closed sibling resets the open menu's selection every frame; arrow-key navigation and the selection highlight break with 2+ menus (the gallery ships 5 sibling dropdowns at `examples/gallery/src/main.rs:2761-2860`). | Wrap the three calls in `window.with_element_namespace(id.clone(), …)` — the instance `id` is already a parameter. `context_menu.rs:67-77` is the correct in-repo pattern. | +| 2 | `crates/core/src/dropdown_menu.rs:210` | Same bug: `"dropdown-open-origin"` bare key. | Same namespacing. | +| 3 | `crates/core/src/slider.rs:143` | `"slider-dragging"` bare key → dragging one slider drives both. | Key by `(self.id.clone(), "slider-dragging")` — the pattern `clipboard.rs:105` already uses. | +| 4 | `crates/core/src/scrollbar.rs:144,157` | Default key `ElementId::Name("scrollbar")` → two default-keyed scrollbars in one view share drag + autohide state (gallery renders two side by side at `main.rs:3231,3257`). | Make the id a required constructor parameter (`Scrollbar::vertical(id, handle)`) and delete `.key()`; 6 call sites, most already have ids to pass. | + +### B. Correctness bugs + +| # | Location | Bug | Fix | +|---|---|---|---| +| 5 | `crates/core/src/input.rs:2518` (+ `:1802`) | `utf8_range_to_utf16` has a byte-for-byte copy of `utf16_range_to_utf8`'s body — it never converts UTF-8→UTF-16. Every IME selection/marked-range report is wrong for non-ASCII text (CJK/emoji candidate anchoring). `character_index_for_point` likewise returns UTF-8 where the trait contract (gpui `platform.rs`) demands UTF-16. ASCII-only tests cannot catch this. | Implement the real conversion (`encode_utf16().count()`); add a non-ASCII regression test. | +| 6 | `crates/core/src/combobox.rs:509-518` | Chevron-open with a selected value refilters using the field text — which *is* the selected label — so only the selected option is listed. Inconsistent with `Select` (reopens listing all) and with query-focused open. | `refilter("")` when `state.selected.is_some()` in the chevron branch. | +| 7 | `crates/core/src/command_palette.rs:481-510` | The modal overlay is not `deferred` — z-order depends on mount position; in the gallery, alerts/toasts paint and receive clicks above the open modal's scrim. Also: the whole `panel_bounds` `Rc>>` + capture canvas is dead (the scrim is a sibling of the panel, so its mouse handler cannot fire for panel clicks). | Wrap in `deferred(…)` like `alert.rs:303` / `toast.rs:221`; delete the bounds machinery, keep a bare canvas for the focus-steal paint. | +| 8 | `crates/editor/src/code_view.rs:351` | Mouse-down maps position with `Pixels::ZERO` for scroll while mouse-move correctly uses `ScrollbarHandle::offset(...).y` → clicking after scrolling selects the wrong row. | Pass the same offset in the down handler. | +| 9 | `crates/editor/src/code_view.rs:102-108,137` | tree-sitter `old_tree` reused across changed text without `Tree::edit()` — violates the documented parse contract → stale/garbled highlighting after edits (the gallery's Rust→Large sample toggle triggers it). | Delete the `tree` field and the `old_tree` param of `highlight_with_tree`; full reparse per change. Reintroduce incremental parsing only when a real consumer needs it. | +| 10 | `crates/layout/src/resize_handle.rs:241-268` | The move handler checks only the stored `dragging` flag, never `event.pressed_button`. macOS synthetic drag does not synthesize a `MouseUpEvent` on outside-release → the seam keeps resizing on hover until the next click (stuck interaction state). | In the move handler, clear `dragging` when `event.pressed_button != Some(MouseButton::Left)`. | + +### C. Memory / performance + +| # | Location | Bug | Fix | +|---|---|---|---| +| 11 | `crates/utils/src/lib.rs:40-73` | `AnchorBoundsRegistry`: app-global `HashMap` that only inserts (never removes), keyed **without `WindowId`** (a two-window app can read a same-named element's bounds from the other window), and every record **clones the whole map** (`cx.try_global().cloned()` then `set_global`). Every `Button` records per frame (`button.rs:244-248`) → unbounded growth + O(N²)-ish per-frame allocation churn. Consumed by tooltip anchoring. | Key by `(WindowId, ElementId)`; mutate in place instead of clone-and-set (verify the in-place global-update API in the pinned gpui before rewriting). | +| 12 | `crates/core/src/input.rs:2049-2054` | Caret blink is an uncapped `Animation::new(CARET_BLINK_INTERVAL * 2).repeat()` → full-window repaint + full text re-shaping at display refresh rate whenever a field is focused (~119 wasted repaints/s at 120 Hz). Violates the repo's own `with_max_fps` rule. | `.with_max_fps(4.0)` (blink period is 1 s; `reduce_motion` is already handled by gpui). | +| 13 | `crates/editor/src/diff_view.rs:332-349` | `widest_row` recomputed with an O(document) scan **every render** (walks every span of every row to feed `with_width_from_item`) — defeats virtualization on large diffs. | Compute once in `new`/`set_versions`, store in a field. | +| 14 | `crates/data/src/tabs.rs:304,367,404` | Index-keyed element ids on a drag-reorderable strip (dock reorders tabs via `move_panel`) — violates the stable-domain-id rule; ids permute on every drag, re-keying interaction state, the spinner, and the close button. | Add a stable per-tab key (`Tab::new(key, label)` or `.key(SharedString)`); stable keys already exist at every call site (`tab.panel`, labels). The index-based `on_change`/`on_close` API stays unchanged. | +| 15 | `crates/core/src/menu.rs:461` | Per-frame `Vec` deep clone into the `on_key_down` closure — every label + handler cloned each frame the panel renders. | One `Rc<[MenuItem]>` per render; clone the `Rc` into the closure. | + +--- + +## Param / dead-API removals + +All verified by workspace-wide greps with **zero call sites** outside their own definitions/tests. + +| Item | Location | Note | +|---|---|---| +| `Slider::value()` | `slider.rs:83-86` | `new` already takes `value` | +| `Separator::orientation()` | `separator.rs:46-49` | third way to set one enum; only its own test uses it | +| `Menu checked: Option` | `menu.rs:38,598` | only ever compared to `Some(true)`; `checked(false)` ≡ unset → plain `bool` (gallery/select call sites compile unchanged) | +| `Tab::lead` | `tabs.rs:37,67-74,342-357` | zero callers; also silently overrides `icon` when both set | +| `PanelDescriptor::icon` | `dock.rs:61` | zero of 45 constructor sites use it; delete the 5-hop plumbing (`PanelTab.icon`, `TabView.icon`, snapshot copy, open_panel copy, context-menu branch) | +| `DropdownMenu::icon()`, `.tab_stop()` | `dropdown_menu.rs:76,88` | untested and unexercised | +| `Alert::icon()` / `.no_icon()` | `alert.rs:82,88` | severity already derives an icon | +| `ComboboxChanged` event + `EventEmitter` impl | `combobox.rs:181,212,352` | zero subscribers workspace-wide; `on_change` covers it | +| `CodeView::language()` getter, `clear_selection()` | `code_view.rs:150,227` | zero callers | +| `markdown plain_code()` / `highlight_code()` | `markdown.rs:199-203` | cascade: delete the `highlight_code` flag + `parse` param; construction-time double parse goes with it | +| `MarkdownTableCell.column` | `markdown.rs:104` | derivable, unused by the renderer (only a test asserts it) | +| `Input::submit_on()` getter | `input.rs:698` | the setter `set_submit_on` is used; getter is not | +| `Input::accessibility_label` | `input.rs:1879` | placeholder/id fallback covers it | +| `ScrollArea::accessibility_label` | `scroll_area.rs:190` | | +| `scrollbar_state_key` → `pub(crate)` | `scroll_area.rs:200` | only consumer is `scrollbar.rs` | +| `ResizeHandle::accessibility_label`, `.disabled()` | `resize_handle.rs:70,64` | `disabled` used only by its own test | +| `Resizable::sizes()` / `set_sizes()` | `resizable.rs:38-49` | built for a persistence feature that doesn't exist (docs/CHECKLIST lists persistence as a follow-up) | +| `Side::Right` arms in `toggle_icon`/`toggle_label` | `shell.rs:112-124` | the right strip renders `WindowControls`; arms unreachable | +| `WindowControls::maximize_on_double_click` | `window_controls.rs:153` | single internal caller → private | +| `input into_bindings` | `input.rs:273` | only called by `bind_input_keys` → private | +| `render_tab(selected, selected_index)` | `tabs.rs:265-277` | same fact passed twice → keep `selected_index`, derive `selected` | +| `register_panel(cx)` notify | `dock.rs:195-199` | notifies on first insert only, and nothing renders from `panels` → drop notify + the `cx` parameter (45 call sites, mechanical) | +| `data` item closures taking `&mut Window` | `list.rs:102`, `sidebar.rs:148`, `tabs.rs:164`, `tree.rs:129` | every in-repo caller passes `|_, _|`; `&mut App` is real capability, `&mut Window` has no demonstrated use — judgment call, 9 call sites | +| `input copy_selection(_window)`, `caret_visible(&mut self)` | `input.rs:919,820` | unused param / needless mut | +| `accessibility_label` batch | `checkbox.rs:68`, `toggle.rs:65`, `switch.rs:68`, `link.rs:68`, `radio.rs:57,112`, `segmented.rs:114`, `popover.rs:83`, `dialog.rs:102`, `dropdown_menu.rs:112` | zero external callers on all of them. A uniform library convention — decide once: keep the convention or delete the dead ones. `Segmented`'s id-fallback announcing internal ids ("filter", "theme-demo-mode") should go regardless. | + +--- + +## Over-engineering + +- `dialog.rs:186,236-252` — the panel's `stop_propagation` fires before the scrim handler can, making the scrim's `panel_bounds.contains` check unreachable → the whole `BoundsCell` + capture canvas is dead machinery. One dismissal mechanism suffices. +- `scroll_area.rs:279-426` — a drifted ~150-line copy of `scrollbar.rs`'s painter (no horizontal axis, no track-click jump, different autohide policy); `ScrollAxes::Horizontal` is promised but unimplemented, with zero call sites. Delete the copy; compose the real `Scrollbar` element. This is deletion of a copy, not a new abstraction. +- `input.rs:525,563,1088` — `Rc>>` with a single owner → plain `Option` field. +- `input.rs:494-496,2180,2191` — two always-`Some` `Option>` fields papered over with `expect()`s → non-optional `Rc` fields, delete the expects. +- `input.rs:522,772,809` — `caret_epoch_started: Option` is never `None` → plain `Instant`. +- `markdown.rs:302-311` — `CodeBuilder` is a pass-through wrapper around `String` → use `String::push_str` directly. +- `markdown.rs:296-299` — `TableBuilder::None / Body { rows }` is exactly `Option>` → replace with the Option. +- `markdown.rs:662` — `Event::FootnoteReference` arm is unreachable (`ENABLE_FOOTNOTES` not enabled) and contradicts the module doc → delete. +- `avatar.rs:110-189` — user style refined twice (base + `PixelAvatar`), children re-clipped/re-rounded by the base anyway → delete `PixelAvatar.style` + its `Styled` impl + duplicate clipping. +- `combobox.rs:329-332` — `refilter` returns a `Vec` that all five call sites ignore (per-keystroke collect for nothing) → return `()`. +- `combobox.rs:320-333,430-433` — `refilter` doesn't clamp `highlight` (the command palette does); the identity round-trip `position(visible[row]) == row` is a bounds check in disguise → clamp in `refilter`, then `highlight.filter(|&row| row < visible.len())`. +- `code_view.rs:251-253` — `.max(0)` after `.clamp(0, len - 1)` is a no-op. +- `code_view.rs:282` / `diff_view.rs:390` — `char_width = text_size * 0.62` guessed monospace advance, duplicated in two files, drives hit-testing and row width → hoist to a theme metric. +- `segmented.rs:167-171` — `selectable: Vec = (0..len).collect()` is always `0..len` → index math. +- `Cargo.toml` (layout) — the `dock` feature enables `resizable` for nothing. + +--- + +## Performance (beyond the majors) + +### Eager `format!`/`to_string` for `debug_selector` + +gpui's cfg-gate only disables the method *body*; arguments allocate every frame **including release builds**. The correct pattern (already used in `clipboard.rs:126`, `status_dot.rs:97`) captures cheap clones and formats inside the closure: `.debug_selector(move || format!("{id}-row-{n}"))`. + +Sites: `switch.rs:136`, `field.rs:97-117` (×3), `combobox.rs:601,679`, `command_palette.rs:337,413`, `tabs.rs:305,409`, `sidebar.rs:198`, `code_view.rs:559`, `diff_view.rs:404,542`, `input.rs:2044`, `menu.rs:286` (+ the popover/dialog a11y-label fallbacks). + +### Per-frame clones/collects in render + +- `segmented.rs:167-171` — deep clone of every `SegmentedOption` + the `0..len` Vec, per frame → store `Rc<[SegmentedOption]>` at construction. +- `select.rs:209-225` — whole options `Vec` cloned per frame and the trigger re-derives the label inside a closure → compute `text` once in `render`, move only the `SharedString` in. +- `dock.rs:999-1111` — per pane per frame for the context menu: `pane.clone()`, `tabs.clone()`, three separate `panels` collects → capture one slim tuple Vec, collect once. +- `alert.rs:307` — `entries().to_vec()` clones the entry Vec every render of `AlertsView` (borrow-scoping workaround; low priority). +- `code_view.rs:478` — the whole `Line` cloned per visible row per frame; `render_row` only reads → borrow. +- `menu.rs:286` + `popover.rs:115`, `dropdown_menu.rs:205`, `dialog.rs:158` — per-frame `SharedString::from(self.id.to_string())` fallback labels. + +### No-op notifies + +- `input.rs:897-916,1253-1261` — `move_cursor`/`select_range`/`drag_select_to`/`scroll_by` always `cx.notify()` + `window.notify_text_input_changed(...)`: pressing Left at offset 0 or wheeling a non-scrollable field triggers a full re-render + IME re-query → early-return when cursor/anchor/scroll are unchanged. + +### Theme-token violations + +Rule: `crates/theme` owns every color and metric — no raw `rgb`/`rgba`/`hsla` or layout `px(...)` elsewhere. + +- `button.rs:202-205` — `hsla(0.0, 0.0, 0.0, 0.0)` ×4 as a "no border" color → apply the border conditionally instead. +- `dropdown_menu.rs:196` — `hsla(0.0, 0.0, 0.0, 0.0)` → `gpui::transparent_black()`. +- `avatar.rs:187` — `Hsla { a: 0.16, ..main }` + luminance multipliers `1.6/0.95/0.45/0.18` → theme token. +- `separator.rs:72` — `px(1.)` → `theme::BORDER_WIDTH` (already exists, used by button). +- `menu.rs:472`, `combobox.rs:666`, `command_palette.rs:404` — `px(320.)` popup max-height, three copies → one theme metric. +- `tooltip.rs:79,92` — `px(360.)` max width, `px(1.)` nudge. +- `input.rs:1195,1278` + `:32` — `px(1.5)` caret width, `px(1.)` underline, `SCROLL_LINES_HEIGHT = 20.0` → theme metrics (next to `theme.scrollbar.*`). +- `resize_handle.rs:17` — `KEYBOARD_STEP: f32 = 32.0`. +- `window_controls.rs:26,215-220,237` — `TRAFFIC_LIGHT_PAD`, `rgba(...)` Windows close-hover, caption width. +- `window_frame.rs:13-15` — `RESIZE_INSET`, `BORDER_WIDTH`. + +### Input misc + +- `input.rs:1726` — every IME composition step pushes undo history; steps >300 ms apart fragment undo → merge unconditionally while `marked_range` is active. +- `input.rs:1168` — `entry.line.paint(...).ok()` swallows a `Result` (failure = invisible text) → log like the shape path at `input.rs:984` does. +- `input.rs:820` — `caret_visible(&mut self)` doesn't mutate → `&self`. + +### Semantic inconsistencies + +- `alert.rs:264-269` — `Alerts::clear()` drops entries without running `on_dismiss` handlers (`Toasts::clear` runs them via `start_exit` → `finish_exit`) → run the handlers or document the difference. +- `scroll_area.rs:403-407` vs `scrollbar.rs:348-353` — autohide touch-on-drag vs touch-on-hover: two painters, two behaviors → pick one policy (subsumed by the scroll_area consolidation). +- `dock.rs:316-362` — `close_panel` reconciles the displayed right view when its tab closes, but `close_others`/`close_pane` don't → the sidebar can keep showing a closed panel's inspector. +- `combobox.rs:651-658` — "No matches" row is near-dead (empty results close the popup); the palette stays open showing "No matches" → pick one semantic. +- `command_palette.rs:336` — row ids are list indexes `("command", row)` for content that reorders every keystroke; `Command.id` is a stable `SharedString` → `("command", command.id.clone())`. +- `shell.rs:283-324` — hardcoded element ids not namespaced by the shell id (`"sidebar-strip-left"` etc.), unlike `Dock` which composes every id from `dock_id` → compose `(id, "…")`. +- `data` a11y sets — `sidebar.rs:270-298` counts headers in row sets; `tree.rs:250-261` uses flat-list position/size instead of per-sibling → compute among real siblings. + +--- + +## Clean bills + +`badge`, `progress` (fps-capped), `spinner`, `status_dot`, `clipboard`, `empty_state`, `context_menu` (the namespacing exemplar), `list`, `editor/lib` + `editor/utils` + `highlight`, `layout/lib`, `theme` (engine sound; watchers are app-lifetime; `set_preset` has one small redundant name lookup), `icons` (cache bounded, negatives cached), all `Cargo.toml` feature graphs, and all lifecycle plumbing: no detached tasks/subscriptions anywhere in `crates/`, `WeakEntity` used correctly, `cx.notify()` never from `render`, overlay state reclaimed by gpui's element-state eviction on dismiss. + +--- + +## Recommended fix order (before migrating `signed`) + +1. **P0 — shared keyed state sweep** (findings 1-4): ~15 lines total; add a two-sibling regression test per component, since single-instance tests provably mask this class. +2. **P0 — user-facing bugs**: IME UTF-16 conversion (5), combobox chevron (6), palette z-order (7), code_view click (8) + incremental misuse (9), resize capture loss (10), tab element ids (14). +3. **P1 — performance**: registry (11), caret blink (12), widest_row (13), the `debug_selector` batch, the per-frame clone batch. +4. **P2 — deletions**: the param/dead-API table and the over-engineering list — this is where "no over-engineering" actually gets enforced, and it shrinks the API surface `signed` will migrate onto. -- 2.54.0 From 73f7dc31d4ce51cf80ddbe09be6a96dd85ae9db2 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Mon, 9 Nov 2026 14:04:25 +0700 Subject: [PATCH 2/5] Fix shared element state across component instances --- crates/core/src/combobox.rs | 8 +- crates/core/src/command_palette.rs | 64 +++++-------- crates/core/src/dropdown_menu.rs | 3 +- crates/core/src/input.rs | 8 +- crates/core/src/menu.rs | 7 +- crates/core/src/scrollbar.rs | 36 +++----- crates/core/src/slider.rs | 10 +- crates/core/tests/dropdown_menu.rs | 133 +++++++++++++++++++++++++++ crates/core/tests/scrollbar.rs | 100 +++++++++++++++++++- crates/core/tests/slider.rs | 129 ++++++++++++++++++++++++++ crates/data/src/tabs.rs | 14 ++- crates/data/tests/tabs.rs | 8 +- crates/editor/src/code_view.rs | 27 ++---- crates/editor/src/diff_view.rs | 6 +- crates/editor/src/highlight.rs | 19 +--- crates/editor/src/markdown.rs | 7 +- crates/layout/src/dock.rs | 2 +- crates/layout/src/resize_handle.rs | 8 ++ crates/layout/tests/resize_handle.rs | 22 +++++ examples/gallery/src/main.rs | 14 +-- 20 files changed, 489 insertions(+), 136 deletions(-) diff --git a/crates/core/src/combobox.rs b/crates/core/src/combobox.rs index 2e19081..4bae38b 100644 --- a/crates/core/src/combobox.rs +++ b/crates/core/src/combobox.rs @@ -510,8 +510,12 @@ where if state.highlight.is_some() { state.highlight = None; } else { - let query = state.query.read(cx).value().to_string(); - state.refilter(&query); + if state.selected.is_some() { + state.refilter(""); + } else { + let query = state.query.read(cx).value().to_string(); + state.refilter(&query); + } state.highlight = (!state.visible.is_empty()).then_some(0usize); } cx.notify(); diff --git a/crates/core/src/command_palette.rs b/crates/core/src/command_palette.rs index 1fc6568..da6a388 100644 --- a/crates/core/src/command_palette.rs +++ b/crates/core/src/command_palette.rs @@ -1,14 +1,13 @@ //! The command palette: a modal query field over the application's actions. -use std::cell::RefCell; use std::rc::Rc; use gpui::prelude::FluentBuilder as _; use gpui::{ - AnyElement, App, Bounds, Context, ElementId, Entity, InteractiveElement as _, IntoElement, - KeyBinding, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Render, Role, - ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Window, - actions, canvas, div, px, relative, + AnyElement, App, Context, ElementId, Entity, InteractiveElement as _, IntoElement, KeyBinding, + MouseButton, ParentElement as _, Render, Role, ScrollHandle, SharedString, + StatefulInteractiveElement as _, Styled as _, Subscription, Window, actions, canvas, deferred, + div, px, relative, }; use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, Size}; @@ -322,6 +321,7 @@ impl Render for CommandPaletteState { let id = self.id.clone(); let state = cx.entity(); + let query_focus = self.query.read(cx).focus_handle(); let rows: Vec = self .matches @@ -406,9 +406,6 @@ impl Render for CommandPaletteState { .into_any_element() }; - let panel_bounds: Rc>>> = Rc::new(RefCell::new(None)); - let query_focus = self.query.read(cx).focus_handle(); - let panel = v_flex() .id((id.clone(), "panel")) .debug_selector({ @@ -460,20 +457,14 @@ impl Render for CommandPaletteState { ) .child(rows) .child( - canvas( - { - let cell = panel_bounds.clone(); - move |bounds, _window, _cx| *cell.borrow_mut() = Some(bounds) - }, - { - let focus = query_focus.clone(); - move |_, _, window, cx| { - if !focus.contains_focused(window, cx) { - window.focus(&focus, cx); - } + canvas(|_, _, _| {}, { + let focus = query_focus.clone(); + move |_, _, window, cx| { + if !focus.contains_focused(window, cx) { + window.focus(&focus, cx); } - }, - ) + } + }) .absolute() .inset_0(), ); @@ -485,28 +476,23 @@ impl Render for CommandPaletteState { .occlude() .on_mouse_down(MouseButton::Left, { let state = state.clone(); - let panel_bounds = panel_bounds.clone(); - move |event: &MouseDownEvent, _window, cx| { - let on_panel = panel_bounds - .borrow() - .is_some_and(|bounds| bounds.contains(&event.position)); - if on_panel { - return; - } + move |_, _window, cx| { state.update(cx, |state, cx| state.close(cx)); cx.stop_propagation(); } }); - div() - .absolute() - .inset_0() - .flex() - .justify_center() - .items_start() - .pt_10() - .child(scrim) - .child(panel) - .into_any_element() + deferred( + div() + .absolute() + .inset_0() + .flex() + .justify_center() + .items_start() + .pt_10() + .child(scrim) + .child(panel), + ) + .into_any_element() } } diff --git a/crates/core/src/dropdown_menu.rs b/crates/core/src/dropdown_menu.rs index 26a154b..d890356 100644 --- a/crates/core/src/dropdown_menu.rs +++ b/crates/core/src/dropdown_menu.rs @@ -207,7 +207,8 @@ impl RenderOnce for DropdownMenu { let a11y_label = self.a11y_label.or_else(|| self.label.clone()); let focus = focus_handle(&self.id, window, cx).tab_stop(self.tab_stop); // How the menu was last opened, read by the frame that mounts the panel. - let keyboard_open = window.use_keyed_state("dropdown-open-origin", cx, |_, _| false); + let keyboard_open = + window.use_keyed_state((self.id.clone(), "dropdown-open-origin"), cx, |_, _| false); let on_open: ChangeHandler = { let focus = focus.clone(); diff --git a/crates/core/src/input.rs b/crates/core/src/input.rs index b9c7015..d7ebb8d 100644 --- a/crates/core/src/input.rs +++ b/crates/core/src/input.rs @@ -1799,7 +1799,9 @@ impl EntityInputHandler for InputState { ) -> Option { let layout = self.layout.borrow(); let layout = layout.as_ref()?; - Some(real_index(&layout.mask_map, layout.index_at(window_point)).min(self.text.len())) + let index = + real_index(&layout.mask_map, layout.index_at(window_point)).min(self.text.len()); + Some(utf8_range_to_utf16(&self.text, index..index).start) } fn text_length_utf16( @@ -2516,7 +2518,9 @@ fn utf16_to_utf8_offset(text: &str, utf16_offset: usize) -> usize { /// Converts a UTF-8 byte range to a UTF-16 range. fn utf8_range_to_utf16(text: &str, range: Range) -> Range { - utf16_to_utf8_offset(text, range.start)..utf16_to_utf8_offset(text, range.end) + let start = text[..range.start].encode_utf16().count(); + let end = start + text[range.start..range.end].encode_utf16().count(); + start..end } /// Converts a UTF-16 range to a UTF-8 byte range. diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index cc5d414..70b5d27 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -382,7 +382,7 @@ pub(crate) fn anchored_menu_panel( chrome: &MenuChrome, enter_duration: std::time::Duration, ) -> Option { - let was_open = window.use_keyed_state("menu-was-open", cx, |_, _| false); + let was_open = window.use_keyed_state((id.clone(), "menu-was-open"), cx, |_, _| false); if !open { was_open.update(cx, |was_open, _| *was_open = false); @@ -392,8 +392,9 @@ pub(crate) fn anchored_menu_panel( let on_change = on_change.clone(); // Keyboard focus rides the panel across frames let focus = gpui_native_utils::focus_handle(&(id.clone(), "panel").into(), window, cx); - let scroll = window.use_keyed_state("menu-scroll", cx, |_, _| ScrollHandle::new()); - let selection = window.use_keyed_state("menu-selection", cx, |_, _| None); + let scroll = + window.use_keyed_state((id.clone(), "menu-scroll"), cx, |_, _| ScrollHandle::new()); + let selection = window.use_keyed_state((id.clone(), "menu-selection"), cx, |_, _| None); let selected = if *was_open.read(cx) { *selection.read(cx) diff --git a/crates/core/src/scrollbar.rs b/crates/core/src/scrollbar.rs index d63d77c..6ccbd6a 100644 --- a/crates/core/src/scrollbar.rs +++ b/crates/core/src/scrollbar.rs @@ -128,33 +128,35 @@ pub struct Scrollbar { axis: Axis, handle: Rc, visibility: ScrollbarVisibility, - key: ElementId, + id: ElementId, } impl Scrollbar { - /// A vertical scrollbar driven by `handle`. - pub fn vertical(handle: H) -> Self + /// A vertical scrollbar named by `id`, driven by `handle`. + pub fn vertical(id: I, handle: H) -> Self where + I: Into, H: ScrollbarHandle, { Self { axis: Axis::Vertical, handle: Rc::new(handle), visibility: ScrollbarVisibility::default(), - key: ElementId::Name("scrollbar".into()), + id: id.into(), } } - /// A horizontal scrollbar driven by `handle`. - pub fn horizontal(handle: H) -> Self + /// A horizontal scrollbar named by `id`, driven by `handle`. + pub fn horizontal(id: I, handle: H) -> Self where + I: Into, H: ScrollbarHandle, { Self { axis: Axis::Horizontal, handle: Rc::new(handle), visibility: ScrollbarVisibility::default(), - key: ElementId::Name("scrollbar".into()), + id: id.into(), } } @@ -163,12 +165,6 @@ impl Scrollbar { self.visibility = visibility; self } - - /// Names this scrollbar's state. - pub fn key(mut self, key: impl Into) -> Self { - self.key = key.into(); - self - } } impl RenderOnce for Scrollbar { @@ -177,8 +173,8 @@ impl RenderOnce for Scrollbar { let handle = self.handle; let axis = self.axis; let visibility = self.visibility; - let key = self.key; - move |_, _, window, cx| paint_scrollbar(&handle, axis, visibility, &key, window, cx) + let id = self.id; + move |_, _, window, cx| paint_scrollbar(&handle, axis, visibility, &id, window, cx) }) .absolute() .inset_0() @@ -192,7 +188,7 @@ fn paint_scrollbar( handle: &Rc, axis: Axis, visibility: ScrollbarVisibility, - key: &ElementId, + id: &ElementId, window: &mut Window, cx: &mut App, ) { @@ -217,12 +213,10 @@ fn paint_scrollbar( ScrollbarVisibility::Always => false, ScrollbarVisibility::Autohide => true, }; - let autohide_state = - autohide.then(|| Autohide::new(scrollbar_state_key("autohide", key), window, cx)); + autohide.then(|| Autohide::new(scrollbar_state_key("autohide", id), window, cx)); let track_length = viewport_along - inset * 2.0; - let (thumb_length, thumb_travel) = thumb_geometry( viewport_along, viewport_along + max_offset, @@ -267,9 +261,7 @@ fn paint_scrollbar( } }; - let drag = window.use_keyed_state(scrollbar_state_key("thumb-drag", key), cx, |_, _| { - Drag::None - }); + let drag = window.use_keyed_state(scrollbar_state_key("thumb-drag", id), cx, |_, _| Drag::None); let dragging = drag.read(cx).is_some(); let alpha = match &autohide_state { diff --git a/crates/core/src/slider.rs b/crates/core/src/slider.rs index 3a0173e..e246226 100644 --- a/crates/core/src/slider.rs +++ b/crates/core/src/slider.rs @@ -135,16 +135,12 @@ impl RenderOnce for Slider { 0.0 }; - // The track bounds, recorded by the overlay canvas each frame, - // turn pointer positions into values inside the listeners. let bounds_cell = Rc::new(Cell::new(None::>)); - // Dragging crosses frames, so it lives in keyed element state, - // a per-render flag would reset when the owner's update repaints. - let dragging = window.use_keyed_state("slider-dragging", cx, |_, _| false); - // A held handle gives the arrow-key listener something to check focus against, - // and the click-to-focus behavior a target. let focus_handle = gpui_native_utils::focus_handle(&self.id, window, cx); + let dragging = + window.use_keyed_state((self.id.clone(), "slider-dragging"), cx, |_, _| false); + let set_value: ValueSetter = { let bounds_cell = bounds_cell.clone(); let on_change = self.on_change.clone(); diff --git a/crates/core/tests/dropdown_menu.rs b/crates/core/tests/dropdown_menu.rs index 227b6c1..7f84de9 100644 --- a/crates/core/tests/dropdown_menu.rs +++ b/crates/core/tests/dropdown_menu.rs @@ -322,3 +322,136 @@ fn disabled_trigger_ignores_clicks() { assert!(changes.borrow().is_empty()); assert!(visual.debug_bounds("menu-panel").is_none()); } + +struct PairHarness { + open_a: bool, + open_b: bool, + changes_a: Changes, + changes_b: Changes, + activated_a: Activated, + activated_b: Activated, +} + +impl Render for PairHarness { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let (changes_a, changes_b) = (self.changes_a.clone(), self.changes_b.clone()); + let (activated_a, activated_b) = (self.activated_a.clone(), self.activated_b.clone()); + + let menu_a = DropdownMenu::new("pair-a") + .label("Alpha") + .variant(ButtonVariant::Secondary) + .open(self.open_a) + .on_open_change({ + let view = cx.entity(); + let changes = changes_a.clone(); + move |next, _, cx| { + changes.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.open_a = next; + cx.notify(); + }); + } + }) + .items(move |_, _| { + vec![ + MenuItem::new("png").handler({ + let activated = activated_a.clone(); + move |_, _| activated.borrow_mut().push("png") + }), + MenuItem::new("pdf").handler({ + let activated = activated_a.clone(); + move |_, _| activated.borrow_mut().push("pdf") + }), + ] + }); + + let menu_b = DropdownMenu::new("pair-b") + .label("Beta") + .variant(ButtonVariant::Secondary) + .open(self.open_b) + .on_open_change({ + let view = cx.entity(); + let changes = changes_b.clone(); + move |next, _, cx| { + changes.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.open_b = next; + cx.notify(); + }); + } + }) + .items(move |_, _| { + vec![ + MenuItem::new("zip").handler({ + let activated = activated_b.clone(); + move |_, _| activated.borrow_mut().push("zip") + }), + MenuItem::new("7z").handler({ + let activated = activated_b.clone(); + move |_, _| activated.borrow_mut().push("7z") + }), + ] + }); + + div().size_full().flex().gap_2().child(menu_a).child(menu_b) + } +} + +fn pair_window() -> ( + VisualTestContext, + AnyWindowHandle, + Changes, + Changes, + Activated, + Activated, +) { + let mut cx = TestAppContext::single(); + cx.update(Theme::init); + let changes_a = Rc::new(RefCell::new(Vec::new())); + let changes_b = Rc::new(RefCell::new(Vec::new())); + let activated_a = Rc::new(RefCell::new(Vec::new())); + let activated_b = Rc::new(RefCell::new(Vec::new())); + let (changes_a_for_window, changes_b_for_window) = (changes_a.clone(), changes_b.clone()); + let (activated_a_for_window, activated_b_for_window) = + (activated_a.clone(), activated_b.clone()); + let window = cx.add_window(move |_, _| PairHarness { + open_a: false, + open_b: false, + changes_a: changes_a_for_window, + changes_b: changes_b_for_window, + activated_a: activated_a_for_window, + activated_b: activated_b_for_window, + }); + let window = window.into(); + cx.update_window(window, |_, window, cx| { + window.draw(cx).clear(cx); + }) + .unwrap(); + ( + VisualTestContext::from_window(window, &cx), + window, + changes_a, + changes_b, + activated_a, + activated_b, + ) +} + +/// With a closed sibling dropdown on screen, a pointer-opened menu must still +/// arm rows with the keyboard: the menu's open/selection state is per +/// instance, not shared keyed state. +#[test] +fn sibling_dropdowns_keep_independent_menu_state() { + let (mut visual, window, changes_a, changes_b, activated_a, activated_b) = pair_window(); + + // Menu A sits at the window's top-left; menu B stays closed beside it. + click(&mut visual, point(px(30.), px(14.))); + assert_eq!(*changes_a.borrow(), vec![true]); + draw(&mut visual, window); + + visual.simulate_keystrokes("down enter"); + assert_eq!(*activated_a.borrow(), vec!["png"]); + assert_eq!(*changes_a.borrow(), vec![true, false]); + assert!(changes_b.borrow().is_empty(), "B must stay closed"); + assert!(activated_b.borrow().is_empty()); +} diff --git a/crates/core/tests/scrollbar.rs b/crates/core/tests/scrollbar.rs index 65fa880..d84c94d 100644 --- a/crates/core/tests/scrollbar.rs +++ b/crates/core/tests/scrollbar.rs @@ -38,7 +38,7 @@ impl Render for UniformShell { .h_full() .w_full(), ) - .child(Scrollbar::vertical(self.handle.clone())), + .child(Scrollbar::vertical("rows", self.handle.clone())), ) } } @@ -64,7 +64,7 @@ impl Render for ListShell { .h_full() .w_full(), ) - .child(Scrollbar::vertical(self.state.clone())), + .child(Scrollbar::vertical("rows", self.state.clone())), ) } } @@ -271,3 +271,99 @@ fn dragging_the_thumb_drives_a_list_state_and_reports_the_drag() { "the drag ends with the mouse" ); } + +struct TwoShell { + left: UniformListScrollHandle, + right: UniformListScrollHandle, +} + +impl Render for TwoShell { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + div() + .size_full() + .flex() + .flex_row() + .child( + div() + .relative() + .h(px(VIEWPORT_HEIGHT)) + .w(px(300.)) + .child( + uniform_list("left-rows", ROWS, |range, _, _| { + range + .map(|ix| div().h(px(ROW_HEIGHT)).child(format!("row {ix}"))) + .collect() + }) + .track_scroll(&self.left) + .h_full() + .w_full(), + ) + .child(Scrollbar::vertical("left", self.left.clone())), + ) + .child( + div() + .relative() + .h(px(VIEWPORT_HEIGHT)) + .w(px(300.)) + .child( + uniform_list("right-rows", ROWS, |range, _, _| { + range + .map(|ix| div().h(px(ROW_HEIGHT)).child(format!("row {ix}"))) + .collect() + }) + .track_scroll(&self.right) + .h_full() + .w_full(), + ) + .child(Scrollbar::vertical("right", self.right.clone())), + ) + } +} + +fn two_shell() -> ( + TestAppContext, + VisualTestContext, + UniformListScrollHandle, + UniformListScrollHandle, +) { + let mut cx = TestAppContext::single(); + cx.update(init); + let (left, right) = ( + UniformListScrollHandle::new(), + UniformListScrollHandle::new(), + ); + let (left_for_view, right_for_view) = (left.clone(), right.clone()); + let window = cx.add_window(move |_, _| TwoShell { + left: left_for_view, + right: right_for_view, + }); + let mut visual = VisualTestContext::from_window(window.into(), &cx); + visual.update(|window, cx| { + window.draw(cx).clear(cx); + }); + (cx, visual, left, right) +} + +/// Dragging one scrollbar's thumb must never move a sibling scrollbar: the +/// drag state is keyed per scrollbar id, not shared across instances. +#[test] +fn sibling_scrollbars_drag_independently() { + let (cx, mut visual, left, right) = two_shell(); + let metrics = Metrics::measure(&cx, &left); + assert!(metrics.max_offset > px(0.)); + + press(&mut visual, metrics.thumb_center()); + drag_to(&mut visual, metrics.full_drag_target()); + release(&mut visual, metrics.full_drag_target()); + + let offset = left.offset().y; + assert!( + (offset.abs() - metrics.max_offset).abs() < px(0.5), + "the left drag reaches the end, got {offset}" + ); + assert_eq!( + right.offset().y, + px(0.), + "the left drag leaked into the right scrollbar" + ); +} diff --git a/crates/core/tests/slider.rs b/crates/core/tests/slider.rs index a03db3a..d3df044 100644 --- a/crates/core/tests/slider.rs +++ b/crates/core/tests/slider.rs @@ -153,3 +153,132 @@ fn arrow_keys_step_the_value_while_focused() { assert_eq!(*changes.borrow(), vec![0.0, 10.0, 20.0, 10.0]); } + +struct TwoSliders { + changes_a: Rc>>, + changes_b: Rc>>, + value_a: f32, + value_b: f32, +} + +impl Render for TwoSliders { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let (changes_a, changes_b) = (self.changes_a.clone(), self.changes_b.clone()); + div() + .size_full() + .child( + div().absolute().top_0().left_0().w(px(200.)).child( + Slider::new("slider-a", self.value_a) + .min(0.0) + .max(100.0) + .step(10.0) + .on_change({ + let view = cx.entity(); + let changes_a = changes_a.clone(); + move |next, _, cx| { + changes_a.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.value_a = next; + cx.notify(); + }); + } + }), + ), + ) + .child( + div().absolute().top(px(40.)).left_0().w(px(200.)).child( + Slider::new("slider-b", self.value_b) + .min(0.0) + .max(100.0) + .step(10.0) + .on_change({ + let view = cx.entity(); + move |next, _, cx| { + changes_b.borrow_mut().push(next); + view.update(cx, |harness, cx| { + harness.value_b = next; + cx.notify(); + }); + } + }), + ), + ) + } +} + +#[allow(clippy::type_complexity)] +fn two_slider_window() -> ( + VisualTestContext, + Rc>>, + Rc>>, +) { + let mut cx = TestAppContext::single(); + cx.update(Theme::init); + let changes_a = Rc::new(RefCell::new(Vec::new())); + let changes_b = Rc::new(RefCell::new(Vec::new())); + let (changes_a_for_window, changes_b_for_window) = (changes_a.clone(), changes_b.clone()); + let window = cx.add_window(move |_, _| TwoSliders { + changes_a: changes_a_for_window, + changes_b: changes_b_for_window, + value_a: 0.0, + value_b: 0.0, + }); + let window = window.into(); + cx.update_window(window, |_, window, cx| { + window.draw(cx).clear(cx); + }) + .unwrap(); + ( + VisualTestContext::from_window(window, &cx), + changes_a, + changes_b, + ) +} + +/// Dragging one slider must never drive a sibling slider: the drag flag is +/// per instance, not shared keyed state. +#[test] +fn two_sliders_drag_independently() { + let (mut cx, changes_a, changes_b) = two_slider_window(); + + // Slider A occupies y 0..16, slider B starts at y 40. + cx.simulate_mouse_down( + point(px(100.), px(10.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_move( + point(px(120.), px(10.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_up( + point(px(120.), px(10.)), + MouseButton::Left, + Modifiers::default(), + ); + assert_eq!(*changes_a.borrow(), vec![50.0, 60.0]); + assert!(changes_b.borrow().is_empty(), "A's drag leaked into B"); + + cx.simulate_mouse_down( + point(px(100.), px(50.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_move( + point(px(40.), px(50.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_up( + point(px(40.), px(50.)), + MouseButton::Left, + Modifiers::default(), + ); + assert_eq!(*changes_b.borrow(), vec![50.0, 20.0]); + assert_eq!( + *changes_a.borrow(), + vec![50.0, 60.0], + "B's drag leaked into A" + ); +} diff --git a/crates/data/src/tabs.rs b/crates/data/src/tabs.rs index 841ff81..9ae9d69 100644 --- a/crates/data/src/tabs.rs +++ b/crates/data/src/tabs.rs @@ -32,6 +32,8 @@ pub enum TabsVariant { /// One tab of a [`Tabs`] strip. pub struct Tab { + /// The stable identity for element state; must survive reordering. + key: ElementId, label: SharedString, icon: Option, lead: Option, @@ -42,12 +44,14 @@ pub struct Tab { } impl Tab { - /// Creates a tab with this label. - pub fn new(label: L) -> Self + /// Creates a tab shown as `label`, identified by `key`. + pub fn new(key: K, label: L) -> Self where + K: Into, L: Into, { Self { + key: key.into(), label: label.into(), icon: None, lead: None, @@ -301,7 +305,7 @@ fn render_tab( }; let mut strip_tab = div() - .id(("tab", index)) + .id((tab.key.clone(), "tab")) .debug_selector({ let id = format!("{tabs_id}-tab-{}", tab.label); move || id @@ -364,7 +368,7 @@ fn render_tab( if tab.busy && !tab.disabled { strip_tab = strip_tab.child( - Spinner::new(("tab-busy", index)) + Spinner::new((tab.key.clone(), "tab-busy")) .small() .accessibility_label("Loading"), ); @@ -401,7 +405,7 @@ fn render_tab_close( index: usize, on_close: &Option, ) -> AnyElement { - Button::new(("tab-close", index)) + Button::new((tab.key.clone(), "tab-close")) .icon(IconRole::Close) .variant(ButtonVariant::Ghost) .small() diff --git a/crates/data/tests/tabs.rs b/crates/data/tests/tabs.rs index 2d8d555..dbeeaae 100644 --- a/crates/data/tests/tabs.rs +++ b/crates/data/tests/tabs.rs @@ -30,10 +30,10 @@ impl Render for Harness { }) .tabs(|_, _| { vec![ - Tab::new("One"), - Tab::new("Two"), - Tab::new("Off").disabled(true), - Tab::new("End"), + Tab::new("one", "One"), + Tab::new("two", "Two"), + Tab::new("off", "Off").disabled(true), + Tab::new("end", "End"), ] }), ) diff --git a/crates/editor/src/code_view.rs b/crates/editor/src/code_view.rs index 7520d38..b0335a1 100644 --- a/crates/editor/src/code_view.rs +++ b/crates/editor/src/code_view.rs @@ -13,7 +13,7 @@ use gpui::{ use gpui_native_core::scrollbar::{Scrollbar, ScrollbarHandle, ScrollbarVisibility}; use gpui_native_theme::{ActiveTheme as _, ColorTokens, Ramps}; -use crate::highlight::{HighlightedCode, Language, Role, highlight_with_tree}; +use crate::highlight::{HighlightedCode, Language, Role, highlight}; use crate::utils::{gutter_cell, run_style}; actions!(code_view, [Copy]); @@ -53,8 +53,6 @@ pub struct CodeViewState { source: SharedString, language: Language, code: HighlightedCode, - /// The parse tree of `source`, so updates reparse incrementally instead of from scratch. - tree: Option, handle: UniformListScrollHandle, /// The hovered line, driving the active-line wash and number emphasis. active_line: Option, @@ -74,14 +72,13 @@ impl CodeViewState { S: Into, { let source = source.into(); - let (code, tree) = highlight_with_tree(&source, language, None); + let code = highlight(&source, language); Self { id: id.into(), source, language, code, - tree, handle: UniformListScrollHandle::new(), active_line: None, selection: None, @@ -98,18 +95,9 @@ impl CodeViewState { { let source = source.into(); - // The old tree is only reusable when the grammar stays the same. - let old_tree = if language == self.language { - self.tree.as_ref() - } else { - None - }; - - let (code, tree) = highlight_with_tree(&source, language, old_tree); + self.code = highlight(&source, language); self.source = source; self.language = language; - self.code = code; - self.tree = tree; self.selection = None; self.handle.scroll_to_item(0, ScrollStrategy::Top); @@ -134,9 +122,7 @@ impl CodeViewState { lines.splice(line_ix..line_ix + 1, replacement); self.source = (lines.join("\n") + if trailing_newline { "\n" } else { "" }).into(); - let (code, tree) = highlight_with_tree(&self.source, self.language, self.tree.as_ref()); - self.code = code; - self.tree = tree; + self.code = highlight(&self.source, self.language); self.selection = None; cx.notify(); } @@ -348,7 +334,7 @@ impl Render for CodeViewState { code.position_at( *bounds, event.position, - Pixels::ZERO, + ScrollbarHandle::offset(&code.handle).y, line_height, char_width, text_x, @@ -453,7 +439,8 @@ impl Render for CodeViewState { ) .child(rows) .child( - Scrollbar::vertical(self.handle.clone()).visibility(ScrollbarVisibility::Platform), + Scrollbar::vertical((self.id.clone(), "v-scrollbar"), self.handle.clone()) + .visibility(ScrollbarVisibility::Platform), ) .when_some(self.a11y_label.clone(), |view, label| { view.aria_label(label) diff --git a/crates/editor/src/diff_view.rs b/crates/editor/src/diff_view.rs index 674b4cb..13e67cb 100644 --- a/crates/editor/src/diff_view.rs +++ b/crates/editor/src/diff_view.rs @@ -363,13 +363,11 @@ impl Render for DiffViewState { }) .child(rows) .child( - Scrollbar::vertical(self.handle.clone()) - .key("v-scrollbar") + Scrollbar::vertical((self.id.clone(), "v-scrollbar"), self.handle.clone()) .visibility(ScrollbarVisibility::Platform), ) .child( - Scrollbar::horizontal(self.handle.clone()) - .key("h-scrollbar") + Scrollbar::horizontal((self.id.clone(), "h-scrollbar"), self.handle.clone()) .visibility(ScrollbarVisibility::Platform), ) } diff --git a/crates/editor/src/highlight.rs b/crates/editor/src/highlight.rs index 9ccfbd2..45b87ae 100644 --- a/crates/editor/src/highlight.rs +++ b/crates/editor/src/highlight.rs @@ -67,17 +67,8 @@ pub struct HighlightedCode { /// Highlights `source` with `language`'s tree-sitter grammar. pub fn highlight(source: &str, language: Language) -> HighlightedCode { - highlight_with_tree(source, language, None).0 -} - -/// Highlights `source`, reusing `old_tree` so only the changed region is re-lexed. -pub fn highlight_with_tree( - source: &str, - language: Language, - old_tree: Option<&tree_sitter::Tree>, -) -> (HighlightedCode, Option) { let Some((grammar, query_source)) = grammar_for(language) else { - return (plain(source), None); + return plain(source); }; HIGHLIGHTERS.with(|cache| { @@ -104,11 +95,11 @@ pub fn highlight_with_tree( .as_mut(); let Some(highlighter) = highlighter else { - return (plain(source), None); + return plain(source); }; - let Some(tree) = highlighter.parser.parse(source, old_tree) else { - return (plain(source), None); + let Some(tree) = highlighter.parser.parse(source, None) else { + return plain(source); }; // One role per byte; matches arrive in query order @@ -134,7 +125,7 @@ pub fn highlight_with_tree( } } - (build_lines(source, &role_at), Some(tree)) + build_lines(source, &role_at) }) } diff --git a/crates/editor/src/markdown.rs b/crates/editor/src/markdown.rs index 924df75..6c5b0e5 100644 --- a/crates/editor/src/markdown.rs +++ b/crates/editor/src/markdown.rs @@ -18,7 +18,7 @@ use pulldown_cmark::{CodeBlockKind, Event, HeadingLevel, Options, Parser, Tag, T use std::ops::Range; -use crate::highlight::{HighlightedCode, highlight_with_tree, plain}; +use crate::highlight::{HighlightedCode, highlight, plain}; use crate::utils::{code_line, grammar_language}; /// Heading scale steps relative to the root font size, H1 first. @@ -277,7 +277,8 @@ impl Render for MarkdownViewState { .size_full() .child(blocks) .child( - Scrollbar::vertical(self.handle.clone()).visibility(ScrollbarVisibility::Platform), + Scrollbar::vertical((self.id.clone(), "v-scrollbar"), self.handle.clone()) + .visibility(ScrollbarVisibility::Platform), ) .when_some(self.a11y_label.clone(), |view, label| { view.aria_label(label) @@ -588,7 +589,7 @@ fn parse(source: &str, highlight_code: bool) -> Vec { .then(|| language.as_deref().and_then(grammar_language)) .flatten() { - Some(grammar) => highlight_with_tree(text, grammar, None).0, + Some(grammar) => highlight(text, grammar), None => plain(text), }; blocks.push(MarkdownBlock::Code { diff --git a/crates/layout/src/dock.rs b/crates/layout/src/dock.rs index d4de65d..46d5847 100644 --- a/crates/layout/src/dock.rs +++ b/crates/layout/src/dock.rs @@ -1016,7 +1016,7 @@ fn render_pane( move |_, _| { tabs.iter() .map(|tab| { - let mut entry = Tab::new(tab.title.clone()); + let mut entry = Tab::new(tab.panel.clone(), tab.title.clone()); if let Some(role) = tab.icon { entry = entry.icon(role); } diff --git a/crates/layout/src/resize_handle.rs b/crates/layout/src/resize_handle.rs index a02d99f..377cad1 100644 --- a/crates/layout/src/resize_handle.rs +++ b/crates/layout/src/resize_handle.rs @@ -255,6 +255,14 @@ impl RenderOnce for ResizeHandle { return; } + if event.pressed_button != Some(MouseButton::Left) { + drag.update(cx, |drag, cx| { + drag.dragging = false; + cx.notify(); + }); + return; + } + let delta = match axis { Axis::Horizontal => event.position.x - last.x, Axis::Vertical => event.position.y - last.y, diff --git a/crates/layout/tests/resize_handle.rs b/crates/layout/tests/resize_handle.rs index 860f4d9..a53df18 100644 --- a/crates/layout/tests/resize_handle.rs +++ b/crates/layout/tests/resize_handle.rs @@ -151,6 +151,28 @@ fn moves_without_a_drag_report_nothing() { assert!(deltas.borrow().is_empty()); } +/// The button may be released outside the window, where no MouseUp is +/// delivered — the drag must end when a move reports no pressed button. +#[test] +fn releasing_outside_the_window_ends_the_drag() { + let (_cx, deltas, _resets, mut visual) = handle_window(Axis::Horizontal, false); + let start = handle_center(&mut visual); + + visual.simulate_mouse_down(start, MouseButton::Left, Modifiers::default()); + visual.simulate_event(MouseMoveEvent { + position: point(start.x + px(30.), start.y), + pressed_button: None, + modifiers: Modifiers::default(), + }); + visual.simulate_event(MouseMoveEvent { + position: point(start.x + px(60.), start.y), + pressed_button: None, + modifiers: Modifiers::default(), + }); + + assert!(deltas.borrow().is_empty(), "the drag outlived its button"); +} + #[test] fn arrow_keys_move_the_seam_when_focused() { let (_cx, deltas, _resets, mut visual) = handle_window(Axis::Horizontal, false); diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 6f1a053..23dbe46 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -2309,10 +2309,10 @@ impl Render for TabsDemo { }) .tabs(|_, _| { vec![ - Tab::new("Overview"), - Tab::new("Details"), - Tab::new("Settings"), - Tab::new("Locked").disabled(true), + Tab::new("overview", "Overview"), + Tab::new("details", "Details"), + Tab::new("settings", "Settings"), + Tab::new("locked", "Locked").disabled(true), ] }), )) @@ -2356,7 +2356,7 @@ impl Render for TabsDemo { labels .iter() .map(|label| { - Tab::new(label.clone()) + Tab::new(label.clone(), label.clone()) .closeable(true) .busy(label == "Syncing") }) @@ -3228,7 +3228,7 @@ impl Render for ScrollbarDemo { .h_full() .w_full(), ) - .child(Scrollbar::vertical(self.uniform.clone()).visibility(visibility)), + .child(Scrollbar::vertical("uniform", self.uniform.clone()).visibility(visibility)), )) .child(demo_section( "list", @@ -3254,7 +3254,7 @@ impl Render for ScrollbarDemo { .h_full() .w_full(), ) - .child(Scrollbar::vertical(self.list.clone()).visibility(visibility)), + .child(Scrollbar::vertical("list", self.list.clone()).visibility(visibility)), )) .child( div() -- 2.54.0 From 122f9b12ae8f9c9d7d0572f8d5252ca2e1b9c0fe Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Mon, 9 Nov 2026 14:35:15 +0700 Subject: [PATCH 3/5] Reduce render allocations and redundant work in core components --- crates/core/src/alert.rs | 2 +- crates/core/src/combobox.rs | 8 +++--- crates/core/src/command_palette.rs | 8 +++--- crates/core/src/dropdown_menu.rs | 6 ++-- crates/core/src/field.rs | 12 ++++---- crates/core/src/input.rs | 6 +++- crates/core/src/menu.rs | 8 +++--- crates/core/src/segmented.rs | 46 +++++++++++++++++------------- crates/core/src/select.rs | 24 ++++++---------- crates/core/src/switch.rs | 5 +--- crates/core/src/tooltip.rs | 16 ++++++----- crates/core/tests/field_bench.rs | 45 +++++++++++++++++++++++------ crates/data/src/sidebar.rs | 6 ++-- crates/data/src/tabs.rs | 10 ++++--- crates/editor/src/code_view.rs | 11 +++---- crates/editor/src/diff_view.rs | 38 ++++++++++++++++-------- crates/editor/src/markdown.rs | 4 +-- crates/layout/src/dock.rs | 24 ++++++---------- crates/utils/src/lib.rs | 34 +++++++++++----------- 19 files changed, 174 insertions(+), 139 deletions(-) diff --git a/crates/core/src/alert.rs b/crates/core/src/alert.rs index 57eb08c..f74eb28 100644 --- a/crates/core/src/alert.rs +++ b/crates/core/src/alert.rs @@ -304,7 +304,7 @@ impl RenderOnce for AlertsView { let theme = cx.theme(); let enter_duration = theme.motion.overlay; - let entries: Vec = self.alerts.read(cx).entries().to_vec(); + let entries = self.alerts.read(cx).entries(); deferred( div() diff --git a/crates/core/src/combobox.rs b/crates/core/src/combobox.rs index 4bae38b..3367308 100644 --- a/crates/core/src/combobox.rs +++ b/crates/core/src/combobox.rs @@ -604,8 +604,8 @@ where div() .id(("option", row)) .debug_selector({ - let name = format!("{id}-option-{row}"); - move || name + let id = id.clone(); + move || format!("{id}-option-{row}") }) .flex() .items_center() @@ -681,8 +681,8 @@ where let panel = v_flex() .id((id.clone(), "panel")) .debug_selector({ - let name = format!("{id}-panel"); - move || name + let id = id.clone(); + move || format!("{id}-panel") }) .w(popup_width) .p_1() diff --git a/crates/core/src/command_palette.rs b/crates/core/src/command_palette.rs index da6a388..be2a201 100644 --- a/crates/core/src/command_palette.rs +++ b/crates/core/src/command_palette.rs @@ -335,8 +335,8 @@ impl Render for CommandPaletteState { h_flex() .id(("command", row)) .debug_selector({ - let name = format!("{id}-command-{row}"); - move || name + let id = id.clone(); + move || format!("{id}-command-{row}") }) .gap_2() .px_2() @@ -409,8 +409,8 @@ impl Render for CommandPaletteState { let panel = v_flex() .id((id.clone(), "panel")) .debug_selector({ - let name = format!("{id}-panel"); - move || name + let id = id.clone(); + move || format!("{id}-panel") }) .key_context(COMMAND_PALETTE_CONTEXT) .role(Role::Dialog) diff --git a/crates/core/src/dropdown_menu.rs b/crates/core/src/dropdown_menu.rs index d890356..8873e83 100644 --- a/crates/core/src/dropdown_menu.rs +++ b/crates/core/src/dropdown_menu.rs @@ -422,10 +422,10 @@ impl RenderOnce for DropdownMenu { let items = if self.open { self.items - .map(|build| build(window, cx)) - .unwrap_or_default() + .map(|build| Rc::from(build(window, cx))) + .unwrap_or_else(|| Rc::from([])) } else { - Vec::new() + Rc::from([]) }; if let Some(panel) = anchored_menu_panel( diff --git a/crates/core/src/field.rs b/crates/core/src/field.rs index e3ccc3a..bcad282 100644 --- a/crates/core/src/field.rs +++ b/crates/core/src/field.rs @@ -95,8 +95,8 @@ impl RenderOnce for Field { let title = div() .debug_selector({ - let name = format!("{id}-label"); - move || name + let id = id.clone(); + move || format!("{id}-label") }) .text_size(metrics.text_size) .child(self.label.clone()); @@ -105,16 +105,16 @@ impl RenderOnce for Field { .items_center() .gap_2() .debug_selector({ - let name = format!("{id}-control"); - move || name + let id = id.clone(); + move || format!("{id}-control") }) .children(self.control); let error = self.error.map(|error| { div() .debug_selector({ - let name = format!("{id}-error"); - move || name + let id = id.clone(); + move || format!("{id}-error") }) .text_size(metrics.caption_size) .text_color(error_color) diff --git a/crates/core/src/input.rs b/crates/core/src/input.rs index d7ebb8d..0577f27 100644 --- a/crates/core/src/input.rs +++ b/crates/core/src/input.rs @@ -2051,7 +2051,11 @@ impl RenderOnce for Input { canvas .with_animation( animation_id, - Animation::new(CARET_BLINK_INTERVAL * 2).repeat(), + // The blink needs four repaints per period, + // not the display's refresh rate. + Animation::new(CARET_BLINK_INTERVAL * 2) + .repeat() + .with_max_fps(4.0), |canvas, _| canvas, ) .into_any_element() diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index 70b5d27..2bffa8c 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -336,11 +336,11 @@ impl RenderOnce for Menu { let items = if self.open { match self.items { - Some(build) => build(window, cx), - None => Vec::new(), + Some(build) => Rc::from(build(window, cx)), + None => Rc::from([]), } } else { - Vec::new() + Rc::from([]) }; if let Some(panel) = anchored_menu_panel( @@ -372,7 +372,7 @@ pub(crate) fn anchored_menu_panel( cx: &mut App, id: &ElementId, open: bool, - items: Vec, + items: Rc<[MenuItem]>, initial_selection: Option, preselect: bool, on_change: Option, diff --git a/crates/core/src/segmented.rs b/crates/core/src/segmented.rs index 5142e9d..7e52a82 100644 --- a/crates/core/src/segmented.rs +++ b/crates/core/src/segmented.rs @@ -9,7 +9,7 @@ use gpui::{ }; use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, Size}; -use gpui_native_utils::{adjacent_selectable, focus_handle, focus_ring}; +use gpui_native_utils::{focus_handle, focus_ring}; use crate::common::{Sizable, StyledExt, h_flex}; @@ -64,7 +64,8 @@ where T: Clone + PartialEq + 'static, { id: ElementId, - options: Vec>, + /// Shared with the per-segment and key handlers, so captures are bumps. + options: Rc<[SegmentedOption]>, selected: T, a11y_label: Option, size: Size, @@ -82,7 +83,7 @@ where { Self { id: id.into(), - options: Vec::new(), + options: Rc::from([]), selected, a11y_label: None, size: Size::Small, @@ -163,12 +164,12 @@ where .options .iter() .position(|segment| segment.value == self.selected); + let option_count = self.options.len(); - let selectable: Vec = (0..self.options.len()).collect(); let on_change = self.on_change.clone(); let key_on_change = on_change.clone(); let key_options = self.options.clone(); - let key_selectable = selectable.clone(); + let options = self.options.clone(); let label = self .a11y_label @@ -176,14 +177,13 @@ where .unwrap_or_else(|| SharedString::from(self.id.to_string())); window.with_element_namespace(self.id.clone(), |window| { - let focus = focus_handle(&self.id, window, cx); - let geometry: Entity>>> = window.use_keyed_state("segmented-geometry", cx, |_, _| Vec::new()); let track_origin: Entity>> = window.use_keyed_state("segmented-track-origin", cx, |_, _| None); + let focus = focus_handle(&self.id, window, cx); let anim_config = SpringConfig::new(500., 35., 1.); let track_point = *track_origin.read(cx); @@ -197,12 +197,13 @@ where let segments = self .options - .into_iter() + .iter() .enumerate() .map(move |(index, segment)| { let is_selected = Some(index) == selected_index; let on_change = on_change.clone(); - let value = segment.value; + let options = options.clone(); + let content_color = if is_selected { foreground } else { @@ -228,7 +229,7 @@ where .when_some(segment.icon, |segment, role| { segment.child(icon(role).size(icon_size).color(content_color)) }) - .child(segment.label) + .child(segment.label.clone()) .child( canvas( { @@ -251,8 +252,9 @@ where move |_, window, cx| { if Some(index) != selected_index && let Some(on_change) = &on_change + && let Some(option) = options.get(index) { - on_change(value.clone(), window, cx); + on_change(option.value.clone(), window, cx); } } }) @@ -319,22 +321,26 @@ where .on_key_down({ move |event: &KeyDownEvent, window, cx| { let target = match event.keystroke.key.as_str() { - "right" => { - adjacent_selectable(&key_selectable, selected_index, true) - } - "left" => { - adjacent_selectable(&key_selectable, selected_index, false) - } - "home" => key_selectable.first().copied(), - "end" => key_selectable.last().copied(), + "right" => match selected_index { + Some(index) => { + index.checked_add(1).filter(|next| *next < option_count) + } + None => (option_count > 0).then_some(0), + }, + "left" => selected_index.and_then(|index| index.checked_sub(1)), + "home" => (option_count > 0).then_some(0), + "end" => option_count.checked_sub(1), _ => return, }; + if let Some(next) = target && Some(next) != selected_index && let Some(on_change) = &key_on_change + && let Some(option) = key_options.get(next) { - on_change(key_options[next].value.clone(), window, cx); + on_change(option.value.clone(), window, cx); } + cx.stop_propagation(); } }) diff --git a/crates/core/src/select.rs b/crates/core/src/select.rs index 2acea6c..cddaca5 100644 --- a/crates/core/src/select.rs +++ b/crates/core/src/select.rs @@ -206,24 +206,18 @@ where .position(|option| &option.value == value) }); - let trigger_options = self.options.clone(); - let trigger_selected = self.selected.clone(); - let trigger_placeholder = self.placeholder.clone(); + // The trigger's text is fixed at render time; only it moves into the closure. + let trigger_text = selected_index + .and_then(|index| self.options.get(index)) + .map(|option| option.label.clone()) + .unwrap_or_else(|| self.placeholder.clone()); + + let trigger_has_value = self.selected.is_some(); let trigger_id = self.id.clone(); let trigger_label = label.clone(); let trigger_style = self.style; let trigger = move |_window: &mut Window, _cx: &mut App| { - let text = trigger_selected - .as_ref() - .and_then(|value| { - trigger_options - .iter() - .find(|option| &option.value == value) - .map(|option| option.label.clone()) - }) - .unwrap_or_else(|| trigger_placeholder.clone()); - h_flex() .id((trigger_id.clone(), "select-trigger")) .debug_selector(|| "select-trigger".to_string()) @@ -240,7 +234,7 @@ where .border_color(rest_border) .bg(input) .text_size(metrics.text_size) - .text_color(if trigger_selected.is_some() { + .text_color(if trigger_has_value { foreground } else { muted_foreground @@ -249,7 +243,7 @@ where trigger.shadow(focus_ring(ring)) }) .hover(|style| style.bg(muted).border_color(hover_border)) - .child(text) + .child(trigger_text.clone()) .child( icon(IconRole::ChevronDown) .size(icon_size) diff --git a/crates/core/src/switch.rs b/crates/core/src/switch.rs index 23f1d27..f09c216 100644 --- a/crates/core/src/switch.rs +++ b/crates/core/src/switch.rs @@ -133,10 +133,7 @@ impl RenderOnce for Switch { div() .flex_none() .relative() - .debug_selector({ - let name = format!("{id}-track"); - move || name - }) + .debug_selector(move || format!("{id}-track")) .w(track_width) .h(track_height) .rounded(theme.radius.full) diff --git a/crates/core/src/tooltip.rs b/crates/core/src/tooltip.rs index 31c1e2b..26e7d4f 100644 --- a/crates/core/src/tooltip.rs +++ b/crates/core/src/tooltip.rs @@ -34,8 +34,8 @@ impl Tooltip { { let text = text.into(); let anchor_id = anchor_id.into(); - move |_window, cx| { - let anchor = anchor_bounds(&anchor_id, cx); + move |window, cx| { + let anchor = anchor_bounds(window.window_handle().window_id(), &anchor_id, cx); cx.new(|_| Self { text: text.clone(), anchor, @@ -92,11 +92,13 @@ impl Render for Tooltip { let origin = window.mouse_position() + point(px(1.), px(1.)); let viewport_height = window.viewport_size().height; - let height = anchor_bounds(&ElementId::from(PANEL_BOUNDS_ID), cx) - .map(|bounds| bounds.size.height) - .unwrap_or_else(|| { - text_size * 1.5 + window.rem_size() * 0.5 + BORDER_WIDTH * 2.0 - }); + let height = anchor_bounds( + window.window_handle().window_id(), + &ElementId::from(PANEL_BOUNDS_ID), + cx, + ) + .map(|bounds| bounds.size.height) + .unwrap_or_else(|| text_size * 1.5 + window.rem_size() * 0.5 + BORDER_WIDTH * 2.0); let below = viewport_height - spacing_unit - parent.bottom(); let above = parent.top() - spacing_unit; diff --git a/crates/core/tests/field_bench.rs b/crates/core/tests/field_bench.rs index dd299a5..5b07fe0 100644 --- a/crates/core/tests/field_bench.rs +++ b/crates/core/tests/field_bench.rs @@ -1,3 +1,5 @@ +use std::{cell::Cell, rc::Rc, time::Duration}; + use gpui::{ AppContext as _, Context, Entity, IntoElement, ParentElement as _, Render, SharedString, Styled as _, TestAppContext, VisualTestContext, Window, div, @@ -9,10 +11,12 @@ use gpui_native_core::input::{Input, InputState}; struct FieldHost { url: Entity, + renders: Rc>, } impl Render for FieldHost { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + self.renders.set(self.renders.get() + 1); let url_error = self .url .read(cx) @@ -34,15 +38,27 @@ impl Render for FieldHost { /// A field page in a real window; the url input is focused unless /// `focus` is false, which leaves the page truly idle. -fn field_host(focus: bool) -> (TestAppContext, Entity, VisualTestContext) { +fn field_host( + focus: bool, +) -> ( + TestAppContext, + Entity, + VisualTestContext, + Rc>, +) { let cx = TestAppContext::single(); cx.update(init); + let renders = Rc::new(Cell::new(0)); + let renders_for_host = renders.clone(); let (url, window) = cx.update(|cx| { let url = InputState::new(cx); let url_for_host = url.clone(); let window = cx .open_window(gpui::WindowOptions::default(), |_, cx| { - cx.new(|_| FieldHost { url: url_for_host }) + cx.new(|_| FieldHost { + url: url_for_host, + renders: renders_for_host.clone(), + }) }) .unwrap(); (url, window) @@ -55,12 +71,12 @@ fn field_host(focus: bool) -> (TestAppContext, Entity, VisualTestCon } window.draw(cx).clear(cx); }); - (cx, url, visual) + (cx, url, visual, renders) } #[test] fn an_idle_field_page_schedules_no_frames() { - let (_cx, _url, mut visual) = field_host(false); + let (_cx, _url, mut visual, _renders) = field_host(false); let frames = visual.update(|window, cx| window.simulate_next_frame(cx)); assert_eq!( frames, 0, @@ -68,12 +84,25 @@ fn an_idle_field_page_schedules_no_frames() { ); } +/// The caret blink is fps-capped: it must never demand a next frame, and its +/// redraws arrive on the executor timer at the blink cadence. #[test] -fn a_focused_field_page_schedules_frames_for_its_blink() { - let (_cx, _url, mut visual) = field_host(true); +fn a_focused_field_blink_is_fps_capped_and_still_redraws() { + let (cx, _url, mut visual, renders) = field_host(true); + let renders_after_focus = renders.get(); + let frames = visual.update(|window, cx| window.simulate_next_frame(cx)); + assert_eq!( + frames, 0, + "the blink must be fps-capped, not per-frame scheduled" + ); + + // The blink redraws through the executor timer (4 fps cap). + cx.executor().advance_clock(Duration::from_millis(251)); + cx.run_until_parked(); + visual.update(|window, cx| window.draw(cx).clear(cx)); assert!( - frames > 0, - "the focused caret's blink needs frames to redraw at the blink cadence" + renders.get() > renders_after_focus, + "the capped blink must still redraw its page" ); } diff --git a/crates/data/src/sidebar.rs b/crates/data/src/sidebar.rs index 2b8abfa..8d80bef 100644 --- a/crates/data/src/sidebar.rs +++ b/crates/data/src/sidebar.rs @@ -195,7 +195,6 @@ impl RenderOnce for Sidebar { .clone() .unwrap_or_else(|| SharedString::from(self.id.to_string())); - let id_text = self.id.to_string(); let scroll_area_id = (self.id.clone(), "entries"); // Entry paint reads the theme through this borrow. @@ -216,9 +215,10 @@ impl RenderOnce for Sidebar { }) .collect(); + let id = self.id; div() - .id(self.id) - .debug_selector(move || format!("sidebar-{id_text}")) + .id(id.clone()) + .debug_selector(move || format!("sidebar-{id}")) .size_full() .flex() .flex_col() diff --git a/crates/data/src/tabs.rs b/crates/data/src/tabs.rs index 9ae9d69..fe13093 100644 --- a/crates/data/src/tabs.rs +++ b/crates/data/src/tabs.rs @@ -307,8 +307,9 @@ fn render_tab( let mut strip_tab = div() .id((tab.key.clone(), "tab")) .debug_selector({ - let id = format!("{tabs_id}-tab-{}", tab.label); - move || id + let tabs_id = tabs_id.clone(); + let label = tab.label.clone(); + move || format!("{tabs_id}-tab-{}", label) }) .flex() .items_center() @@ -411,8 +412,9 @@ fn render_tab_close( .small() .tab_stop(false) .debug_selector({ - let id = format!("{tabs_id}-close-{}", tab.label); - move || id.clone() + let tabs_id = tabs_id.clone(); + let label = tab.label.clone(); + move || format!("{tabs_id}-close-{}", label) }) .tooltip(Tooltip::text("Close")) .accessibility_label(SharedString::from(format!("Close {}", tab.label))) diff --git a/crates/editor/src/code_view.rs b/crates/editor/src/code_view.rs index b0335a1..41d03aa 100644 --- a/crates/editor/src/code_view.rs +++ b/crates/editor/src/code_view.rs @@ -462,10 +462,10 @@ fn render_row( line_height: Pixels, cx: &App, ) -> gpui::AnyElement { - let line = code.code().lines.get(line_ix).cloned().unwrap_or_default(); + let line = code.code().lines.get(line_ix); let mut text = String::new(); - for span in &line.spans { + for span in line.map_or(&[][..], |line| line.spans.as_slice()) { text.push_str(&span.text); } @@ -473,7 +473,7 @@ fn render_row( let mut roles = vec![Role::Text; char_count]; let mut offset = 0usize; - for span in &line.spans { + for span in line.map_or(&[][..], |line| line.spans.as_slice()) { let chars = span.text.chars().count(); for slot in &mut roles[offset..offset + chars] { *slot = span.role; @@ -543,10 +543,7 @@ fn render_row( }; div() - .debug_selector({ - let name = format!("{id}-row-{line_ix}"); - move || name - }) + .debug_selector(move || format!("{id}-row-{line_ix}")) .id(("code-row", line_ix)) .w_full() .h(line_height) diff --git a/crates/editor/src/diff_view.rs b/crates/editor/src/diff_view.rs index 13e67cb..1395e75 100644 --- a/crates/editor/src/diff_view.rs +++ b/crates/editor/src/diff_view.rs @@ -237,6 +237,8 @@ pub fn diff_rows(old: &str, new: &str, language: Language) -> Vec { pub struct DiffViewState { id: ElementId, rows: Vec, + /// The row whose width drives horizontal scroll extent, `rows`-derived. + widest_row: Option, options: DiffViewOptions, /// Drives both scroll axes of the virtualized row list. handle: UniformListScrollHandle, @@ -251,13 +253,32 @@ impl DiffViewState { O: AsRef, N: AsRef, { - Self { + let mut state = Self { id: id.into(), - rows: diff_rows(old.as_ref(), new.as_ref(), language), + rows: Vec::new(), + widest_row: None, options: DiffViewOptions::default(), handle: UniformListScrollHandle::new(), a11y_label: None, - } + }; + state.set_rows(old, new, language); + state + } + + /// Replaces the rows, recomputing `widest_row` with them. + fn set_rows(&mut self, old: O, new: N, language: Language) + where + O: AsRef, + N: AsRef, + { + self.rows = diff_rows(old.as_ref(), new.as_ref(), language); + self.widest_row = self + .rows + .iter() + .enumerate() + .filter(|(_, row)| row.kind != DiffRowKind::Hunk) + .max_by_key(|(_, row)| row.char_count()) + .map(|(ix, _)| ix); } /// Sets how changed rows are marked. @@ -284,7 +305,7 @@ impl DiffViewState { O: AsRef, N: AsRef, { - self.rows = diff_rows(old.as_ref(), new.as_ref(), language); + self.set_rows(old, new, language); self.handle.scroll_to_item(0, ScrollStrategy::Top); // The replaced versions rarely share the old width; start at the left. let offset = ScrollbarHandle::offset(&self.handle); @@ -328,14 +349,7 @@ impl Render for DiffViewState { let state = cx.entity(); let row_count = self.rows.len(); let options = self.options; - - let widest_row = self - .rows - .iter() - .enumerate() - .filter(|(_, row)| row.kind != DiffRowKind::Hunk) - .max_by_key(|(_, row)| row.char_count()) - .map(|(ix, _)| ix); + let widest_row = self.widest_row; let mut rows = uniform_list((id.clone(), "rows"), row_count, { move |range, _window, cx| { diff --git a/crates/editor/src/markdown.rs b/crates/editor/src/markdown.rs index 6c5b0e5..0e54087 100644 --- a/crates/editor/src/markdown.rs +++ b/crates/editor/src/markdown.rs @@ -784,8 +784,8 @@ fn block_frame( let theme = cx.theme(); let content = div() .debug_selector({ - let name = SharedString::from(format!("{id}")); - move || name.to_string() + let id = id.clone(); + move || id.to_string() }) .w_full() .flex() diff --git a/crates/layout/src/dock.rs b/crates/layout/src/dock.rs index 46d5847..ab4fd5e 100644 --- a/crates/layout/src/dock.rs +++ b/crates/layout/src/dock.rs @@ -969,15 +969,10 @@ fn render_pane( .map(|tab| tab.view.clone()); // The cache keeps an un-notified panel from re-rendering on dock frames. + let content_id = SharedString::from(format!("{dock_id}-content-{}", pane.key)); let content = div() - .id(SharedString::from(format!( - "{dock_id}-content-{}", - pane.key - ))) - .debug_selector({ - let id = SharedString::from(format!("{dock_id}-content-{}", pane.key)); - move || id.to_string() - }) + .id(content_id.clone()) + .debug_selector(move || content_id.to_string()) .flex_1() .min_h_0() .overflow_hidden() @@ -999,6 +994,7 @@ fn render_pane( let pane = pane.clone(); let titlebar = *titlebar; let padding_x = metrics.dense_padding_x; + move |window, cx| { let selected = pane .active @@ -1065,15 +1061,11 @@ fn render_pane( } }); + let bar_id = SharedString::from(format!("{dock_id}-pane-{}-bar", pane.key)); + h_flex() - .id(SharedString::from(format!( - "{dock_id}-pane-{}-bar", - pane.key - ))) - .debug_selector({ - let id = SharedString::from(format!("{dock_id}-pane-{}-bar", pane.key)); - move || id.to_string() - }) + .id(bar_id.clone()) + .debug_selector(move || bar_id.to_string()) .flex_none() .w_full() .h(titlebar.height) diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index fd350ea..05f5992 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -1,7 +1,8 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ App, Bounds, BoxShadow, Div, ElementId, FocusHandle, Global, Hsla, IntoElement, Pixels, - Refineable as _, StatefulInteractiveElement, StyleRefinement, Styled, Window, div, px, + Refineable as _, StatefulInteractiveElement, StyleRefinement, Styled, Window, WindowId, div, + px, }; use gpui_native_theme::ActiveTheme as _; use std::collections::HashMap; @@ -36,39 +37,36 @@ pub fn focus_handle(id: &ElementId, window: &mut Window, cx: &mut App) -> FocusH .clone() } -/// The painted bounds of elements that tooltips anchor to, keyed by element id. -#[derive(Clone, Default)] -struct AnchorBoundsRegistry(HashMap>); +/// The painted bounds of elements that tooltips anchor to, per window and element id. +#[derive(Default)] +struct AnchorBoundsRegistry(HashMap<(WindowId, ElementId), Bounds>); impl Global for AnchorBoundsRegistry {} impl AnchorBoundsRegistry { - fn record(&mut self, id: ElementId, bounds: Bounds) { - self.0.insert(id, bounds); + fn record(&mut self, window: WindowId, id: ElementId, bounds: Bounds) { + self.0.insert((window, id), bounds); } - fn get(&self, id: &ElementId) -> Option> { - self.0.get(id).copied() + fn get(&self, window: WindowId, id: &ElementId) -> Option> { + self.0.get(&(window, id.clone())).copied() } } -/// The last bounds an element painted at. -pub fn anchor_bounds(id: &ElementId, cx: &mut App) -> Option> { +/// The last bounds an element painted at, in the given window. +pub fn anchor_bounds(window: WindowId, id: &ElementId, cx: &App) -> Option> { cx.try_global::() - .and_then(|registry| registry.get(id)) + .and_then(|registry| registry.get(window, id)) } /// A canvas prepaint closure recording the element's painted bounds. pub fn record_anchor_bounds( id: ElementId, ) -> impl Fn(Bounds, &mut Window, &mut App) + 'static { - move |bounds, _window, cx| { - let mut registry = cx - .try_global::() - .cloned() - .unwrap_or_default(); - registry.record(id.clone(), bounds); - cx.set_global(registry); + move |bounds, window, cx| { + let window_id = window.window_handle().window_id(); + cx.default_global::() + .record(window_id, id.clone(), bounds); } } -- 2.54.0 From 2e7711ee2e6a3c2041395586616a452ca13a0483 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Mon, 9 Nov 2026 15:59:13 +0700 Subject: [PATCH 4/5] Move layout metrics into theme and apply audit cleanups --- crates/core/src/alert.rs | 16 +- crates/core/src/avatar.rs | 29 +- crates/core/src/button.rs | 8 +- crates/core/src/combobox.rs | 38 +-- crates/core/src/command_palette.rs | 8 +- crates/core/src/dialog.rs | 41 +-- crates/core/src/dropdown_menu.rs | 4 +- crates/core/src/input.rs | 219 ++++++------ crates/core/src/menu.rs | 14 +- crates/core/src/scroll_area.rs | 321 ++---------------- crates/core/src/scrollbar.rs | 135 +++++++- crates/core/src/separator.rs | 8 +- crates/core/src/slider.rs | 6 - crates/core/src/tooltip.rs | 8 +- crates/data/src/sidebar.rs | 26 +- crates/data/src/tabs.rs | 3 +- crates/data/src/tree.rs | 48 ++- crates/editor/src/code_view.rs | 25 +- crates/editor/src/diff_view.rs | 4 +- crates/editor/src/markdown.rs | 54 +-- crates/editor/tests/markdown.rs | 8 - crates/layout/Cargo.toml | 1 - crates/layout/src/dock.rs | 36 +- crates/layout/src/resize_handle.rs | 12 +- crates/layout/src/shell.rs | 60 ++-- crates/layout/src/window_controls.rs | 19 +- crates/layout/src/window_frame.rs | 16 +- crates/layout/tests/dock.rs | 16 +- crates/layout/tests/window_controls.rs | 20 +- crates/theme/src/lib.rs | 34 +- docs/component-audit.md | 53 ++- examples/gallery/src/main.rs | 444 ++++++++++--------------- 32 files changed, 762 insertions(+), 972 deletions(-) diff --git a/crates/core/src/alert.rs b/crates/core/src/alert.rs index f74eb28..8af6b5a 100644 --- a/crates/core/src/alert.rs +++ b/crates/core/src/alert.rs @@ -260,11 +260,19 @@ impl Alerts { } } - /// Dismisses every alert. + /// Dismisses every alert, running each entry's `on_dismiss` handler. pub fn clear(&mut self, cx: &mut Context) { - if !self.entries.is_empty() { - self.entries.clear(); - cx.notify(); + if self.entries.is_empty() { + return; + } + + let entries = std::mem::take(&mut self.entries); + cx.notify(); + + for entry in entries { + if let Some(on_dismiss) = entry.on_dismiss { + on_dismiss(cx); + } } } diff --git a/crates/core/src/avatar.rs b/crates/core/src/avatar.rs index af87432..a5cfb9a 100644 --- a/crates/core/src/avatar.rs +++ b/crates/core/src/avatar.rs @@ -4,7 +4,10 @@ use gpui::{ App, ElementId, InteractiveElement as _, IntoElement, ParentElement as _, RenderOnce, Role, SharedString, StatefulInteractiveElement as _, StyleRefinement, Styled as _, Window, div, }; -use gpui_native_theme::{ActiveTheme as _, Oklch, Size, ThemeVariant}; +use gpui_native_theme::{ + ActiveTheme as _, Oklch, PIXEL_AVATAR_DARK_FACTOR, PIXEL_AVATAR_DARK_FLOOR, + PIXEL_AVATAR_LIGHT_CAP, PIXEL_AVATAR_LIGHT_LIFT, PIXEL_AVATAR_TINT_ALPHA, Size, ThemeVariant, +}; use crate::common::{Sizable, StyledExt, h_flex}; @@ -108,12 +111,10 @@ impl RenderOnce for Avatar { .refine_style(&self.style); match self.picture { - Some(url) => base.child(gpui::img(url).size_full().rounded(radius)), + Some(url) => base.child(gpui::img(url).size_full()), None => base.child(PixelAvatar { seed: fnv1a(self.id.as_bytes()), side, - radius, - style: self.style, }), } .into_any_element() @@ -125,14 +126,6 @@ impl RenderOnce for Avatar { struct PixelAvatar { seed: u64, side: gpui::Pixels, - radius: gpui::Pixels, - style: StyleRefinement, -} - -impl gpui::Styled for PixelAvatar { - fn style(&mut self) -> &mut StyleRefinement { - &mut self.style - } } impl RenderOnce for PixelAvatar { @@ -147,13 +140,13 @@ impl RenderOnce for PixelAvatar { let shade = if theme.variant == ThemeVariant::Dark { Oklch { - l: (accent.l * 1.6).min(0.95), + l: (accent.l * PIXEL_AVATAR_LIGHT_LIFT).min(PIXEL_AVATAR_LIGHT_CAP), ..Oklch::from_hsla(main) } .to_hsla() } else { Oklch { - l: (accent.l * 0.45).max(0.18), + l: (accent.l * PIXEL_AVATAR_DARK_FACTOR).max(PIXEL_AVATAR_DARK_FLOOR), ..Oklch::from_hsla(main) } .to_hsla() @@ -182,11 +175,11 @@ impl RenderOnce for PixelAvatar { .grid_rows(GRID_SIZE as u16) .size(self.side) .flex_none() - .rounded(self.radius) - .overflow_hidden() - .bg(gpui::Hsla { a: 0.16, ..main }) + .bg(gpui::Hsla { + a: PIXEL_AVATAR_TINT_ALPHA, + ..main + }) .children(cells) - .refine_style(&self.style) } } diff --git a/crates/core/src/button.rs b/crates/core/src/button.rs index 524ceef..315e2a2 100644 --- a/crates/core/src/button.rs +++ b/crates/core/src/button.rs @@ -4,7 +4,7 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ AnyView, App, ClickEvent, ElementId, InteractiveElement as _, IntoElement, ParentElement as _, RenderOnce, Role, SharedString, StatefulInteractiveElement as _, Styled as _, Window, canvas, - hsla, relative, + relative, transparent_black, }; use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; @@ -199,9 +199,9 @@ impl RenderOnce for Button { match background { Some(fill) => (fill, hover, pressed), None => ( - hsla(0.0, 0.0, 0.0, 0.0), - hsla(0.0, 0.0, 0.0, 0.0), - hsla(0.0, 0.0, 0.0, 0.0), + transparent_black(), + transparent_black(), + transparent_black(), ), } }; diff --git a/crates/core/src/combobox.rs b/crates/core/src/combobox.rs index 3367308..8329c10 100644 --- a/crates/core/src/combobox.rs +++ b/crates/core/src/combobox.rs @@ -10,13 +10,13 @@ use std::rc::Rc; use gpui::prelude::FluentBuilder as _; use gpui::{ - App, Bounds, Context, ElementId, Entity, EventEmitter, InteractiveElement as _, IntoElement, - KeyBinding, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Render, Role, - ScrollHandle, SharedString, StatefulInteractiveElement as _, StyleRefinement, Styled, - Subscription, Task, Window, actions, anchored, canvas, deferred, div, point, px, + App, Bounds, Context, ElementId, Entity, InteractiveElement as _, IntoElement, KeyBinding, + MouseButton, MouseDownEvent, ParentElement as _, Pixels, Render, Role, ScrollHandle, + SharedString, StatefulInteractiveElement as _, StyleRefinement, Styled, Subscription, Task, + Window, actions, anchored, canvas, deferred, div, point, px, }; use gpui_native_icons::{IconRole, icon}; -use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, POPUP_MAX_HEIGHT, Size}; use crate::common::{Sizable, StyledExt as _, h_flex, v_flex}; use crate::input::{Input, InputState, InputStateEvent}; @@ -177,9 +177,6 @@ where } } -/// Emitted when the combobox's value changes. -pub struct ComboboxChanged; - /// The stateful combobox. pub struct ComboboxState where @@ -209,8 +206,6 @@ where _subscription: Subscription, } -impl EventEmitter for ComboboxState where T: Clone + PartialEq + 'static {} - impl Sizable for ComboboxState where T: Clone + PartialEq + 'static, @@ -316,8 +311,8 @@ where .map(|(_, label)| label.clone()) } - /// Recomputes `visible` for `query` and hands the survivors' labels back. - fn refilter(&mut self, query: &str) -> Vec { + /// Recomputes `visible` for `query` and clamps the highlight onto it. + fn refilter(&mut self, query: &str) { let filter = self.filter.clone(); self.visible = self .options @@ -326,10 +321,7 @@ where .filter(|(_, (value, label))| filter(query, value, label)) .map(|(index, _)| index) .collect(); - self.visible - .iter() - .map(|index| self.options[*index].1.clone()) - .collect() + self.highlight = self.highlight.filter(|row| *row < self.visible.len()); } fn query_text(&self, cx: &Context) -> String { @@ -349,7 +341,6 @@ where }); self.tasks.push(task); } - cx.emit(ComboboxChanged); cx.notify(); } @@ -375,7 +366,7 @@ where fn text_changed(&mut self, window: &mut Window, cx: &mut Context) { let query = self.query_text(cx); self.refilter(&query); - self.highlight = (!self.visible.is_empty()).then_some(0usize); + self.highlight = Some(0); self.set_value(None, window, cx); cx.notify(); } @@ -386,7 +377,7 @@ where return; } self.refilter(""); - self.highlight = (!self.visible.is_empty()).then_some(0usize); + self.highlight = Some(0); cx.notify(); } @@ -427,10 +418,7 @@ where if self.visible.is_empty() { return; } - let current = self - .highlight - .and_then(|row| self.visible.get(row).copied()) - .and_then(|option_ix| self.visible.iter().position(|&row| row == option_ix)); + let current = self.highlight.filter(|row| *row < self.visible.len()); let next = match current { Some(row) => { let row = (row as i32 + delta).rem_euclid(self.visible.len() as i32); @@ -516,7 +504,7 @@ where let query = state.query.read(cx).value().to_string(); state.refilter(&query); } - state.highlight = (!state.visible.is_empty()).then_some(0usize); + state.highlight = Some(0); } cx.notify(); }); @@ -667,7 +655,7 @@ where ScrollArea::new((id.clone(), "rows")) .handle(scroll.read(cx).clone()) .axes(crate::scroll_area::ScrollAxes::Vertical) - .max_h(px(320.)) + .max_h(POPUP_MAX_HEIGHT) .children(rows) .into_any_element() }; diff --git a/crates/core/src/command_palette.rs b/crates/core/src/command_palette.rs index be2a201..213845a 100644 --- a/crates/core/src/command_palette.rs +++ b/crates/core/src/command_palette.rs @@ -7,10 +7,10 @@ use gpui::{ AnyElement, App, Context, ElementId, Entity, InteractiveElement as _, IntoElement, KeyBinding, MouseButton, ParentElement as _, Render, Role, ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Window, actions, canvas, deferred, - div, px, relative, + div, relative, }; use gpui_native_icons::{IconRole, icon}; -use gpui_native_theme::{ActiveTheme as _, Size}; +use gpui_native_theme::{ActiveTheme as _, POPUP_MAX_HEIGHT, Size}; use crate::common::{h_flex, v_flex}; use crate::input::{Input, InputState, InputStateEvent}; @@ -333,7 +333,7 @@ impl Render for CommandPaletteState { let row_ix = row; h_flex() - .id(("command", row)) + .id((ElementId::from(command.id.clone()), "command")) .debug_selector({ let id = id.clone(); move || format!("{id}-command-{row}") @@ -401,7 +401,7 @@ impl Render for CommandPaletteState { ScrollArea::new((id.clone(), "rows")) .handle(scroll.read(cx).clone()) .axes(ScrollAxes::Vertical) - .max_h(px(320.)) + .max_h(POPUP_MAX_HEIGHT) .children(rows) .into_any_element() }; diff --git a/crates/core/src/dialog.rs b/crates/core/src/dialog.rs index 3856ebc..6aba2f4 100644 --- a/crates/core/src/dialog.rs +++ b/crates/core/src/dialog.rs @@ -1,12 +1,10 @@ -use std::cell::RefCell; +use gpui::prelude::FluentBuilder as _; use std::rc::Rc; -use gpui::prelude::FluentBuilder as _; use gpui::{ - Animation, AnimationExt as _, AnyElement, App, Bounds, ElementId, FontWeight, - InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, - ParentElement as _, Pixels, RenderOnce, Role, SharedString, StatefulInteractiveElement as _, - Styled as _, Window, canvas, div, ease_out_quint, relative, + Animation, AnimationExt as _, AnyElement, App, ElementId, FontWeight, InteractiveElement as _, + IntoElement, KeyDownEvent, MouseButton, ParentElement as _, RenderOnce, Role, SharedString, + StatefulInteractiveElement as _, Styled as _, Window, canvas, div, ease_out_quint, relative, }; use gpui_native_theme::ActiveTheme as _; @@ -16,9 +14,6 @@ use crate::common::v_flex; pub type ChangeHandler = Rc; type ContentFn = Box AnyElement>; type ActionsFn = Box Vec>; -/// Bounds captured during the last paint, shared with the scrim listener. -type BoundsCell = Rc>>>; - /// A centred panel over a dim scrim: the platform's modal. #[derive(IntoElement)] pub struct Dialog { @@ -165,7 +160,6 @@ impl RenderOnce for Dialog { let content = self.content.map(|build| build(window, cx)); let actions = self.actions.map(|build| build(window, cx)); - let panel_bounds: BoundsCell = Rc::new(RefCell::new(None)); let panel = v_flex() .id("dialog-panel") @@ -211,19 +205,13 @@ impl RenderOnce for Dialog { panel.child(div().flex().justify_end().gap_2().children(actions)) }) .child( - canvas( - { - let cell = panel_bounds.clone(); - move |bounds, _window, _cx| *cell.borrow_mut() = Some(bounds) - }, - { - move |_, _, window, cx| { - if !focus.contains_focused(window, cx) { - window.focus(&focus, cx); - } + canvas(|_, _, _| {}, { + move |_, _, window, cx| { + if !focus.contains_focused(window, cx) { + window.focus(&focus, cx); } - }, - ) + } + }) .absolute() .inset_0(), ); @@ -235,15 +223,8 @@ impl RenderOnce for Dialog { .occlude() .when(!alert, |scrim| { scrim.on_mouse_down(MouseButton::Left, { - let panel_bounds = panel_bounds.clone(); let on_change = on_change.clone(); - move |event: &MouseDownEvent, _window, cx| { - let on_panel = panel_bounds - .borrow() - .is_some_and(|bounds| bounds.contains(&event.position)); - if on_panel { - return; - } + move |_, _window, cx| { if let Some(on_change) = &on_change { on_change(false, _window, cx); } diff --git a/crates/core/src/dropdown_menu.rs b/crates/core/src/dropdown_menu.rs index 8873e83..1f6a976 100644 --- a/crates/core/src/dropdown_menu.rs +++ b/crates/core/src/dropdown_menu.rs @@ -7,7 +7,7 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ App, ClickEvent, DispatchPhase, ElementId, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent, ParentElement as _, RenderOnce, Role, SharedString, - StatefulInteractiveElement as _, Styled as _, Window, canvas, div, hsla, relative, + StatefulInteractiveElement as _, Styled as _, Window, canvas, div, relative, }; use gpui_native_icons::{IconRole, icon}; use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, Size}; @@ -193,7 +193,7 @@ impl RenderOnce for DropdownMenu { let border = if show_border { colors.border } else { - background.unwrap_or_else(|| hsla(0.0, 0.0, 0.0, 0.0)) + background.unwrap_or_else(gpui::transparent_black) }; let split = self.on_click.is_some(); diff --git a/crates/core/src/input.rs b/crates/core/src/input.rs index 0577f27..d8b32d1 100644 --- a/crates/core/src/input.rs +++ b/crates/core/src/input.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::ops::Range; use std::rc::Rc; use std::time::{Duration, Instant}; @@ -16,7 +15,10 @@ use gpui::{ WrappedLine, actions, canvas, div, fill, point, px, }; use gpui_native_icons::IconRole; -use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, ColorTokens, Size}; +use gpui_native_theme::{ + ActiveTheme as _, BORDER_WIDTH, CARET_WIDTH, ColorTokens, Size, UNDERLINE_WIDTH, + WHEEL_LINE_HEIGHT, +}; use gpui_native_utils::focus_ring; use crate::button::{Button, ButtonVariant}; @@ -29,7 +31,7 @@ const DEFAULT_GROUP_INTERVAL: Duration = Duration::from_millis(300); /// How often the focused caret blinks. const CARET_BLINK_INTERVAL: Duration = Duration::from_millis(500); /// Pixel height of one scroll "line" for wheel scrolling. -const SCROLL_LINES_HEIGHT: f32 = 20.0; +const SCROLL_LINES_HEIGHT: f32 = WHEEL_LINE_HEIGHT; /// The row count a multiline field shows until the app sets one. const DEFAULT_ROWS: usize = 4; /// Maximum undo entries kept; the oldest drops off the front. @@ -270,7 +272,7 @@ impl InputBindings { } /// Collects the table into keybindings. - pub fn into_bindings(self) -> Vec { + pub(crate) fn into_bindings(self) -> Vec { let mut bindings: Vec = [ self.escape, self.submit, @@ -491,9 +493,9 @@ pub struct InputState { focused: bool, focus_handle: gpui::FocusHandle, /// The key-down handler, bound to this entity with `cx.listener`. - key_down_handler: Option, + key_down_handler: KeyDownHandler, /// The mouse-down handler, bound to this entity with `cx.listener`. - mouse_down_handler: Option, + mouse_down_handler: MouseDownHandler, /// Whether a drag-select is in progress. dragging: bool, /// Patch history. `redo_stack` holds undone entries awaiting redo. @@ -520,9 +522,9 @@ pub struct InputState { /// Bumped on every edit. caret_epoch: usize, /// When the current caret epoch started. - caret_epoch_started: Option, + caret_epoch_started: Instant, /// The shaped line and element bounds, recorded during each paint. - layout: Rc>>, + layout: Option, } impl EventEmitter for InputState {} @@ -536,16 +538,14 @@ impl InputState { marked_range: None, focused: false, focus_handle: cx.focus_handle(), - key_down_handler: Some(Rc::new(cx.listener( - |state, event: &KeyDownEvent, window, cx| { - state.handle_key_down(event, window, cx); - }, - ))), - mouse_down_handler: Some(Rc::new(cx.listener( + key_down_handler: Rc::new(cx.listener(|state, event: &KeyDownEvent, window, cx| { + state.handle_key_down(event, window, cx); + })), + mouse_down_handler: Rc::new(cx.listener( |state, event: &MouseDownEvent, window, cx| { state.handle_mouse_down(event, window, cx); }, - ))), + )), dragging: false, undo_stack: Vec::new(), redo_stack: Vec::new(), @@ -559,8 +559,8 @@ impl InputState { scroll_offset: point(Pixels::ZERO, Pixels::ZERO), scroll_to_caret_pending: true, caret_epoch: 0, - caret_epoch_started: Some(Instant::now()), - layout: Rc::new(RefCell::new(None)), + caret_epoch_started: Instant::now(), + layout: None, } } @@ -696,12 +696,7 @@ impl InputState { } } - /// Which keystroke submits a multiline field. - pub fn submit_on(&self) -> SubmitOn { - self.submit_on - } - - /// Whether the painted line masks the buffer. + // Whether the painted line masks the buffer. fn masked(&self) -> bool { self.password && !self.revealed } @@ -711,32 +706,43 @@ impl InputState { self.redo_stack.clear(); let now = Instant::now(); - if let Some(last) = self.undo_stack.last_mut() - && now.duration_since(last.timestamp) < self.group_interval - { + if let Some(last) = self.undo_stack.last_mut() { let last_end = last.range.start + last.new_len; - // Typing that continues right after the previous edit. - if range.start == last_end { - last.new_len += new_len; - last.timestamp = now; - return; + if self.marked_range.is_some() { + // A pre-edit step overlapping the previous entry's region. + if range.start >= last.range.start { + last.new_len += new_len; + let replaced = range.len(); + last.new_len -= replaced; + last.timestamp = now; + return; + } } - // A deletion of text that sat right before the previous edit. - if range.end == last.range.start && new_len == 0 { - last.range.start = range.start; - last.old_text = old_text + &last.old_text; - last.timestamp = now; - return; - } + if now.duration_since(last.timestamp) < self.group_interval { + // Typing that continues right after the previous edit. + if range.start == last_end { + last.new_len += new_len; + last.timestamp = now; + return; + } - // An edit fully inside the previous edit's replacement. - if range.start >= last.range.start && range.end <= last_end { - last.new_len += new_len; - last.new_len -= range.len(); - last.timestamp = now; - return; + // A deletion of text that sat right before the previous edit. + if range.end == last.range.start && new_len == 0 { + last.range.start = range.start; + last.old_text = old_text + &last.old_text; + last.timestamp = now; + return; + } + + // An edit fully inside the previous edit's replacement. + if range.start >= last.range.start && range.end <= last_end { + last.new_len += new_len; + last.new_len -= range.len(); + last.timestamp = now; + return; + } } } @@ -769,7 +775,7 @@ impl InputState { self.scroll_to_caret_pending = true; self.goal_x = None; self.caret_epoch += 1; - self.caret_epoch_started = Some(Instant::now()); + self.caret_epoch_started = Instant::now(); window.notify_text_input_changed(&self.focus_handle); } @@ -781,20 +787,38 @@ impl InputState { cx: &mut Context, ) -> bool { if self.multiline { - self.scroll_offset.y = (self.scroll_offset.y + delta.y).max(Pixels::ZERO); + let next_y = (self.scroll_offset.y + delta.y).max(Pixels::ZERO); + if next_y == self.scroll_offset.y { + return self.multiline_scrollable(); + } + self.scroll_offset.y = next_y; } else { - self.scroll_offset.x = (self.scroll_offset.x - delta.x).max(Pixels::ZERO); + let next_x = (self.scroll_offset.x - delta.x).max(Pixels::ZERO); + if next_x == self.scroll_offset.x { + return false; + } + self.scroll_offset.x = next_x; } + let scrollable = self .layout - .borrow() .as_ref() .is_some_and(|layout| layout.content_height > layout.visible_height); + cx.notify(); window.notify_text_input_changed(&self.focus_handle); self.multiline && scrollable } + /// Whether a multiline field's content overflows vertically. + fn multiline_scrollable(&self) -> bool { + self.multiline + && self + .layout + .as_ref() + .is_some_and(|layout| layout.content_height > layout.visible_height) + } + /// Emits `Focused`/`Blurred` when the field's focus state changed since the last paint. fn sync_focus(&mut self, focused: bool, cx: &mut Context) { if self.focused == focused { @@ -802,11 +826,9 @@ impl InputState { } self.focused = focused; - // A fresh focus starts a fresh blink epoch, so the caret shows solid - // for one interval before blinking resumes. if focused { self.caret_epoch += 1; - self.caret_epoch_started = Some(Instant::now()); + self.caret_epoch_started = Instant::now(); } cx.emit(if focused { @@ -817,11 +839,8 @@ impl InputState { } /// Whether the caret should paint. - pub fn caret_visible(&mut self, focused: bool) -> bool { - let elapsed = self - .caret_epoch_started - .map(|started| started.elapsed()) - .unwrap_or_default(); + pub fn caret_visible(&self, focused: bool) -> bool { + let elapsed = self.caret_epoch_started.elapsed(); self.caret_blink_visible_at(focused, elapsed) } @@ -901,15 +920,21 @@ impl InputState { window: &mut Window, cx: &mut Context, ) { - self.cursor = to.clamp(0, self.text.len()); - if !extend { - self.anchor = self.cursor; + let cursor = to.clamp(0, self.text.len()); + let anchor = if extend { self.anchor } else { cursor }; + if cursor == self.cursor && anchor == self.anchor { + return; } + self.cursor = cursor; + self.anchor = anchor; self.notify(window, cx); } /// Selects `range`. fn select_range(&mut self, range: Range, window: &mut Window, cx: &mut Context) { + if range.start == self.anchor && range.end == self.cursor { + return; + } self.anchor = range.start; self.cursor = range.end; self.notify(window, cx); @@ -1085,7 +1110,7 @@ impl InputState { ); let text_width = (bounds.size.width - padding.left - padding.right).max(Pixels::ZERO); - *self.layout.borrow_mut() = Some(InputLayout { + self.layout = Some(InputLayout { lines: text.lines, rows, bounds, @@ -1107,8 +1132,7 @@ impl InputState { return; } - let layout = self.layout.borrow(); - let Some(layout) = layout.as_ref() else { + let Some(layout) = self.layout.as_ref() else { return; }; @@ -1138,8 +1162,7 @@ impl InputState { window: &mut Window, cx: &mut App, ) { - let layout = self.layout.borrow(); - let Some(layout) = layout.as_ref() else { + let Some(layout) = self.layout.as_ref() else { return; }; @@ -1155,17 +1178,16 @@ impl InputState { let mut line_top = Pixels::ZERO; for entry in &layout.lines { let line_origin = point(origin.x, origin.y + line_top); - entry - .line - .paint( - line_origin, - layout.line_height, - TextAlign::Left, - None, - window, - cx, - ) - .ok(); + if let Err(err) = entry.line.paint( + line_origin, + layout.line_height, + TextAlign::Left, + None, + window, + cx, + ) { + eprintln!("painting input text failed: {err}"); + } line_top += layout.line_height * (entry.line.wrap_boundaries().len() + 1) as f32; } }); @@ -1179,8 +1201,7 @@ impl InputState { color: gpui::Hsla, window: &mut Window, ) { - let layout = self.layout.borrow(); - let Some(layout) = layout.as_ref() else { + let Some(layout) = self.layout.as_ref() else { return; }; @@ -1192,7 +1213,7 @@ impl InputState { let caret_top = origin.y + layout.rows[row_ix].y + (layout.line_height - font_size) / 2.0; let caret = Bounds::from_corners( point(origin.x + caret_x, caret_top), - point(origin.x + caret_x + px(1.5), caret_top + font_size), + point(origin.x + caret_x + CARET_WIDTH, caret_top + font_size), ); window.paint_quad(fill(caret, color)); @@ -1242,8 +1263,7 @@ impl InputState { /// The text index at a window point, using the last painted layout. fn index_at(&self, position: gpui::Point) -> usize { - let layout = self.layout.borrow(); - let Some(layout) = layout.as_ref() else { + let Some(layout) = self.layout.as_ref() else { return 0; }; real_index(&layout.mask_map, layout.index_at(position)) @@ -1256,7 +1276,11 @@ impl InputState { window: &mut Window, cx: &mut Context, ) { - self.cursor = self.index_at(position); + let cursor = self.index_at(position); + if cursor == self.cursor { + return; + } + self.cursor = cursor; self.notify(window, cx); } @@ -1268,14 +1292,13 @@ impl InputState { color: gpui::Hsla, window: &mut Window, ) { - let layout = self.layout.borrow(); - let Some(layout) = layout.as_ref() else { + let Some(layout) = self.layout.as_ref() else { return; }; let start = display_index(&layout.mask_map, range.start); let end = display_index(&layout.mask_map, range.end); - let thickness = px(1.0); + let thickness = UNDERLINE_WIDTH; for (row_ix, clipped, beyond) in layout.row_overlaps(start..end) { let top = origin.y + layout.rows[row_ix].y + layout.line_height - thickness; @@ -1329,7 +1352,7 @@ impl InputState { if event.click_count >= 3 { // Triple-click selects the clicked row, which is all of it when single-line. - let row_range = self.layout.borrow().as_ref().and_then(|layout| { + let row_range = self.layout.as_ref().and_then(|layout| { let display = display_index(&layout.mask_map, index); layout.row_range_at(display).map(|range| { real_index(&layout.mask_map, range.start) @@ -1462,15 +1485,16 @@ impl InputState { } let target = { - let layout = self.layout.borrow(); - let Some(layout) = layout.as_ref() else { + let Some(layout) = self.layout.as_ref() else { return; }; let display = display_index(&layout.mask_map, self.cursor); + let Some((row_ix, caret_x)) = layout.row_for_index(display) else { return; }; let target = row_ix as isize + delta; + let (display, goal_x) = if target < 0 { (0, None) } else if target as usize >= layout.rows.len() { @@ -1479,6 +1503,7 @@ impl InputState { let goal_x = self.goal_x.unwrap_or(caret_x); (layout.index_at_row_x(target as usize, goal_x), Some(goal_x)) }; + Some((real_index(&layout.mask_map, display), goal_x)) }; @@ -1748,8 +1773,7 @@ impl EntityInputHandler for InputState { _window: &mut Window, _cx: &mut Context, ) -> Option> { - let layout = self.layout.borrow(); - let layout = layout.as_ref()?; + let layout = self.layout.as_ref()?; let start = display_index( &layout.mask_map, @@ -1797,8 +1821,7 @@ impl EntityInputHandler for InputState { _window: &mut Window, _cx: &mut Context, ) -> Option { - let layout = self.layout.borrow(); - let layout = layout.as_ref()?; + let layout = self.layout.as_ref()?; let index = real_index(&layout.mask_map, layout.index_at(window_point)).min(self.text.len()); Some(utf8_range_to_utf16(&self.text, index..index).start) @@ -2178,23 +2201,13 @@ impl RenderOnce for Input { .when(!self.disabled, |input| { input .on_mouse_down(MouseButton::Left, { - let mouse_down_handler = self - .state - .read(cx) - .mouse_down_handler - .clone() - .expect("input state always carries its mouse-down handler"); + let mouse_down_handler = self.state.read(cx).mouse_down_handler.clone(); move |event: &MouseDownEvent, window, cx| { mouse_down_handler(event, window, cx); } }) .on_key_down({ - let key_down_handler = self - .state - .read(cx) - .key_down_handler - .clone() - .expect("input state always carries its key handler"); + let key_down_handler = self.state.read(cx).key_down_handler.clone(); move |event: &KeyDownEvent, window, cx| { key_down_handler(event, window, cx); } diff --git a/crates/core/src/menu.rs b/crates/core/src/menu.rs index 2bffa8c..d9751b9 100644 --- a/crates/core/src/menu.rs +++ b/crates/core/src/menu.rs @@ -10,7 +10,7 @@ use gpui::{ ease_out_quint, point, px, relative, }; use gpui_native_icons::{IconRole, icon as icon_glyph}; -use gpui_native_theme::ActiveTheme as _; +use gpui_native_theme::{ActiveTheme as _, POPUP_MAX_HEIGHT}; use crate::common::v_flex; use crate::scroll_area::{ScrollArea, ScrollAxes}; @@ -35,7 +35,7 @@ pub struct MenuItem { handler: Option, disabled: bool, destructive: bool, - checked: Option, + checked: bool, } #[derive(Clone)] @@ -58,7 +58,7 @@ impl MenuItem { handler: None, disabled: false, destructive: false, - checked: None, + checked: false, } } @@ -72,7 +72,7 @@ impl MenuItem { handler: None, disabled: false, destructive: false, - checked: None, + checked: false, } } @@ -114,7 +114,7 @@ impl MenuItem { /// Shows a check slot before the label. pub fn checked(mut self, checked: bool) -> Self { - self.checked = Some(checked); + self.checked = checked; self } @@ -470,7 +470,7 @@ pub(crate) fn anchored_menu_panel( ScrollArea::new("menu-items") .handle(scroll.read(cx).clone()) .axes(ScrollAxes::Vertical) - .max_h(px(320.)) + .max_h(POPUP_MAX_HEIGHT) .child(v_flex().gap_0p5().children(rows)), ) .child( @@ -596,7 +596,7 @@ fn render_item( .child(shortcut), ) }) - .when(checked == Some(true), |row| { + .when(checked, |row| { row.child( icon_glyph(IconRole::Check) .size(chrome.dense_icon_size) diff --git a/crates/core/src/scroll_area.rs b/crates/core/src/scroll_area.rs index 0b5ccb4..bc540c7 100644 --- a/crates/core/src/scroll_area.rs +++ b/crates/core/src/scroll_area.rs @@ -1,17 +1,13 @@ -use std::sync::Arc; -use std::time::Duration; -use std::time::Instant; +//! A scrollable container with an overlay scrollbar. use gpui::prelude::FluentBuilder as _; use gpui::{ - App, Bounds, DispatchPhase, ElementId, Entity, EntityId, InteractiveElement as _, IntoElement, - MouseDownEvent, MouseMoveEvent, MouseUpEvent, ParentElement, Pixels, RenderOnce, ScrollEvent, - ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled, Task, Window, canvas, div, - fill, point, + App, ElementId, InteractiveElement as _, IntoElement, ParentElement, RenderOnce, ScrollHandle, + SharedString, StatefulInteractiveElement as _, Styled, Window, div, }; -use gpui_native_theme::ActiveTheme as _; use crate::common::StyledExt; +use crate::scrollbar::{Scrollbar, ScrollbarVisibility}; /// Which directions the content scrolls in. #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] @@ -25,108 +21,6 @@ pub enum ScrollAxes { Both, } -/// When an overlay scrollbar shows. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] -pub enum ScrollbarVisibility { - /// The running platform's behavior, from the theme. - #[default] - Platform, - Always, - Autohide, -} - -/// How long an autohide thumb stays fully visible before its fade begins. -const AUTOHIDE_DELAY: Duration = Duration::from_millis(700); -/// How long the autohide fade runs. -const AUTOHIDE_FADE: Duration = Duration::from_millis(250); -/// How often the fade repaints while it runs. -const AUTOHIDE_FADE_STEP: Duration = Duration::from_millis(50); - -/// The shared autohide timeline for an overlay scrollbar. -#[derive(Clone)] -pub(crate) struct Autohide { - view: EntityId, - state: Entity, -} - -struct AutohideState { - last_activity: Option, - hide_task: Option>, -} - -impl Autohide { - pub(crate) fn new(key: ElementId, window: &mut Window, cx: &mut App) -> Self { - let view = window.current_view(); - let state = window.use_keyed_state(key, cx, |_, _| AutohideState { - last_activity: None, - hide_task: None, - }); - Self { view, state } - } - - /// Marks interaction. - pub(crate) fn touch(&self, window: &mut Window, cx: &mut App) { - let view = self.view; - let reduce_motion = cx.theme().accessibility.reduce_motion; - - let task = window.spawn(cx, async move |cx| { - if reduce_motion { - cx.background_executor() - .timer(AUTOHIDE_DELAY + AUTOHIDE_FADE) - .await; - cx.update(|_, cx| cx.notify(view)).ok(); - return; - } - - let steps = (AUTOHIDE_FADE.as_millis() / AUTOHIDE_FADE_STEP.as_millis()).max(1); - cx.background_executor().timer(AUTOHIDE_DELAY).await; - - for _ in 0..steps { - cx.update(|_, cx| cx.notify(view)).ok(); - cx.background_executor().timer(AUTOHIDE_FADE_STEP).await; - } - - cx.update(|_, cx| cx.notify(view)).ok(); - }); - - self.state.update(cx, |state, _| { - state.last_activity = Some(Instant::now()); - state.hide_task = Some(task); - }); - } - - /// The thumb's opacity. - pub(crate) fn alpha(&self, cx: &App, dragging: bool) -> f32 { - if dragging { - return 1.0; - } - - let Some(activity) = self.state.read(cx).last_activity else { - return 0.0; - }; - - let idle = activity.elapsed(); - if idle <= AUTOHIDE_DELAY { - return 1.0; - } - - let fade = idle.saturating_sub(AUTOHIDE_DELAY); - if fade >= AUTOHIDE_FADE { - return 0.0; - } - - 1.0 - fade.as_secs_f32() / AUTOHIDE_FADE.as_secs_f32() - } -} - -/// An overlay scrollbar thumb's style for one paint: `None` hides it. -pub(crate) fn thumb_style(color: gpui::Hsla, alpha: f32) -> Option { - (alpha > 0.0).then_some(gpui::Hsla { - a: color.a * alpha, - ..color - }) -} - /// A scrollable container with an overlay scrollbar. #[derive(IntoElement)] pub struct ScrollArea { @@ -196,32 +90,8 @@ impl ScrollArea { } } -/// Names keyed state under the scrollbar's own id. -pub fn scrollbar_state_key(name: &'static str, id: &ElementId) -> ElementId { - ElementId::NamedChild(Arc::new(id.clone()), SharedString::from(name)) -} - -/// Pure thumb geometry for an overlay scrollbar. -pub(crate) fn thumb_geometry( - viewport: Pixels, - content: Pixels, - offset: Pixels, - track: Pixels, - min_thumb: Pixels, -) -> (Pixels, Pixels) { - if content <= viewport || content <= Pixels::ZERO { - return (track, Pixels::ZERO); - } - - let length = (viewport / content * track).max(min_thumb).min(track); - let overflow = content - viewport; - let progress = (-offset / overflow).clamp(0., 1.); - let travel = track - length; - (length, travel * progress) -} - impl RenderOnce for ScrollArea { - fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + fn render(self, _window: &mut Window, _cx: &mut App) -> impl IntoElement { let mut pane_style = self.style.clone(); pane_style.padding = Default::default(); @@ -252,175 +122,24 @@ impl RenderOnce for ScrollArea { if let Some(handle) = self.handle { let id = self.id.clone(); - let autohide = match self.visibility { - ScrollbarVisibility::Platform => cx.theme().scrollbar.autohide, - ScrollbarVisibility::Always => false, - ScrollbarVisibility::Autohide => true, - }; - wrapper.extend(std::iter::once( - canvas(|bounds, _, _| bounds, { - let handle = handle.clone(); - move |overlay, _, window, cx| { - paint_scrollbar(&handle, &id, autohide, overlay, window, cx); - } - }) - .absolute() - .inset_0() - .into_any_element(), - )); + if self.axes != ScrollAxes::Horizontal { + wrapper.extend(std::iter::once( + Scrollbar::vertical((id.clone(), "v-scrollbar"), handle.clone()) + .visibility(self.visibility) + .into_any_element(), + )); + } + + if self.axes != ScrollAxes::Vertical { + wrapper.extend(std::iter::once( + Scrollbar::horizontal((id, "h-scrollbar"), handle) + .visibility(self.visibility) + .into_any_element(), + )); + } } wrapper } } - -/// Paints the overlay scrollbar thumb. -fn paint_scrollbar( - handle: &ScrollHandle, - key: &ElementId, - autohide: bool, - overlay: Bounds, - window: &mut Window, - cx: &mut App, -) { - let theme = cx.theme(); - let view = window.current_view(); - - let color = theme.colors.scrollbar_thumb; - let width = theme.scrollbar.width; - let inset = theme.scrollbar.inset; - let min_thumb = theme.scrollbar.min_thumb; - - let viewport = handle.bounds(); - let max_offset = handle.max_offset().y; - - if max_offset <= Pixels::ZERO || viewport.size.height <= Pixels::ZERO { - return; - } - - let autohide_state = - autohide.then(|| Autohide::new(scrollbar_state_key("autohide", key), window, cx)); - let drag = window.use_keyed_state(scrollbar_state_key("thumb-drag", key), cx, |_, _| { - Option::<(Pixels, Pixels)>::None - }); - let dragging = drag.read(cx).is_some(); - - let alpha = match (&autohide_state, autohide) { - (Some(state), true) => state.alpha(cx, dragging), - _ => 1.0, - }; - - let track = overlay.size.height - inset * 2.0; - let (thumb_length, thumb_top) = thumb_geometry( - viewport.size.height, - viewport.size.height + max_offset, - handle.offset().y, - track, - min_thumb, - ); - - let thumb_x = overlay.right() - width - inset; - let thumb_bounds = Bounds::from_corners( - point(thumb_x, overlay.top() + inset + thumb_top), - point( - thumb_x + width, - overlay.top() + inset + thumb_top + thumb_length, - ), - ); - - if let Some(color) = thumb_style(color, alpha) { - window.paint_quad(fill(thumb_bounds, color).corner_radii(width / 2.0)); - } - - if let Some(state) = &autohide_state { - window.on_mouse_event({ - let state = state.clone(); - move |event: &ScrollEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble || !overlay.contains(&event.position) { - return; - } - state.touch(window, cx); - } - }); - } - - window.on_mouse_event({ - let handle = handle.clone(); - let drag = drag.clone(); - let autohide_state = autohide_state.clone(); - move |event: &MouseDownEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble - || event.button != gpui::MouseButton::Left - || !thumb_bounds.contains(&event.position) - { - return; - } - - drag.update(cx, |drag, _| { - *drag = Some((event.position.y, handle.offset().y)); - }); - - if let Some(state) = &autohide_state { - state.touch(window, cx); - } - - cx.stop_propagation(); - } - }); - - window.on_mouse_event({ - let drag = drag.clone(); - let autohide_state = autohide_state.clone(); - move |event: &MouseUpEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble || event.button != gpui::MouseButton::Left { - return; - } - - if drag.read(cx).is_none() { - return; - } - - drag.update(cx, |drag, _| { - *drag = None; - }); - - if let Some(state) = &autohide_state { - state.touch(window, cx); - } - } - }); - - window.on_mouse_event({ - let handle = handle.clone(); - let drag = drag.clone(); - let autohide_state = autohide_state.clone(); - move |event: &MouseMoveEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble { - return; - } - - if let Some(state) = &autohide_state - && drag.read(cx).is_some() - { - state.touch(window, cx); - } - - if event.pressed_button != Some(gpui::MouseButton::Left) { - return; - } - - let Some((start_y, start_offset)) = *drag.read(cx) else { - return; - }; - - if max_offset > Pixels::ZERO && track > thumb_length { - let scale = max_offset / (track - thumb_length); - let offset = (start_offset - (event.position.y - start_y) * scale) - .clamp(-max_offset, Pixels::ZERO); - handle.set_offset(point(handle.offset().x, offset)); - cx.notify(view); - } - } - }); -} diff --git a/crates/core/src/scrollbar.rs b/crates/core/src/scrollbar.rs index 6ccbd6a..15b34aa 100644 --- a/crates/core/src/scrollbar.rs +++ b/crates/core/src/scrollbar.rs @@ -1,18 +1,141 @@ //! Overlay scrollbars for scroll surfaces. use std::rc::Rc; +use std::sync::Arc; +use std::time::{Duration, Instant}; use gpui::{ - App, Bounds, DispatchPhase, ElementId, IntoElement, ListState, MouseButton, MouseDownEvent, - MouseMoveEvent, MouseUpEvent, Pixels, Point, RenderOnce, ScrollEvent, ScrollHandle, - Styled as _, UniformListScrollHandle, Window, canvas, fill, point, + App, Bounds, DispatchPhase, ElementId, Entity, EntityId, IntoElement, ListState, MouseButton, + MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, RenderOnce, ScrollEvent, + ScrollHandle, SharedString, Styled as _, Task, UniformListScrollHandle, Window, canvas, fill, + point, }; use gpui_native_theme::ActiveTheme as _; -use crate::scroll_area::{Autohide, scrollbar_state_key, thumb_geometry, thumb_style}; +/// When an overlay scrollbar shows. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum ScrollbarVisibility { + /// The running platform's behavior, from the theme. + #[default] + Platform, + Always, + Autohide, +} -/// Re-exported so callers only need the scrollbar module. -pub use crate::scroll_area::ScrollbarVisibility; +/// How long an autohide thumb stays fully visible before its fade begins. +const AUTOHIDE_DELAY: Duration = Duration::from_millis(700); +/// How long the autohide fade runs. +const AUTOHIDE_FADE: Duration = Duration::from_millis(250); +/// How often the fade repaints while it runs. +const AUTOHIDE_FADE_STEP: Duration = Duration::from_millis(50); + +/// The shared autohide timeline for an overlay scrollbar. +#[derive(Clone)] +struct Autohide { + view: EntityId, + state: Entity, +} + +struct AutohideState { + last_activity: Option, + hide_task: Option>, +} + +impl Autohide { + fn new(key: ElementId, window: &mut Window, cx: &mut App) -> Self { + let view = window.current_view(); + let state = window.use_keyed_state(key, cx, |_, _| AutohideState { + last_activity: None, + hide_task: None, + }); + Self { view, state } + } + + /// Marks interaction. + fn touch(&self, window: &mut Window, cx: &mut App) { + let view = self.view; + let reduce_motion = cx.theme().accessibility.reduce_motion; + + let task = window.spawn(cx, async move |cx| { + if reduce_motion { + cx.background_executor() + .timer(AUTOHIDE_DELAY + AUTOHIDE_FADE) + .await; + cx.update(|_, cx| cx.notify(view)).ok(); + return; + } + + let steps = (AUTOHIDE_FADE.as_millis() / AUTOHIDE_FADE_STEP.as_millis()).max(1); + cx.background_executor().timer(AUTOHIDE_DELAY).await; + + for _ in 0..steps { + cx.update(|_, cx| cx.notify(view)).ok(); + cx.background_executor().timer(AUTOHIDE_FADE_STEP).await; + } + + cx.update(|_, cx| cx.notify(view)).ok(); + }); + + self.state.update(cx, |state, _| { + state.last_activity = Some(Instant::now()); + state.hide_task = Some(task); + }); + } + + /// The thumb's opacity. + fn alpha(&self, cx: &App, dragging: bool) -> f32 { + if dragging { + return 1.0; + } + + let Some(activity) = self.state.read(cx).last_activity else { + return 0.0; + }; + + let idle = activity.elapsed(); + if idle <= AUTOHIDE_DELAY { + return 1.0; + } + + let fade = idle.saturating_sub(AUTOHIDE_DELAY); + if fade >= AUTOHIDE_FADE { + return 0.0; + } + + 1.0 - fade.as_secs_f32() / AUTOHIDE_FADE.as_secs_f32() + } +} + +fn scrollbar_state_key(name: &'static str, id: &ElementId) -> ElementId { + ElementId::NamedChild(Arc::new(id.clone()), SharedString::from(name)) +} + +/// Pure thumb geometry for an overlay scrollbar. +fn thumb_geometry( + viewport: Pixels, + content: Pixels, + offset: Pixels, + track: Pixels, + min_thumb: Pixels, +) -> (Pixels, Pixels) { + if content <= viewport || content <= Pixels::ZERO { + return (track, Pixels::ZERO); + } + + let length = (viewport / content * track).max(min_thumb).min(track); + let overflow = content - viewport; + let progress = (-offset / overflow).clamp(0., 1.); + let travel = track - length; + (length, travel * progress) +} + +/// An overlay scrollbar thumb's style for one paint: `None` hides it. +fn thumb_style(color: gpui::Hsla, alpha: f32) -> Option { + (alpha > 0.0).then_some(gpui::Hsla { + a: color.a * alpha, + ..color + }) +} /// The scroll state a scrollbar drives, implemented for gpui's scroll handles. pub trait ScrollbarHandle: 'static { diff --git a/crates/core/src/separator.rs b/crates/core/src/separator.rs index 0b46652..c53315a 100644 --- a/crates/core/src/separator.rs +++ b/crates/core/src/separator.rs @@ -3,8 +3,8 @@ use std::rc::Rc; use gpui::prelude::FluentBuilder as _; -use gpui::{App, InteractiveElement as _, IntoElement, RenderOnce, Styled as _, Window, div, px}; -use gpui_native_theme::ActiveTheme as _; +use gpui::{App, InteractiveElement as _, IntoElement, RenderOnce, Styled as _, Window, div}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH}; /// Which axis the rule runs along. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -69,8 +69,8 @@ impl RenderOnce for Separator { rule.debug_selector(move || selector()) }) .map(|rule| match self.orientation { - SeparatorOrientation::Horizontal => rule.w_full().h(px(1.)), - SeparatorOrientation::Vertical => rule.self_stretch().w(px(1.)), + SeparatorOrientation::Horizontal => rule.w_full().h(BORDER_WIDTH), + SeparatorOrientation::Vertical => rule.self_stretch().w(BORDER_WIDTH), }) } } diff --git a/crates/core/src/slider.rs b/crates/core/src/slider.rs index e246226..b29f11e 100644 --- a/crates/core/src/slider.rs +++ b/crates/core/src/slider.rs @@ -79,12 +79,6 @@ impl Slider { self } - /// Sets the current value; the slider never owns it. - pub fn value(mut self, value: f32) -> Self { - self.value = value; - self - } - /// Renders the slider inert, unfocusable, and dimmed; disabled always wins. pub fn disabled(mut self, disabled: bool) -> Self { self.disabled = disabled; diff --git a/crates/core/src/tooltip.rs b/crates/core/src/tooltip.rs index 26e7d4f..f7af13c 100644 --- a/crates/core/src/tooltip.rs +++ b/crates/core/src/tooltip.rs @@ -1,9 +1,9 @@ use gpui::AppContext as _; use gpui::{ AnyView, App, Bounds, Context, ElementId, Entity, InteractiveElement as _, IntoElement, - ParentElement as _, Pixels, Render, SharedString, Styled as _, Window, canvas, div, point, px, + ParentElement as _, Pixels, Render, SharedString, Styled as _, Window, canvas, div, point, }; -use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH}; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, TOOLTIP_MAX_WIDTH, TOOLTIP_OFFSET}; use gpui_native_utils::{anchor_bounds, record_anchor_bounds}; const PANEL_BOUNDS_ID: &str = "tooltip-panel-bounds"; @@ -76,7 +76,7 @@ impl Render for Tooltip { .debug_selector(|| "tooltip-panel".to_string()) .px_2() .py_1() - .max_w(px(360.)) + .max_w(TOOLTIP_MAX_WIDTH) .bg(surface) .border_1() .border_color(border) @@ -89,7 +89,7 @@ impl Render for Tooltip { let (wrapper, panel) = match self.anchor { Some(parent) => { - let origin = window.mouse_position() + point(px(1.), px(1.)); + let origin = window.mouse_position() + point(TOOLTIP_OFFSET, TOOLTIP_OFFSET); let viewport_height = window.viewport_size().height; let height = anchor_bounds( diff --git a/crates/data/src/sidebar.rs b/crates/data/src/sidebar.rs index 8d80bef..4046b60 100644 --- a/crates/data/src/sidebar.rs +++ b/crates/data/src/sidebar.rs @@ -197,16 +197,31 @@ impl RenderOnce for Sidebar { let scroll_area_id = (self.id.clone(), "entries"); - // Entry paint reads the theme through this borrow. + let row_count = entries + .iter() + .filter(|entry| matches!(entry, SidebarEntry::Row { .. })) + .count(); + + let mut row_position = 0usize; let children: Vec = entries .iter() .enumerate() .map(|(index, entry)| { + let is_row = matches!(entry, SidebarEntry::Row { .. }); + if is_row { + row_position += 1; + } + let (position_in_set, size_of_set) = if is_row { + (row_position, row_count) + } else { + (0, 0) + }; render_entry( cx, entry, index, - entries.len(), + position_in_set, + size_of_set, self.selected == Some(index), material, &self.on_change, @@ -258,7 +273,8 @@ fn render_entry( cx: &App, entry: &SidebarEntry, index: usize, - count: usize, + position_in_set: usize, + size_of_set: usize, selected: bool, material: bool, on_change: &Option, @@ -294,8 +310,8 @@ fn render_entry( .rounded(theme.radius.sm) .role(Role::ListItem) .aria_selected(selected) - .aria_position_in_set(index + 1) - .aria_size_of_set(count) + .aria_position_in_set(position_in_set) + .aria_size_of_set(size_of_set) .map(|row| { if material && !*row_disabled { if selected { diff --git a/crates/data/src/tabs.rs b/crates/data/src/tabs.rs index fe13093..51333cb 100644 --- a/crates/data/src/tabs.rs +++ b/crates/data/src/tabs.rs @@ -214,7 +214,6 @@ impl RenderOnce for Tabs { cx, tab, index, - self.selected == index, self.selected, self.variant, &self.on_change, @@ -272,7 +271,6 @@ fn render_tab( cx: &App, tab: Tab, index: usize, - selected: bool, selected_index: usize, variant: TabsVariant, on_change: &Option, @@ -280,6 +278,7 @@ fn render_tab( focus: &gpui::FocusHandle, ) -> AnyElement { let mut tab = tab; + let selected = index == selected_index; let theme = cx.theme(); let metrics = theme.sizes.resolve(Size::default()); diff --git a/crates/data/src/tree.rs b/crates/data/src/tree.rs index 1444a05..d653f82 100644 --- a/crates/data/src/tree.rs +++ b/crates/data/src/tree.rs @@ -166,14 +166,49 @@ impl RenderOnce for Tree { .clone() .unwrap_or_else(|| SharedString::from(self.id.to_string())); + let sibling_sets: Vec<(usize, usize)> = { + let mut sets = vec![(0usize, 0usize); items.len()]; + let mut start = 0usize; + + while start < items.len() { + let depth = items[start].depth; + + let mut end = start + 1; + while end < items.len() && items[end].depth > depth { + end += 1; + } + + let mut position = 0usize; + for index in start..end { + if items[index].depth == depth { + position += 1; + sets[index] = (position, 0); + } + } + + for index in start..end { + if items[index].depth == depth { + sets[index].1 = position; + } + } + + start = end; + } + + sets + }; + let rows: Vec = items .iter() .enumerate() .map(|(index, _item)| { + let (position_in_set, size_of_set) = sibling_sets[index]; render_row( cx, - &items, + &items[index], index, + position_in_set, + size_of_set, self.selected == Some(index), &self.on_change, &self.on_disclosure, @@ -228,21 +263,21 @@ impl RenderOnce for Tree { } } +#[allow(clippy::too_many_arguments)] fn render_row( cx: &App, - items: &[TreeItem], + item: &TreeItem, index: usize, + position_in_set: usize, + size_of_set: usize, selected: bool, on_change: &Option, on_disclosure: &Option, focus: &FocusHandle, ) -> AnyElement { let theme = cx.theme(); - let item = &items[index]; - let count = items.len(); let label_color = selectable_label_color(cx, selected, item.disabled); let disabled = item.disabled; - // One indent step per depth level, on the theme's horizontal padding. let indent = theme.sizes.medium.padding_x * (item.depth as f32 + 1.0); @@ -258,7 +293,8 @@ fn render_row( .role(Role::TreeItem) .aria_selected(selected) .aria_level(item.depth + 1) - .aria_size_of_set(count) + .aria_position_in_set(position_in_set) + .aria_size_of_set(size_of_set) .when(item.expandable, |row| row.aria_expanded(item.expanded)) .map(|row| paint_selectable_row(cx, row, selected, disabled)) .when(!disabled, |row| { diff --git a/crates/editor/src/code_view.rs b/crates/editor/src/code_view.rs index 41d03aa..bd0509d 100644 --- a/crates/editor/src/code_view.rs +++ b/crates/editor/src/code_view.rs @@ -11,7 +11,7 @@ use gpui::{ actions, canvas, div, uniform_list, }; use gpui_native_core::scrollbar::{Scrollbar, ScrollbarHandle, ScrollbarVisibility}; -use gpui_native_theme::{ActiveTheme as _, ColorTokens, Ramps}; +use gpui_native_theme::{ActiveTheme as _, ColorTokens, MONO_ADVANCE_RATIO, Ramps}; use crate::highlight::{HighlightedCode, Language, Role, highlight}; use crate::utils::{gutter_cell, run_style}; @@ -132,11 +132,6 @@ impl CodeViewState { &self.source } - /// The language the source highlights as. - pub fn language(&self) -> Language { - self.language - } - /// The highlighted lines. pub fn code(&self) -> &HighlightedCode { &self.code @@ -209,13 +204,7 @@ impl CodeViewState { (!selected.is_empty()).then(|| selected.join("\n")) } - /// Drops the selection. - pub fn clear_selection(&mut self) { - self.selection = None; - self.selecting = false; - } - - /// Overrides the accessibility label of the view. + // Overrides the accessibility label of the view. pub fn accessibility_label(mut self, label: L) -> Self where L: Into, @@ -235,8 +224,7 @@ impl CodeViewState { text_x: Pixels, ) -> SelectionPoint { let row = (((position.y - bounds.origin.y - scroll_y) / line_height).floor() as isize) - .clamp(0, self.code.lines.len() as isize - 1) - .max(0) as usize; + .clamp(0, self.code.lines.len() as isize - 1) as usize; let chars = self.code.lines[row] .spans @@ -244,9 +232,8 @@ impl CodeViewState { .map(|span| span.text.chars().count()) .sum::(); - let col = (((position.x - text_x) / char_width).round() as isize) - .clamp(0, chars as isize) - .max(0) as usize; + let col = (((position.x - text_x) / char_width).round() as isize).clamp(0, chars as isize) + as usize; SelectionPoint { row, col } } @@ -265,7 +252,7 @@ impl Render for CodeViewState { let mono_font = theme.mono_font; let text_size = theme.sizes.small.text_size; let line_height = text_size * 1.5; - let char_width = text_size * 0.62; + let char_width = text_size * MONO_ADVANCE_RATIO; let root_font_size = theme.root_font_size; let id = self.id.clone(); diff --git a/crates/editor/src/diff_view.rs b/crates/editor/src/diff_view.rs index 1395e75..9c3e980 100644 --- a/crates/editor/src/diff_view.rs +++ b/crates/editor/src/diff_view.rs @@ -13,7 +13,7 @@ use gpui::{ point, uniform_list, }; use gpui_native_core::scrollbar::{Scrollbar, ScrollbarHandle, ScrollbarVisibility}; -use gpui_native_theme::ActiveTheme as _; +use gpui_native_theme::{ActiveTheme as _, MONO_ADVANCE_RATIO}; use similar::{ChangeTag, TextDiff}; use std::ops::Range; @@ -399,7 +399,7 @@ fn render_row( let ramps = theme.ramps; let text_size = theme.sizes.small.text_size; let line_height = text_size * 1.5; - let char_width = text_size * 0.62; + let char_width = text_size * MONO_ADVANCE_RATIO; let frame = div() .debug_selector(move || format!("diff-row-{row_ix}")) diff --git a/crates/editor/src/markdown.rs b/crates/editor/src/markdown.rs index 0e54087..fd8dcba 100644 --- a/crates/editor/src/markdown.rs +++ b/crates/editor/src/markdown.rs @@ -101,8 +101,6 @@ pub enum InlineStyle { #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct MarkdownTableCell { pub spans: Vec, - /// 1-based column position, so renderers can align to the table's columns. - pub column: usize, } /// A table row; the header row precedes the body rows. @@ -293,24 +291,6 @@ struct ListFrame { task: Option, } -/// The table being assembled, when the parser is inside one. -enum TableBuilder { - None, - Body { rows: Vec }, -} - -/// One open code block, accumulating its raw text. -#[derive(Default)] -struct CodeBuilder { - text: String, -} - -impl CodeBuilder { - fn push(&mut self, text: &str) { - self.text.push_str(text); - } -} - /// Accumulates inline spans across inline events. #[derive(Default)] struct InlineBuilder { @@ -403,8 +383,8 @@ fn parse(source: &str, highlight_code: bool) -> Vec { // Whether each open list item already flushed content rows. let mut items: Vec = Vec::new(); let mut quote_depth = 0usize; - let mut table = TableBuilder::None; - let mut code: Option = None; + let mut table: Option> = None; + let mut code: Option = None; // The fence info string arrives on Start, its text follows. let mut code_language: Option = None; @@ -469,7 +449,7 @@ fn parse(source: &str, highlight_code: bool) -> Vec { &mut blocks, quote_depth, ); - code = Some(CodeBuilder::default()); + code = Some(String::new()); if let CodeBlockKind::Fenced(info) = kind { let info = info.trim(); code_language = @@ -484,10 +464,10 @@ fn parse(source: &str, highlight_code: bool) -> Vec { &mut blocks, quote_depth, ); - table = TableBuilder::Body { rows: Vec::new() } + table = Some(Vec::new()) } Tag::TableHead => { - if let TableBuilder::Body { rows } = &mut table { + if let Some(rows) = &mut table { rows.push(MarkdownTableRow { cells: Vec::new(), header: true, @@ -495,7 +475,7 @@ fn parse(source: &str, highlight_code: bool) -> Vec { } } Tag::TableRow => { - if let TableBuilder::Body { rows } = &mut table { + if let Some(rows) = &mut table { rows.push(MarkdownTableRow { cells: Vec::new(), header: false, @@ -583,7 +563,7 @@ fn parse(source: &str, highlight_code: bool) -> Vec { } TagEnd::CodeBlock => { let built = code.take().unwrap_or_default(); - let text = built.text.trim_end_matches('\n'); + let text = built.trim_end_matches('\n'); let language = code_language.take(); let lines = match highlight_code .then(|| language.as_deref().and_then(grammar_language)) @@ -600,8 +580,7 @@ fn parse(source: &str, highlight_code: bool) -> Vec { }); } TagEnd::Table => { - let built = std::mem::replace(&mut table, TableBuilder::None); - if let TableBuilder::Body { rows } = built { + if let Some(rows) = table.take() { blocks.push(MarkdownBlock::Table { table: MarkdownTable { rows }, indent: lists.len(), @@ -611,16 +590,13 @@ fn parse(source: &str, highlight_code: bool) -> Vec { } TagEnd::TableCell => { let spans = inline.finish(); - let TableBuilder::Body { rows } = &mut table else { + let Some(rows) = &mut table else { continue; }; let Some(row) = rows.last_mut() else { continue; }; - row.cells.push(MarkdownTableCell { - spans, - column: row.cells.len() + 1, - }); + row.cells.push(MarkdownTableCell { spans }); } TagEnd::Emphasis | TagEnd::Strong @@ -630,8 +606,8 @@ fn parse(source: &str, highlight_code: bool) -> Vec { _ => {} }, Event::Text(text) => { - if let Some(built) = code.as_mut() { - built.push(&text); + if let Some(buffer) = code.as_mut() { + buffer.push_str(&text); } else { inline.push_text(&text); } @@ -660,9 +636,9 @@ fn parse(source: &str, highlight_code: bool) -> Vec { frame.task = Some(checked); } } - Event::FootnoteReference(label) => inline.push_text(&label), - // HTML and math render nothing in this view. - Event::InlineHtml(_) + // Footnotes are disabled in the parser options. + Event::FootnoteReference(_) + | Event::InlineHtml(_) | Event::Html(_) | Event::InlineMath(_) | Event::DisplayMath(_) => {} diff --git a/crates/editor/tests/markdown.rs b/crates/editor/tests/markdown.rs index 2b55e9d..ce4e412 100644 --- a/crates/editor/tests/markdown.rs +++ b/crates/editor/tests/markdown.rs @@ -305,14 +305,6 @@ fn tables_parse_header_first_with_column_positions() { .map(|cell| spans_text(&cell.spans)) .collect(); assert_eq!(header, vec!["a", "b"]); - assert_eq!( - table.rows[0] - .cells - .iter() - .map(|cell| cell.column) - .collect::>(), - vec![1, 2] - ); assert_eq!(spans_text(&table.rows[2].cells[1].spans), "4"); } diff --git a/crates/layout/Cargo.toml b/crates/layout/Cargo.toml index 490dcb9..0269a3a 100644 --- a/crates/layout/Cargo.toml +++ b/crates/layout/Cargo.toml @@ -20,7 +20,6 @@ default = ["full"] full = ["dock", "resize_handle", "resizable", "shell", "window_controls"] dock = [ "resize_handle", - "resizable", "dep:gpui-native-core", "dep:gpui-native-data", "dep:gpui-native-icons", diff --git a/crates/layout/src/dock.rs b/crates/layout/src/dock.rs index ab4fd5e..5cc8a4c 100644 --- a/crates/layout/src/dock.rs +++ b/crates/layout/src/dock.rs @@ -192,10 +192,8 @@ impl DockState { } /// Registers a panel descriptor, replacing any earlier one with its id. - pub fn register_panel(&mut self, panel: PanelDescriptor, cx: &mut Context) { - if self.panels.insert(panel.id.clone(), panel).is_none() { - cx.notify(); - } + pub fn register_panel(&mut self, panel: PanelDescriptor) { + self.panels.insert(panel.id.clone(), panel); } /// Opens a panel into the active pane and activates its tab. @@ -330,6 +328,17 @@ impl DockState { }; if changed { + // The kept tab may have no right view, show the first remaining one. + self.right_view = self + .locate(id) + .and_then(|(pane_key, _)| self.pane(pane_key)) + .and_then(|pane| { + pane.active.as_deref().and_then(|active| { + pane.tabs.iter().find(|tab| tab.panel.as_ref() == active) + }) + }) + .and_then(|tab| tab.right_view.clone()) + .or_else(|| first_right_view(&self.root)); cx.notify(); } } @@ -354,10 +363,14 @@ impl DockState { } self.detach_pane(pane_key); + if self.active_pane == pane_key { self.active_pane = self.first_pane_key().unwrap_or(0); } + // The closed pane's views are gone; show the first remaining one. + self.right_view = first_right_view(&self.root); + cx.notify(); } @@ -526,6 +539,19 @@ impl DockState { } } + fn pane(&self, key: u64) -> Option<&Pane> { + Self::pane_in(&self.root, key) + } + + fn pane_in(node: &Node, key: u64) -> Option<&Pane> { + match node { + Node::Pane(pane) => (pane.key == key).then_some(pane), + Node::Split(split) => { + Self::pane_in(&split.a, key).or_else(|| Self::pane_in(&split.b, key)) + } + } + } + fn pane_mut(&mut self, key: u64) -> Option<&mut Pane> { Self::pane_mut_in(&mut self.root, key) } @@ -1071,7 +1097,7 @@ fn render_pane( .h(titlebar.height) .items_center() .pl(if titlebar.traffic_light_pad { - px(TRAFFIC_LIGHT_PAD) + TRAFFIC_LIGHT_PAD } else { padding_x }) diff --git a/crates/layout/src/resize_handle.rs b/crates/layout/src/resize_handle.rs index 377cad1..38ca35a 100644 --- a/crates/layout/src/resize_handle.rs +++ b/crates/layout/src/resize_handle.rs @@ -10,11 +10,11 @@ use gpui::{ SharedString, StatefulInteractiveElement as _, Styled as _, Window, canvas, deferred, div, fill, point, px, }; -use gpui_native_theme::{ActiveTheme as _, Size}; +use gpui_native_theme::{ActiveTheme as _, RESIZE_KEYBOARD_STEP, Size}; use gpui_native_utils::{focus_handle, focus_ring}; /// How far one keyboard arrow step moves the seam. -const KEYBOARD_STEP: f32 = 32.0; +const KEYBOARD_STEP: Pixels = RESIZE_KEYBOARD_STEP; type ResizeHandler = Rc; type ResetHandler = Rc; @@ -180,10 +180,10 @@ impl RenderOnce for ResizeHandle { let on_resize = self.on_resize.clone(); move |event: &KeyDownEvent, window, cx| { let delta = match (axis, event.keystroke.key.as_str()) { - (Axis::Horizontal, "left") => Some(px(-KEYBOARD_STEP)), - (Axis::Horizontal, "right") => Some(px(KEYBOARD_STEP)), - (Axis::Vertical, "up") => Some(px(-KEYBOARD_STEP)), - (Axis::Vertical, "down") => Some(px(KEYBOARD_STEP)), + (Axis::Horizontal, "left") => Some(-KEYBOARD_STEP), + (Axis::Horizontal, "right") => Some(KEYBOARD_STEP), + (Axis::Vertical, "up") => Some(-KEYBOARD_STEP), + (Axis::Vertical, "down") => Some(KEYBOARD_STEP), _ => None, }; if let Some(delta) = delta { diff --git a/crates/layout/src/shell.rs b/crates/layout/src/shell.rs index 1eb232b..e718c70 100644 --- a/crates/layout/src/shell.rs +++ b/crates/layout/src/shell.rs @@ -4,8 +4,7 @@ use gpui::prelude::FluentBuilder as _; use gpui::{ AnyElement, App, AppContext as _, Context, ElementId, Entity, InteractiveElement as _, - IntoElement, ParentElement as _, Pixels, RenderOnce, SharedString, Styled as _, Window, div, - px, + IntoElement, ParentElement as _, Pixels, RenderOnce, Styled as _, Window, div, px, }; use gpui_native_core::button::{Button, ButtonVariant}; use gpui_native_core::common::{Sizable as _, h_flex, v_flex}; @@ -108,20 +107,6 @@ impl Side { Side::Right => "right", } } - - fn toggle_icon(self) -> IconRole { - match self { - Side::Left => IconRole::PanelLeft, - Side::Right => IconRole::PanelRight, - } - } - - fn toggle_label(self) -> SharedString { - match self { - Side::Left => SharedString::from("Hide left sidebar"), - Side::Right => SharedString::from("Hide right sidebar"), - } - } } /// The application shell. @@ -204,6 +189,7 @@ impl RenderOnce for Shell { let width = state.read(cx).left_sidebar_width(); layout .child(sidebar_region( + (id.clone(), "left"), width, left.take() .map(|sidebar| sidebar.into_any_element()) @@ -242,6 +228,7 @@ impl RenderOnce for Shell { } })) .child(sidebar_region( + (id.clone(), "right"), width, right.take().unwrap_or_else(|| div().into_any_element()), Side::Right, @@ -262,6 +249,7 @@ impl RenderOnce for Shell { /// One sidebar: a header strip with the hide toggle over the app's content. #[allow(clippy::too_many_arguments)] fn sidebar_region( + id: impl Into, width: Pixels, content: AnyElement, side: Side, @@ -272,6 +260,7 @@ fn sidebar_region( window: &mut Window, cx: &mut App, ) -> AnyElement { + let id = id.into(); let theme = cx.theme(); let background = theme.colors.background; let strip_background = match (side, material) { @@ -280,7 +269,7 @@ fn sidebar_region( }; let strip = h_flex() - .id(SharedString::from(format!("sidebar-strip-{}", side.name()))) + .id((id.clone(), "strip")) .debug_selector(move || format!("sidebar-strip-{}", side.name())) .flex_none() .h(strip_height) @@ -288,40 +277,31 @@ fn sidebar_region( .items_center() .map(|strip| match side { Side::Left => strip.when(cfg!(target_os = "macos"), |strip| { - strip.pl(px(TRAFFIC_LIGHT_PAD)) + strip.pl(TRAFFIC_LIGHT_PAD) }), Side::Right => strip, }) .px(padding_x) .child(titlebar_strip( - div() - .id(SharedString::from(format!( - "sidebar-strip-drag-{}", - side.name() - ))) - .flex_1() - .self_stretch(), + div().id((id.clone(), "strip-drag")).flex_1().self_stretch(), window, cx, )) .map(|strip| match side { Side::Left => strip.child( - Button::new(SharedString::from(format!( - "sidebar-toggle-{}", - side.name() - ))) - .icon(side.toggle_icon()) - .variant(ButtonVariant::Ghost) - .small() - .accessibility_label(side.toggle_label()) - .on_click({ - let state = state.clone(); - move |_, _, cx| { - state.update(cx, |state, cx| state.set_left_sidebar_visible(false, cx)) - } - }), + Button::new((id, "sidebar-toggle")) + .icon(IconRole::PanelLeft) + .variant(ButtonVariant::Ghost) + .small() + .accessibility_label("Hide left sidebar") + .on_click({ + let state = state.clone(); + move |_, _, cx| { + state.update(cx, |state, cx| state.set_left_sidebar_visible(false, cx)) + } + }), ), - Side::Right => strip.child(WindowControls::new("sidebar-window-controls")), + Side::Right => strip.child(WindowControls::new((id, "window-controls"))), }); v_flex() diff --git a/crates/layout/src/window_controls.rs b/crates/layout/src/window_controls.rs index d4deaae..3560ff4 100644 --- a/crates/layout/src/window_controls.rs +++ b/crates/layout/src/window_controls.rs @@ -23,13 +23,9 @@ use gpui::Decorations; use gpui::{SharedString, px, rgba}; /// macOS traffic-lights zone: the pad a leading titlebar strip reserves. -pub const TRAFFIC_LIGHT_PAD: f32 = 78.0; -/// The width of a Windows caption button. +pub use gpui_native_theme::TRAFFIC_LIGHT_PAD; #[cfg(target_os = "windows")] -const CAPTION_WIDTH: f32 = 36.0; -/// Windows paints its caption close hover red in every theme. -#[cfg(target_os = "windows")] -const WINDOWS_CLOSE_HOVER: (f32, f32, f32) = (232.0 / 255.0, 17.0 / 255.0, 32.0 / 255.0); +use gpui_native_theme::{WINDOWS_CAPTION_WIDTH as CAPTION_WIDTH, WINDOWS_CLOSE_HOVER_HEX}; /// The window controls for the trailing end of a titlebar strip. #[derive(IntoElement)] @@ -150,7 +146,7 @@ impl RenderOnce for WindowControls { } /// Double-click on a titlebar strip maximizes where the platform does not do it. -pub fn maximize_on_double_click() -> impl Fn(&ClickEvent, &mut Window, &mut App) + 'static { +fn maximize_on_double_click() -> impl Fn(&ClickEvent, &mut Window, &mut App) + 'static { move |event, window, _cx| { if event.click_count() != 2 { return; @@ -210,16 +206,9 @@ fn caption_button( ) -> AnyElement { let theme = cx.theme(); let glyph_size = theme.sizes.small.caption_size; - let (close_r, close_g, close_b) = WINDOWS_CLOSE_HOVER; - let close_hover = rgba(( - (close_r * 255.0) as u8, - (close_g * 255.0) as u8, - (close_b * 255.0) as u8, - 0xff, - )); let hover = if close { - close_hover + rgba(WINDOWS_CLOSE_HOVER_HEX) } else { theme.colors.muted_hover }; diff --git a/crates/layout/src/window_frame.rs b/crates/layout/src/window_frame.rs index bc0c3d5..b123aa3 100644 --- a/crates/layout/src/window_frame.rs +++ b/crates/layout/src/window_frame.rs @@ -7,12 +7,8 @@ use gpui::{ IntoElement, MouseButton, ParentElement as _, Pixels, Point, ResizeEdge, Size, Styled as _, Tiling, Window, canvas, div, point, px, size, }; -use gpui_native_theme::ActiveTheme as _; +use gpui_native_theme::{ActiveTheme as _, BORDER_WIDTH, WINDOW_RESIZE_INSET}; -/// The width of the band inside the window's edge that resizes it. -const RESIZE_INSET: f32 = 4.0; -/// The hairline around the content. -const BORDER_WIDTH: f32 = 1.0; /// How much of the shadow's strength survives losing window focus. const INACTIVE_SHADOW_OPACITY: f32 = 0.7; @@ -28,7 +24,7 @@ pub fn frame(child: impl IntoElement, window: &mut Window, cx: &App) -> Div { tiling }; - let inset = px(RESIZE_INSET); + let inset = WINDOW_RESIZE_INSET; let border = cx.theme().colors.border; let radius = cx.theme().radius.overlay; @@ -75,10 +71,10 @@ pub fn frame(child: impl IntoElement, window: &mut Window, cx: &App) -> Div { .size_full() .overflow_hidden() .border_color(border) - .when(!tiling.top, |surface| surface.border_t(px(BORDER_WIDTH))) - .when(!tiling.bottom, |surface| surface.border_b(px(BORDER_WIDTH))) - .when(!tiling.left, |surface| surface.border_l(px(BORDER_WIDTH))) - .when(!tiling.right, |surface| surface.border_r(px(BORDER_WIDTH))) + .when(!tiling.top, |surface| surface.border_t(BORDER_WIDTH)) + .when(!tiling.bottom, |surface| surface.border_b(BORDER_WIDTH)) + .when(!tiling.left, |surface| surface.border_l(BORDER_WIDTH)) + .when(!tiling.right, |surface| surface.border_r(BORDER_WIDTH)) .when(!(tiling.top || tiling.left), |surface| { surface.rounded_tl(radius) }) diff --git a/crates/layout/tests/dock.rs b/crates/layout/tests/dock.rs index 270162e..d96a3bc 100644 --- a/crates/layout/tests/dock.rs +++ b/crates/layout/tests/dock.rs @@ -112,9 +112,9 @@ fn drag(visual: &mut VisualTestContext, from: gpui::Point, to: gpui::Poi fn register(dock: &Entity, tags: &[&'static str], cx: &mut TestAppContext) { cx.update(|cx| { - dock.update(cx, |dock, cx| { + dock.update(cx, |dock, _cx| { for tag in tags { - dock.register_panel(descriptor(tag), cx); + dock.register_panel(descriptor(tag)); } }); }); @@ -492,9 +492,9 @@ fn activating_a_panel_with_a_right_view_opens_the_sidebar() { let mut cx = TestAppContext::single(); let (dock, opened, mut visual) = sidebar_window(&mut cx); cx.update(|cx| { - dock.update(cx, |dock, cx| { - dock.register_panel(descriptor_with_right_sidebar("overview"), cx); - dock.register_panel(descriptor("inbox"), cx); + dock.update(cx, |dock, _cx| { + dock.register_panel(descriptor_with_right_sidebar("overview")); + dock.register_panel(descriptor("inbox")); }); }); @@ -519,9 +519,9 @@ fn closing_the_displayed_tab_falls_back_to_another_right_view() { let mut cx = TestAppContext::single(); let (dock, opened, mut visual) = sidebar_window(&mut cx); cx.update(|cx| { - dock.update(cx, |dock, cx| { - dock.register_panel(descriptor_with_right_sidebar("overview"), cx); - dock.register_panel(descriptor_with_right_sidebar("explore"), cx); + dock.update(cx, |dock, _cx| { + dock.register_panel(descriptor_with_right_sidebar("overview")); + dock.register_panel(descriptor_with_right_sidebar("explore")); }); }); open(&mut visual, &dock, "overview"); diff --git a/crates/layout/tests/window_controls.rs b/crates/layout/tests/window_controls.rs index dc2c57c..d513841 100644 --- a/crates/layout/tests/window_controls.rs +++ b/crates/layout/tests/window_controls.rs @@ -74,9 +74,7 @@ fn the_pill_bar_pads_for_traffic_lights_when_it_leads_the_window() { let mut cx = TestAppContext::single(); let (dock, mut visual) = dock_window(false, false, &mut cx); cx.update(|cx| { - dock.update(cx, |dock, cx| { - dock.register_panel(descriptor("overview"), cx) - }); + dock.update(cx, |dock, _cx| dock.register_panel(descriptor("overview"))); }); open(&mut visual, &dock, "overview"); @@ -87,9 +85,7 @@ fn the_pill_bar_pads_for_traffic_lights_when_it_leads_the_window() { let (dock, mut visual) = dock_window(false, true, &mut cx); let _ = &dock; cx.update(|cx| { - dock.update(cx, |dock, cx| { - dock.register_panel(descriptor("overview"), cx) - }); + dock.update(cx, |dock, _cx| dock.register_panel(descriptor("overview"))); }); open(&mut visual, &dock, "overview"); @@ -101,11 +97,11 @@ fn the_pill_bar_pads_for_traffic_lights_when_it_leads_the_window() { // The bare bar keeps the standard leading padding; the padded bar clears // the traffic lights. assert!( - bare_inset < px(TRAFFIC_LIGHT_PAD), + bare_inset < TRAFFIC_LIGHT_PAD, "the bare bar must not reserve the traffic-light pad (bare {bare_inset:?})" ); assert!( - padded_inset >= px(TRAFFIC_LIGHT_PAD) - px(1.), + padded_inset >= TRAFFIC_LIGHT_PAD - px(1.), "the pad must clear the traffic lights (bare {bare_inset:?}, padded {padded_inset:?})" ); } @@ -115,9 +111,7 @@ fn window_controls_render_only_when_the_flag_is_set() { let mut cx = TestAppContext::single(); let (dock, mut visual) = dock_window(false, false, &mut cx); cx.update(|cx| { - dock.update(cx, |dock, cx| { - dock.register_panel(descriptor("overview"), cx) - }); + dock.update(cx, |dock, _cx| dock.register_panel(descriptor("overview"))); }); open(&mut visual, &dock, "overview"); @@ -138,9 +132,7 @@ fn the_flag_hosts_the_platform_controls_in_the_bar() { let mut cx = TestAppContext::single(); let (dock, mut visual) = dock_window(true, false, &mut cx); cx.update(|cx| { - dock.update(cx, |dock, cx| { - dock.register_panel(descriptor("overview"), cx) - }); + dock.update(cx, |dock, _cx| dock.register_panel(descriptor("overview"))); }); open(&mut visual, &dock, "overview"); diff --git a/crates/theme/src/lib.rs b/crates/theme/src/lib.rs index 4bac397..e133963 100644 --- a/crates/theme/src/lib.rs +++ b/crates/theme/src/lib.rs @@ -23,9 +23,39 @@ pub use system::{AccessibilityPreferences, PartialPalette, SourceKind, SystemThe use gpui::{App, Global, Hsla, Pixels, WindowAppearance, WindowBackgroundAppearance, px}; use std::time::Duration; -/// The hairline width of the control border every button and field paints; -/// sized outside the stated metrics so control sizes match across platforms. +/// The hairline width of the control border every button and field paints. pub const BORDER_WIDTH: Pixels = px(1.0); +/// The tallest an anchored popup's list may grow. +pub const POPUP_MAX_HEIGHT: Pixels = px(320.0); +/// The widest a tooltip may grow. +pub const TOOLTIP_MAX_WIDTH: Pixels = px(360.0); +/// How far a tooltip sits from the pointer. +pub const TOOLTIP_OFFSET: Pixels = px(1.0); +/// The caret's thickness. +pub const CARET_WIDTH: Pixels = px(1.5); +/// The IME preedit underline's thickness. +pub const UNDERLINE_WIDTH: Pixels = px(1.0); +/// How far one wheel line scrolls, as a multiple of the line height. +pub const WHEEL_LINE_HEIGHT: f32 = 20.0; +/// How far one keyboard arrow step moves a resize seam. +pub const RESIZE_KEYBOARD_STEP: Pixels = px(32.0); +/// The inset around a window's edge that starts a resize gesture. +pub const WINDOW_RESIZE_INSET: Pixels = px(4.0); +/// The pad macOS reserves for its traffic lights at a strip's leading edge. +pub const TRAFFIC_LIGHT_PAD: Pixels = px(78.0); +/// The width of a Windows caption button. +pub const WINDOWS_CAPTION_WIDTH: Pixels = px(36.0); +/// Windows paints its caption close-button hover red in every theme. +pub const WINDOWS_CLOSE_HOVER_HEX: u32 = 0xE81120FF; +/// The monospace advance estimate as a share of the text size. +pub const MONO_ADVANCE_RATIO: f32 = 0.62; +/// How much a pixel avatar's tint washes over its background. +pub const PIXEL_AVATAR_TINT_ALPHA: f32 = 0.16; +/// Pixel avatar cell lightness multipliers. +pub const PIXEL_AVATAR_LIGHT_LIFT: f32 = 1.6; +pub const PIXEL_AVATAR_LIGHT_CAP: f32 = 0.95; +pub const PIXEL_AVATAR_DARK_FACTOR: f32 = 0.45; +pub const PIXEL_AVATAR_DARK_FLOOR: f32 = 0.18; /// Light or dark, resolved from the system appearance. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, serde::Deserialize)] diff --git a/docs/component-audit.md b/docs/component-audit.md index 9667a60..3115fab 100644 --- a/docs/component-audit.md +++ b/docs/component-audit.md @@ -65,6 +65,8 @@ gpui scopes `use_keyed_state` by the element-id path; a `RenderOnce` renders und All verified by workspace-wide greps with **zero call sites** outside their own definitions/tests. +> **Re-checked 2026-11-09** (post-P1): several entries below are superseded by the policy "visual/a11y surface stays even when dead" — see the current P2 plan at the bottom of this document for the authoritative delete/keep split. Notably kept: `PanelDescriptor::icon`, `DropdownMenu::icon()`/`.tab_stop()`, `Alert::icon()`/`.no_icon()`, `markdown plain_code()`, the `accessibility_label` convention, `ResizeHandle::disabled()`, `Resizable::sizes()`/`set_sizes()` (test fixture), `Separator::orientation()`. Still deleted: everything non-UI in the list. Already resolved by review passes: `Tab::lead` (deleted), `ComboboxChanged` re-verified still present. + | Item | Location | Note | |---|---|---| | `Slider::value()` | `slider.rs:83-86` | `new` already takes `value` | @@ -175,9 +177,50 @@ Rule: `crates/theme` owns every color and metric — no raw `rgb`/`rgba`/`hsla` --- -## Recommended fix order (before migrating `signed`) +## Progress -1. **P0 — shared keyed state sweep** (findings 1-4): ~15 lines total; add a two-sibling regression test per component, since single-instance tests provably mask this class. -2. **P0 — user-facing bugs**: IME UTF-16 conversion (5), combobox chevron (6), palette z-order (7), code_view click (8) + incremental misuse (9), resize capture loss (10), tab element ids (14). -3. **P1 — performance**: registry (11), caret blink (12), widest_row (13), the `debug_selector` batch, the per-frame clone batch. -4. **P2 — deletions**: the param/dead-API table and the over-engineering list — this is where "no over-engineering" actually gets enforced, and it shrinks the API surface `signed` will migrate onto. +### P0 — done (commit `73f7dc3`, reviewed) + +- Shared keyed-state sweep (findings 1-4): all `use_keyed_state` keys carry the instance id; `Scrollbar::vertical/horizontal(id, handle)` now require an id, `.key()` deleted. +- Correctness bugs (5, 6, 7, 8, 9, 10, 14): IME UTF-16 conversion fixed, combobox chevron lists all options, palette modal deferred + dead bounds machinery removed, code_view click honors scroll, tree-sitter incremental misuse deleted (`highlight_with_tree` folded into `highlight`), resize drag ends on capture loss, `Tab::new(key, label)` with stable element ids. +- Regression tests added: two-sibling independence for slider / dropdown / scrollbar, resize capture-loss, multibyte UTF-16 conversions. + +### P1 — done (in working tree, uncommitted) + +- Registry (11): keyed by `(WindowId, ElementId)`, mutated in place via `cx.default_global` — no whole-map clone, no cross-window bleed. `anchor_bounds` takes the window id now. +- Caret blink (12): `with_max_fps(4.0)`; `field_bench` updated to probe the capped contract (executor-timer redraw, no next-frame demand). +- widest_row (13): cached in `set_rows` (called by `new`/`set_versions`). +- `debug_selector` batch: all eager formats moved into closures (switch, field ×3, combobox ×2, command_palette ×2, tabs ×2, sidebar, code_view, markdown, dock ×2 reusing the `.id()` format). +- Per-frame clone batch: segmented options are `Rc<[SegmentedOption]>` (closures capture bumps; `selectable` Vec → index math), select trigger text computed in render, menu/dropdown items passed as `Rc<[MenuItem]>`, alert borrows entries, code_view borrows the row `Line`. +- Skipped deliberately: the caret animation-id `format!` (removing it needs id plumbing through `InputState` for a ~30-byte alloc — over-engineering). +- Decision: registry stays a gpui `Global`, not a `OnceLock`. `WindowId`s come from a per-`App` slotmap, so a process-static map would leak state across tests (every `TestAppContext` mints the same early window ids); a Global dies with its `App` and needs no locking. + +Validation after P1: `cargo check --workspace --all-targets` clean, `cargo test --workspace` 64 binaries 0 failures, `cargo fmt --check` clean. + +### P2 — done (in working tree, uncommitted) + +Re-verified the whole P2 list against the tree first. Policy recorded in the plan below: visual/a11y surface stays even when dead; only non-UI dead code was deleted. + +**1a — deleted (non-UI dead API):** `Slider::value()`, `ComboboxChanged` + its `EventEmitter` impl + emit, `CodeView::language()`, `CodeView::clear_selection()`, `Input::submit_on()` getter, `MarkdownTableCell.column` (+ bookkeeping + test assertion), `Side::toggle_icon`/`toggle_label` (left values inlined at the call site), `render_tab`'s duplicate `selected` param (derived from `selected_index`), `register_panel`'s first-insert-only notify and its `cx` parameter (all call sites updated), `Menu checked` field → plain `bool`, `refilter` → `()`. Visibility narrowed: `scrollbar_state_key` → `pub(crate)`, `maximize_on_double_click` → private, `into_bindings` → `pub(crate)`. + +**1b — kept (UI/a11y surface, dead is fine):** `PanelDescriptor::icon`, `DropdownMenu::icon()`/`.tab_stop()`, `Alert::icon()`/`.no_icon()`, `markdown plain_code()`, the whole `accessibility_label` convention, `ResizeHandle::disabled()`, `Resizable::sizes()`/`set_sizes()`, `Separator::orientation()`, `data` closure `&mut Window` params. One remaining fix moved to follow-ups: `Segmented`'s unlabeled aria fallback announcing internal ids. + +**2 — over-engineering deletions:** dialog's `BoundsCell` + capture canvas + bounds check deleted (panel `stop_propagation` is the one mechanism); scroll_area's ~150-line drifted painter deleted — it now composes the real `Scrollbar` (`ScrollbarVisibility`, `Autohide`, and the geometry helpers moved into `scrollbar.rs`), and `ScrollAxes::Both` now renders a horizontal thumb too, resolving the promised-but-unimplemented `Horizontal`; input's `Rc>>` → plain `Option` field, both always-`Some` handler `Option`s → non-optional (two `expect`s deleted), `caret_epoch_started: Option` → `Instant`; markdown's `CodeBuilder` and `TableBuilder` deleted (`String` / `Option>`), the unreachable footnote arm folded into the render-nothing group; avatar's `PixelAvatar.style` + `Styled` impl + duplicate clipping deleted; combobox `refilter` clamps the highlight and the identity round-trip bounds check became `highlight.filter(...)`; code_view's two no-op `.max(0)`s deleted; layout `Cargo.toml`: `dock` now depends on `resize_handle` directly instead of `resizable`. + +**3 — theme-token hoists:** new theme consts: `POPUP_MAX_HEIGHT` (the three `px(320.)` popup sites), `TOOLTIP_MAX_WIDTH`/`TOOLTIP_OFFSET`, `CARET_WIDTH`, `UNDERLINE_WIDTH`, `WHEEL_LINE_HEIGHT`, `RESIZE_KEYBOARD_STEP`, `WINDOW_RESIZE_INSET`, `TRAFFIC_LIGHT_PAD` (moved out of window_controls; dock/shell/tests import it from the theme), `WINDOWS_CAPTION_WIDTH`, `WINDOWS_CLOSE_HOVER_HEX`, `MONO_ADVANCE_RATIO` (the duplicated `0.62` estimate), `PIXEL_AVATAR_*` tint/shade factors. Button's Ghost border now paints `transparent_black()` instead of raw `hsla(0,0,0,0)` (kept painted for uniform control metrics), dropdown fallback uses `transparent_black()`, separator hairline uses `BORDER_WIDTH`. + +**4 — smaller cleanups:** input early-returns when a cursor/selection/wheel change is a no-op (no notify, no IME re-query, no blink-epoch reset); IME pre-edit steps merge into the previous undo entry unconditionally while a composition is active; `line.paint` errors logged instead of `.ok()`; `caret_visible` takes `&self`; `Alerts::clear` runs every entry's `on_dismiss` handler (matches `Toasts::clear`); dock `close_others`/`close_pane` now reconcile the displayed right view (`close_panel` already did); combobox arms row 0 unconditionally on open, so empty results show "No matches" instead of closing (palette parity); palette rows keyed `("command", command.id)` — stable domain ids, not per-keystroke indexes; shell element ids composed from the shell id (`(id, "strip")`, `(id, "strip-drag")`, `(id, "sidebar-toggle")`, `(id, "window-controls")`); sidebar a11y sets count sibling rows only (headers excluded); tree a11y position/size computed among same-depth siblings in one pass. + +Validation after P2: `cargo fmt --check` clean, `cargo check --workspace --all-targets` 0 errors/0 warnings, `cargo test --workspace` 64 binaries 0 failures. + +--- + +## Next step — `signed` migration + +The audit backlog (P0-P2) is complete. Remaining small follow-ups, none blocking: `Segmented`'s unlabeled aria fallback; `markdown plain_code()`'s double parse if a consumer ever uses it; `Separator::orientation()` fold if strictness is ever wanted. + +Kickoff: + +1. Clone `signed` (git.reya.info/reya/signed) beside this workspace. +2. Inventory its views against the component list; mark each usage as: direct fit / needs a small adapter / needs a new component. +3. Bring missing components in-tree here (per repo rules: components are written here, not ported), then migrate view-by-view. diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 23dbe46..d7663a8 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -424,294 +424,204 @@ impl Gallery { }) .into() }), - cx, ); - dock.register_panel( - PanelDescriptor::new("button", "Button", |_, cx| { - cx.new(|_| ButtonDemo { clicks: 0 }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("input", "Input", |_, cx| cx.new(InputDemo::new).into()), - cx, - ); - dock.register_panel( - PanelDescriptor::new("field", "Field", |window, cx| { - cx.new(|cx| FieldDemo::new(window, cx)).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("combobox", "Combobox", |window, cx| { - cx.new(|cx| ComboboxDemo::new(window, cx)).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("status-dot", "Status dot", |_, cx| { - cx.new(|_| StatusDotDemo).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("checkbox", "Checkbox", |_, cx| { - cx.new(|_| CheckboxDemo { checked: false }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("radio", "Radio", |_, cx| { - cx.new(|_| RadioDemo { - plan: SharedString::from("starter"), + dock.register_panel(PanelDescriptor::new("button", "Button", |_, cx| { + cx.new(|_| ButtonDemo { clicks: 0 }).into() + })); + dock.register_panel(PanelDescriptor::new("input", "Input", |_, cx| { + cx.new(InputDemo::new).into() + })); + dock.register_panel(PanelDescriptor::new("field", "Field", |window, cx| { + cx.new(|cx| FieldDemo::new(window, cx)).into() + })); + dock.register_panel(PanelDescriptor::new( + "combobox", + "Combobox", + |window, cx| cx.new(|cx| ComboboxDemo::new(window, cx)).into(), + )); + dock.register_panel(PanelDescriptor::new("status-dot", "Status dot", |_, cx| { + cx.new(|_| StatusDotDemo).into() + })); + dock.register_panel(PanelDescriptor::new("checkbox", "Checkbox", |_, cx| { + cx.new(|_| CheckboxDemo { checked: false }).into() + })); + dock.register_panel(PanelDescriptor::new("radio", "Radio", |_, cx| { + cx.new(|_| RadioDemo { + plan: SharedString::from("starter"), + }) + .into() + })); + dock.register_panel(PanelDescriptor::new("switch", "Switch", |_, cx| { + cx.new(|_| SwitchDemo { checked: true }).into() + })); + dock.register_panel(PanelDescriptor::new("toggle", "Toggle", |_, cx| { + cx.new(|_| ToggleDemo { pressed: false }).into() + })); + dock.register_panel(PanelDescriptor::new("slider", "Slider", |_, cx| { + cx.new(|_| SliderDemo { volume: 40.0 }).into() + })); + dock.register_panel(PanelDescriptor::new("segmented", "Segmented", |_, cx| { + cx.new(|_| SegmentedDemo { + filter: ExploreFilter::default(), + }) + .into() + })); + dock.register_panel(PanelDescriptor::new("select", "Select", |_, cx| { + cx.new(|_| SelectDemo { + strategy: Some(Strategy::Curated), + }) + .into() + })); + dock.register_panel(PanelDescriptor::new("link", "Link", |_, cx| { + cx.new(|_| LinkDemo { clicks: 0 }).into() + })); + dock.register_panel(PanelDescriptor::new("clipboard", "Clipboard", |_, cx| { + cx.new(|_| ClipboardDemo).into() + })); + dock.register_panel(PanelDescriptor::new("badge", "Badge", |_, cx| { + cx.new(|_| BadgeDemo).into() + })); + dock.register_panel(PanelDescriptor::new("avatar", "Avatar", |_, cx| { + cx.new(|_| AvatarDemo).into() + })); + dock.register_panel(PanelDescriptor::new("progress", "Progress", |_, cx| { + cx.new(|_| ProgressDemo).into() + })); + dock.register_panel(PanelDescriptor::new("spinner", "Spinner", |_, cx| { + cx.new(|_| SpinnerDemo).into() + })); + dock.register_panel(PanelDescriptor::new("list", "List", |_, cx| { + cx.new(|_| ListDemo { selected: None }).into() + })); + dock.register_panel(PanelDescriptor::new("tree", "Tree", |_, cx| { + cx.new(|_| TreeDemo { + nodes: TreeDemo::demo(), + selected: None, + }) + .into() + })); + dock.register_panel(PanelDescriptor::new("tabs", "Tabs", |_, cx| { + cx.new(|_| TabsDemo { + selected: 0, + pill_tabs: vec![ + SharedString::from("Explorer"), + SharedString::from("Syncing"), + SharedString::from("Blame"), + ], + pill_selected: 0, + }) + .into() + })); + dock.register_panel(PanelDescriptor::new("alert", "Alert", { + let alerts = alerts.clone(); + move |_, cx| { + cx.new(|_| AlertDemo { + alerts: alerts.clone(), + dismissed: false, }) .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("switch", "Switch", |_, cx| { - cx.new(|_| SwitchDemo { checked: true }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("toggle", "Toggle", |_, cx| { - cx.new(|_| ToggleDemo { pressed: false }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("slider", "Slider", |_, cx| { - cx.new(|_| SliderDemo { volume: 40.0 }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("segmented", "Segmented", |_, cx| { - cx.new(|_| SegmentedDemo { - filter: ExploreFilter::default(), + } + })); + dock.register_panel(PanelDescriptor::new("toast", "Toast", { + let toasts = toasts.clone(); + move |_, cx| { + cx.new(|_| ToastDemo { + toasts: toasts.clone(), }) .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("select", "Select", |_, cx| { - cx.new(|_| SelectDemo { - strategy: Some(Strategy::Curated), + } + })); + dock.register_panel(PanelDescriptor::new("dialog", "Dialog", { + let gallery = gallery.clone(); + move |_, cx| { + cx.new(|_| DialogDemo { + gallery: gallery.clone(), }) .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("link", "Link", |_, cx| { - cx.new(|_| LinkDemo { clicks: 0 }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("clipboard", "Clipboard", |_, cx| { - cx.new(|_| ClipboardDemo).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("badge", "Badge", |_, cx| cx.new(|_| BadgeDemo).into()), - cx, - ); - dock.register_panel( - PanelDescriptor::new("avatar", "Avatar", |_, cx| cx.new(|_| AvatarDemo).into()), - cx, - ); - dock.register_panel( - PanelDescriptor::new("progress", "Progress", |_, cx| { - cx.new(|_| ProgressDemo).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("spinner", "Spinner", |_, cx| cx.new(|_| SpinnerDemo).into()), - cx, - ); - dock.register_panel( - PanelDescriptor::new("list", "List", |_, cx| { - cx.new(|_| ListDemo { selected: None }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("tree", "Tree", |_, cx| { - cx.new(|_| TreeDemo { - nodes: TreeDemo::demo(), - selected: None, - }) - .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("tabs", "Tabs", |_, cx| { - cx.new(|_| TabsDemo { - selected: 0, - pill_tabs: vec![ - SharedString::from("Explorer"), - SharedString::from("Syncing"), - SharedString::from("Blame"), - ], - pill_selected: 0, - }) - .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("alert", "Alert", { - let alerts = alerts.clone(); - move |_, cx| { - cx.new(|_| AlertDemo { - alerts: alerts.clone(), - dismissed: false, - }) - .into() - } - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("toast", "Toast", { - let toasts = toasts.clone(); - move |_, cx| { - cx.new(|_| ToastDemo { - toasts: toasts.clone(), - }) - .into() - } - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("dialog", "Dialog", { - let gallery = gallery.clone(); - move |_, cx| { - cx.new(|_| DialogDemo { - gallery: gallery.clone(), - }) - .into() - } - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("command-palette", "Command palette", { + } + })); + dock.register_panel(PanelDescriptor::new( + "command-palette", + "Command palette", + { let palette = palette.clone(); move |_, cx| { cx.new(|cx| CommandPaletteDemo::new(gallery.clone(), palette.clone(), cx)) .into() } - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("menu", "Menu", |_, cx| { - cx.new(|_| MenuDemo { open: false }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("dropdown-menu", "Dropdown menu", |_, cx| { + }, + )); + dock.register_panel(PanelDescriptor::new("menu", "Menu", |_, cx| { + cx.new(|_| MenuDemo { open: false }).into() + })); + dock.register_panel(PanelDescriptor::new( + "dropdown-menu", + "Dropdown menu", + |_, cx| { cx.new(|_| DropdownMenuDemo { open_menu: None, status: None, }) .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("context-menu", "Context menu", |_, cx| { + }, + )); + dock.register_panel(PanelDescriptor::new( + "context-menu", + "Context menu", + |_, cx| { cx.new(|_| ContextMenuDemo { action: SharedString::from("Right-click the card"), }) .into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("popover", "Popover", |_, cx| { - cx.new(|_| PopoverDemo { open: false }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("tooltip", "Tooltip", |_, cx| cx.new(|_| TooltipDemo).into()), - cx, - ); - dock.register_panel( - PanelDescriptor::new("empty-state", "Empty state", |_, cx| { - cx.new(|_| EmptyStateDemo).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("scroll-area", "Scroll area", |_, cx| { - cx.new(|_| ScrollAreaDemo).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("scrollbar", "Scrollbar", |_, cx| { - cx.new(|_| ScrollbarDemo::new()).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("code-view", "Code view", |_, cx| { - cx.new(CodeViewDemo::new).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("diff-view", "Diff view", |_, cx| { - cx.new(DiffViewDemo::new).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("markdown-view", "Markdown view", |_, cx| { - cx.new(MarkdownViewDemo::new).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("sidebar", "Sidebar", |_, cx| { - cx.new(|_| SidebarDemo { selected: None }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("resize-handle", "Resize handle", |_, cx| { - cx.new(|_| ResizeDemo { width: px(200.) }).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("resizable", "Resizable", |_, cx| { - cx.new(ResizableDemo::new).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("separator", "Separator", |_, cx| { - cx.new(|_| SeparatorDemo).into() - }), - cx, - ); - dock.register_panel( - PanelDescriptor::new("theme", "Theme", |_, cx| cx.new(|_| ThemeDemo).into()), - cx, - ); + }, + )); + dock.register_panel(PanelDescriptor::new("popover", "Popover", |_, cx| { + cx.new(|_| PopoverDemo { open: false }).into() + })); + dock.register_panel(PanelDescriptor::new("tooltip", "Tooltip", |_, cx| { + cx.new(|_| TooltipDemo).into() + })); + dock.register_panel(PanelDescriptor::new( + "empty-state", + "Empty state", + |_, cx| cx.new(|_| EmptyStateDemo).into(), + )); + dock.register_panel(PanelDescriptor::new( + "scroll-area", + "Scroll area", + |_, cx| cx.new(|_| ScrollAreaDemo).into(), + )); + dock.register_panel(PanelDescriptor::new("scrollbar", "Scrollbar", |_, cx| { + cx.new(|_| ScrollbarDemo::new()).into() + })); + dock.register_panel(PanelDescriptor::new("code-view", "Code view", |_, cx| { + cx.new(CodeViewDemo::new).into() + })); + dock.register_panel(PanelDescriptor::new("diff-view", "Diff view", |_, cx| { + cx.new(DiffViewDemo::new).into() + })); + dock.register_panel(PanelDescriptor::new( + "markdown-view", + "Markdown view", + |_, cx| cx.new(MarkdownViewDemo::new).into(), + )); + dock.register_panel(PanelDescriptor::new("sidebar", "Sidebar", |_, cx| { + cx.new(|_| SidebarDemo { selected: None }).into() + })); + dock.register_panel(PanelDescriptor::new( + "resize-handle", + "Resize handle", + |_, cx| cx.new(|_| ResizeDemo { width: px(200.) }).into(), + )); + dock.register_panel(PanelDescriptor::new("resizable", "Resizable", |_, cx| { + cx.new(ResizableDemo::new).into() + })); + dock.register_panel(PanelDescriptor::new("separator", "Separator", |_, cx| { + cx.new(|_| SeparatorDemo).into() + })); + dock.register_panel(PanelDescriptor::new("theme", "Theme", |_, cx| { + cx.new(|_| ThemeDemo).into() + })); } } -- 2.54.0 From 37d95d3c75ec53a35c8e9c1d257a546bbeae9b66 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Mon, 9 Nov 2026 16:00:19 +0700 Subject: [PATCH 5/5] clean up --- ...arkdown-view-comparison.md => MARKDOWN.md} | 0 docs/component-audit.md | 226 ------------------ 2 files changed, 226 deletions(-) rename docs/{markdown-view-comparison.md => MARKDOWN.md} (100%) delete mode 100644 docs/component-audit.md diff --git a/docs/markdown-view-comparison.md b/docs/MARKDOWN.md similarity index 100% rename from docs/markdown-view-comparison.md rename to docs/MARKDOWN.md diff --git a/docs/component-audit.md b/docs/component-audit.md deleted file mode 100644 index 3115fab..0000000 --- a/docs/component-audit.md +++ /dev/null @@ -1,226 +0,0 @@ -# Component audit — gpui-native-kit - -Full audit of every component ahead of migrating `signed` onto this kit. Scope: all 7 crates (~21k LOC) — `crates/core` (30 components), `crates/data`, `crates/editor`, `crates/layout`, plus the foundation crates `crates/theme`, `crates/icons`, `crates/utils`, the facade, and every `Cargo.toml` feature graph. - -Audit criteria, in priority order: no over-engineering, justified function arguments only, no memory leaks, best performance. - -## Method - -Seven parallel read-only audits (one per component group) plus a manual audit of the foundation crates. Every gpui-behavior claim was verified against the pinned zed checkout (`504a7f2`, per `Cargo.lock`); every "unused param" claim was backed by workspace-wide greps (`crates/`, `examples/`, `docs/`, `tests/`). All 15 major findings were re-verified by reading the cited lines. No code was modified; static audit only, no tests run. - -- Date: 2026-11-09 -- gpui rev: `504a7f2` (zed checkout, per `Cargo.lock`) -- Revision audited: pre-`signed` migration state - ---- - -## Verdict per criterion - -| Criterion | Verdict | -|---|---| -| **Memory leaks** | Near-clean. Zero `.detach()` on component tasks/subscriptions anywhere in `crates/` (grep-verified); tasks stored in fields, self-references via `WeakEntity`, undo history bounded (100 entries), icon cache bounded, keyed element state pruned by gpui per frame. **One real leak:** `AnchorBoundsRegistry` in utils (finding 11), plus one stuck-state bug in `resize_handle` (finding 10). | -| **Over-engineering** | A handful of cases: dead defensive machinery (dialog/command_palette bounds plumbing), drifted duplicate painter (scroll_area), pass-through wrappers (markdown), single-use indirection, one speculative persistence API (resizable). | -| **Function arguments** | ~20 removal candidates found, all with zero-call-site evidence (list below). Feature graphs and module gating are clean (verified per-file against actual imports). | -| **Performance** | Three systemic hot-path issues (registry clone churn, uncapped caret blink, per-frame O(N) scan in diff_view) plus a recurring `debug_selector` pattern that allocates every frame in release builds across 7+ sites. | - ---- - -## Major findings (15, all verified) - -### A. Shared keyed state across sibling instances — root cause: un-namespaced keys - -gpui scopes `use_keyed_state` by the element-id path; a `RenderOnce` renders under its **type name**, so two instances of the same component under the same ancestor chain resolve to the *same* keyed state. Existing tests never catch this class because every test window mounts exactly one instance. - -| # | Location | Bug | Fix | -|---|---|---|---| -| 1 | `crates/core/src/menu.rs:385,395-396` | `"menu-was-open"` / `"menu-scroll"` / `"menu-selection"` shared by all `Menu`s in a window → a closed sibling resets the open menu's selection every frame; arrow-key navigation and the selection highlight break with 2+ menus (the gallery ships 5 sibling dropdowns at `examples/gallery/src/main.rs:2761-2860`). | Wrap the three calls in `window.with_element_namespace(id.clone(), …)` — the instance `id` is already a parameter. `context_menu.rs:67-77` is the correct in-repo pattern. | -| 2 | `crates/core/src/dropdown_menu.rs:210` | Same bug: `"dropdown-open-origin"` bare key. | Same namespacing. | -| 3 | `crates/core/src/slider.rs:143` | `"slider-dragging"` bare key → dragging one slider drives both. | Key by `(self.id.clone(), "slider-dragging")` — the pattern `clipboard.rs:105` already uses. | -| 4 | `crates/core/src/scrollbar.rs:144,157` | Default key `ElementId::Name("scrollbar")` → two default-keyed scrollbars in one view share drag + autohide state (gallery renders two side by side at `main.rs:3231,3257`). | Make the id a required constructor parameter (`Scrollbar::vertical(id, handle)`) and delete `.key()`; 6 call sites, most already have ids to pass. | - -### B. Correctness bugs - -| # | Location | Bug | Fix | -|---|---|---|---| -| 5 | `crates/core/src/input.rs:2518` (+ `:1802`) | `utf8_range_to_utf16` has a byte-for-byte copy of `utf16_range_to_utf8`'s body — it never converts UTF-8→UTF-16. Every IME selection/marked-range report is wrong for non-ASCII text (CJK/emoji candidate anchoring). `character_index_for_point` likewise returns UTF-8 where the trait contract (gpui `platform.rs`) demands UTF-16. ASCII-only tests cannot catch this. | Implement the real conversion (`encode_utf16().count()`); add a non-ASCII regression test. | -| 6 | `crates/core/src/combobox.rs:509-518` | Chevron-open with a selected value refilters using the field text — which *is* the selected label — so only the selected option is listed. Inconsistent with `Select` (reopens listing all) and with query-focused open. | `refilter("")` when `state.selected.is_some()` in the chevron branch. | -| 7 | `crates/core/src/command_palette.rs:481-510` | The modal overlay is not `deferred` — z-order depends on mount position; in the gallery, alerts/toasts paint and receive clicks above the open modal's scrim. Also: the whole `panel_bounds` `Rc>>` + capture canvas is dead (the scrim is a sibling of the panel, so its mouse handler cannot fire for panel clicks). | Wrap in `deferred(…)` like `alert.rs:303` / `toast.rs:221`; delete the bounds machinery, keep a bare canvas for the focus-steal paint. | -| 8 | `crates/editor/src/code_view.rs:351` | Mouse-down maps position with `Pixels::ZERO` for scroll while mouse-move correctly uses `ScrollbarHandle::offset(...).y` → clicking after scrolling selects the wrong row. | Pass the same offset in the down handler. | -| 9 | `crates/editor/src/code_view.rs:102-108,137` | tree-sitter `old_tree` reused across changed text without `Tree::edit()` — violates the documented parse contract → stale/garbled highlighting after edits (the gallery's Rust→Large sample toggle triggers it). | Delete the `tree` field and the `old_tree` param of `highlight_with_tree`; full reparse per change. Reintroduce incremental parsing only when a real consumer needs it. | -| 10 | `crates/layout/src/resize_handle.rs:241-268` | The move handler checks only the stored `dragging` flag, never `event.pressed_button`. macOS synthetic drag does not synthesize a `MouseUpEvent` on outside-release → the seam keeps resizing on hover until the next click (stuck interaction state). | In the move handler, clear `dragging` when `event.pressed_button != Some(MouseButton::Left)`. | - -### C. Memory / performance - -| # | Location | Bug | Fix | -|---|---|---|---| -| 11 | `crates/utils/src/lib.rs:40-73` | `AnchorBoundsRegistry`: app-global `HashMap` that only inserts (never removes), keyed **without `WindowId`** (a two-window app can read a same-named element's bounds from the other window), and every record **clones the whole map** (`cx.try_global().cloned()` then `set_global`). Every `Button` records per frame (`button.rs:244-248`) → unbounded growth + O(N²)-ish per-frame allocation churn. Consumed by tooltip anchoring. | Key by `(WindowId, ElementId)`; mutate in place instead of clone-and-set (verify the in-place global-update API in the pinned gpui before rewriting). | -| 12 | `crates/core/src/input.rs:2049-2054` | Caret blink is an uncapped `Animation::new(CARET_BLINK_INTERVAL * 2).repeat()` → full-window repaint + full text re-shaping at display refresh rate whenever a field is focused (~119 wasted repaints/s at 120 Hz). Violates the repo's own `with_max_fps` rule. | `.with_max_fps(4.0)` (blink period is 1 s; `reduce_motion` is already handled by gpui). | -| 13 | `crates/editor/src/diff_view.rs:332-349` | `widest_row` recomputed with an O(document) scan **every render** (walks every span of every row to feed `with_width_from_item`) — defeats virtualization on large diffs. | Compute once in `new`/`set_versions`, store in a field. | -| 14 | `crates/data/src/tabs.rs:304,367,404` | Index-keyed element ids on a drag-reorderable strip (dock reorders tabs via `move_panel`) — violates the stable-domain-id rule; ids permute on every drag, re-keying interaction state, the spinner, and the close button. | Add a stable per-tab key (`Tab::new(key, label)` or `.key(SharedString)`); stable keys already exist at every call site (`tab.panel`, labels). The index-based `on_change`/`on_close` API stays unchanged. | -| 15 | `crates/core/src/menu.rs:461` | Per-frame `Vec` deep clone into the `on_key_down` closure — every label + handler cloned each frame the panel renders. | One `Rc<[MenuItem]>` per render; clone the `Rc` into the closure. | - ---- - -## Param / dead-API removals - -All verified by workspace-wide greps with **zero call sites** outside their own definitions/tests. - -> **Re-checked 2026-11-09** (post-P1): several entries below are superseded by the policy "visual/a11y surface stays even when dead" — see the current P2 plan at the bottom of this document for the authoritative delete/keep split. Notably kept: `PanelDescriptor::icon`, `DropdownMenu::icon()`/`.tab_stop()`, `Alert::icon()`/`.no_icon()`, `markdown plain_code()`, the `accessibility_label` convention, `ResizeHandle::disabled()`, `Resizable::sizes()`/`set_sizes()` (test fixture), `Separator::orientation()`. Still deleted: everything non-UI in the list. Already resolved by review passes: `Tab::lead` (deleted), `ComboboxChanged` re-verified still present. - -| Item | Location | Note | -|---|---|---| -| `Slider::value()` | `slider.rs:83-86` | `new` already takes `value` | -| `Separator::orientation()` | `separator.rs:46-49` | third way to set one enum; only its own test uses it | -| `Menu checked: Option` | `menu.rs:38,598` | only ever compared to `Some(true)`; `checked(false)` ≡ unset → plain `bool` (gallery/select call sites compile unchanged) | -| `Tab::lead` | `tabs.rs:37,67-74,342-357` | zero callers; also silently overrides `icon` when both set | -| `PanelDescriptor::icon` | `dock.rs:61` | zero of 45 constructor sites use it; delete the 5-hop plumbing (`PanelTab.icon`, `TabView.icon`, snapshot copy, open_panel copy, context-menu branch) | -| `DropdownMenu::icon()`, `.tab_stop()` | `dropdown_menu.rs:76,88` | untested and unexercised | -| `Alert::icon()` / `.no_icon()` | `alert.rs:82,88` | severity already derives an icon | -| `ComboboxChanged` event + `EventEmitter` impl | `combobox.rs:181,212,352` | zero subscribers workspace-wide; `on_change` covers it | -| `CodeView::language()` getter, `clear_selection()` | `code_view.rs:150,227` | zero callers | -| `markdown plain_code()` / `highlight_code()` | `markdown.rs:199-203` | cascade: delete the `highlight_code` flag + `parse` param; construction-time double parse goes with it | -| `MarkdownTableCell.column` | `markdown.rs:104` | derivable, unused by the renderer (only a test asserts it) | -| `Input::submit_on()` getter | `input.rs:698` | the setter `set_submit_on` is used; getter is not | -| `Input::accessibility_label` | `input.rs:1879` | placeholder/id fallback covers it | -| `ScrollArea::accessibility_label` | `scroll_area.rs:190` | | -| `scrollbar_state_key` → `pub(crate)` | `scroll_area.rs:200` | only consumer is `scrollbar.rs` | -| `ResizeHandle::accessibility_label`, `.disabled()` | `resize_handle.rs:70,64` | `disabled` used only by its own test | -| `Resizable::sizes()` / `set_sizes()` | `resizable.rs:38-49` | built for a persistence feature that doesn't exist (docs/CHECKLIST lists persistence as a follow-up) | -| `Side::Right` arms in `toggle_icon`/`toggle_label` | `shell.rs:112-124` | the right strip renders `WindowControls`; arms unreachable | -| `WindowControls::maximize_on_double_click` | `window_controls.rs:153` | single internal caller → private | -| `input into_bindings` | `input.rs:273` | only called by `bind_input_keys` → private | -| `render_tab(selected, selected_index)` | `tabs.rs:265-277` | same fact passed twice → keep `selected_index`, derive `selected` | -| `register_panel(cx)` notify | `dock.rs:195-199` | notifies on first insert only, and nothing renders from `panels` → drop notify + the `cx` parameter (45 call sites, mechanical) | -| `data` item closures taking `&mut Window` | `list.rs:102`, `sidebar.rs:148`, `tabs.rs:164`, `tree.rs:129` | every in-repo caller passes `|_, _|`; `&mut App` is real capability, `&mut Window` has no demonstrated use — judgment call, 9 call sites | -| `input copy_selection(_window)`, `caret_visible(&mut self)` | `input.rs:919,820` | unused param / needless mut | -| `accessibility_label` batch | `checkbox.rs:68`, `toggle.rs:65`, `switch.rs:68`, `link.rs:68`, `radio.rs:57,112`, `segmented.rs:114`, `popover.rs:83`, `dialog.rs:102`, `dropdown_menu.rs:112` | zero external callers on all of them. A uniform library convention — decide once: keep the convention or delete the dead ones. `Segmented`'s id-fallback announcing internal ids ("filter", "theme-demo-mode") should go regardless. | - ---- - -## Over-engineering - -- `dialog.rs:186,236-252` — the panel's `stop_propagation` fires before the scrim handler can, making the scrim's `panel_bounds.contains` check unreachable → the whole `BoundsCell` + capture canvas is dead machinery. One dismissal mechanism suffices. -- `scroll_area.rs:279-426` — a drifted ~150-line copy of `scrollbar.rs`'s painter (no horizontal axis, no track-click jump, different autohide policy); `ScrollAxes::Horizontal` is promised but unimplemented, with zero call sites. Delete the copy; compose the real `Scrollbar` element. This is deletion of a copy, not a new abstraction. -- `input.rs:525,563,1088` — `Rc>>` with a single owner → plain `Option` field. -- `input.rs:494-496,2180,2191` — two always-`Some` `Option>` fields papered over with `expect()`s → non-optional `Rc` fields, delete the expects. -- `input.rs:522,772,809` — `caret_epoch_started: Option` is never `None` → plain `Instant`. -- `markdown.rs:302-311` — `CodeBuilder` is a pass-through wrapper around `String` → use `String::push_str` directly. -- `markdown.rs:296-299` — `TableBuilder::None / Body { rows }` is exactly `Option>` → replace with the Option. -- `markdown.rs:662` — `Event::FootnoteReference` arm is unreachable (`ENABLE_FOOTNOTES` not enabled) and contradicts the module doc → delete. -- `avatar.rs:110-189` — user style refined twice (base + `PixelAvatar`), children re-clipped/re-rounded by the base anyway → delete `PixelAvatar.style` + its `Styled` impl + duplicate clipping. -- `combobox.rs:329-332` — `refilter` returns a `Vec` that all five call sites ignore (per-keystroke collect for nothing) → return `()`. -- `combobox.rs:320-333,430-433` — `refilter` doesn't clamp `highlight` (the command palette does); the identity round-trip `position(visible[row]) == row` is a bounds check in disguise → clamp in `refilter`, then `highlight.filter(|&row| row < visible.len())`. -- `code_view.rs:251-253` — `.max(0)` after `.clamp(0, len - 1)` is a no-op. -- `code_view.rs:282` / `diff_view.rs:390` — `char_width = text_size * 0.62` guessed monospace advance, duplicated in two files, drives hit-testing and row width → hoist to a theme metric. -- `segmented.rs:167-171` — `selectable: Vec = (0..len).collect()` is always `0..len` → index math. -- `Cargo.toml` (layout) — the `dock` feature enables `resizable` for nothing. - ---- - -## Performance (beyond the majors) - -### Eager `format!`/`to_string` for `debug_selector` - -gpui's cfg-gate only disables the method *body*; arguments allocate every frame **including release builds**. The correct pattern (already used in `clipboard.rs:126`, `status_dot.rs:97`) captures cheap clones and formats inside the closure: `.debug_selector(move || format!("{id}-row-{n}"))`. - -Sites: `switch.rs:136`, `field.rs:97-117` (×3), `combobox.rs:601,679`, `command_palette.rs:337,413`, `tabs.rs:305,409`, `sidebar.rs:198`, `code_view.rs:559`, `diff_view.rs:404,542`, `input.rs:2044`, `menu.rs:286` (+ the popover/dialog a11y-label fallbacks). - -### Per-frame clones/collects in render - -- `segmented.rs:167-171` — deep clone of every `SegmentedOption` + the `0..len` Vec, per frame → store `Rc<[SegmentedOption]>` at construction. -- `select.rs:209-225` — whole options `Vec` cloned per frame and the trigger re-derives the label inside a closure → compute `text` once in `render`, move only the `SharedString` in. -- `dock.rs:999-1111` — per pane per frame for the context menu: `pane.clone()`, `tabs.clone()`, three separate `panels` collects → capture one slim tuple Vec, collect once. -- `alert.rs:307` — `entries().to_vec()` clones the entry Vec every render of `AlertsView` (borrow-scoping workaround; low priority). -- `code_view.rs:478` — the whole `Line` cloned per visible row per frame; `render_row` only reads → borrow. -- `menu.rs:286` + `popover.rs:115`, `dropdown_menu.rs:205`, `dialog.rs:158` — per-frame `SharedString::from(self.id.to_string())` fallback labels. - -### No-op notifies - -- `input.rs:897-916,1253-1261` — `move_cursor`/`select_range`/`drag_select_to`/`scroll_by` always `cx.notify()` + `window.notify_text_input_changed(...)`: pressing Left at offset 0 or wheeling a non-scrollable field triggers a full re-render + IME re-query → early-return when cursor/anchor/scroll are unchanged. - -### Theme-token violations - -Rule: `crates/theme` owns every color and metric — no raw `rgb`/`rgba`/`hsla` or layout `px(...)` elsewhere. - -- `button.rs:202-205` — `hsla(0.0, 0.0, 0.0, 0.0)` ×4 as a "no border" color → apply the border conditionally instead. -- `dropdown_menu.rs:196` — `hsla(0.0, 0.0, 0.0, 0.0)` → `gpui::transparent_black()`. -- `avatar.rs:187` — `Hsla { a: 0.16, ..main }` + luminance multipliers `1.6/0.95/0.45/0.18` → theme token. -- `separator.rs:72` — `px(1.)` → `theme::BORDER_WIDTH` (already exists, used by button). -- `menu.rs:472`, `combobox.rs:666`, `command_palette.rs:404` — `px(320.)` popup max-height, three copies → one theme metric. -- `tooltip.rs:79,92` — `px(360.)` max width, `px(1.)` nudge. -- `input.rs:1195,1278` + `:32` — `px(1.5)` caret width, `px(1.)` underline, `SCROLL_LINES_HEIGHT = 20.0` → theme metrics (next to `theme.scrollbar.*`). -- `resize_handle.rs:17` — `KEYBOARD_STEP: f32 = 32.0`. -- `window_controls.rs:26,215-220,237` — `TRAFFIC_LIGHT_PAD`, `rgba(...)` Windows close-hover, caption width. -- `window_frame.rs:13-15` — `RESIZE_INSET`, `BORDER_WIDTH`. - -### Input misc - -- `input.rs:1726` — every IME composition step pushes undo history; steps >300 ms apart fragment undo → merge unconditionally while `marked_range` is active. -- `input.rs:1168` — `entry.line.paint(...).ok()` swallows a `Result` (failure = invisible text) → log like the shape path at `input.rs:984` does. -- `input.rs:820` — `caret_visible(&mut self)` doesn't mutate → `&self`. - -### Semantic inconsistencies - -- `alert.rs:264-269` — `Alerts::clear()` drops entries without running `on_dismiss` handlers (`Toasts::clear` runs them via `start_exit` → `finish_exit`) → run the handlers or document the difference. -- `scroll_area.rs:403-407` vs `scrollbar.rs:348-353` — autohide touch-on-drag vs touch-on-hover: two painters, two behaviors → pick one policy (subsumed by the scroll_area consolidation). -- `dock.rs:316-362` — `close_panel` reconciles the displayed right view when its tab closes, but `close_others`/`close_pane` don't → the sidebar can keep showing a closed panel's inspector. -- `combobox.rs:651-658` — "No matches" row is near-dead (empty results close the popup); the palette stays open showing "No matches" → pick one semantic. -- `command_palette.rs:336` — row ids are list indexes `("command", row)` for content that reorders every keystroke; `Command.id` is a stable `SharedString` → `("command", command.id.clone())`. -- `shell.rs:283-324` — hardcoded element ids not namespaced by the shell id (`"sidebar-strip-left"` etc.), unlike `Dock` which composes every id from `dock_id` → compose `(id, "…")`. -- `data` a11y sets — `sidebar.rs:270-298` counts headers in row sets; `tree.rs:250-261` uses flat-list position/size instead of per-sibling → compute among real siblings. - ---- - -## Clean bills - -`badge`, `progress` (fps-capped), `spinner`, `status_dot`, `clipboard`, `empty_state`, `context_menu` (the namespacing exemplar), `list`, `editor/lib` + `editor/utils` + `highlight`, `layout/lib`, `theme` (engine sound; watchers are app-lifetime; `set_preset` has one small redundant name lookup), `icons` (cache bounded, negatives cached), all `Cargo.toml` feature graphs, and all lifecycle plumbing: no detached tasks/subscriptions anywhere in `crates/`, `WeakEntity` used correctly, `cx.notify()` never from `render`, overlay state reclaimed by gpui's element-state eviction on dismiss. - ---- - -## Progress - -### P0 — done (commit `73f7dc3`, reviewed) - -- Shared keyed-state sweep (findings 1-4): all `use_keyed_state` keys carry the instance id; `Scrollbar::vertical/horizontal(id, handle)` now require an id, `.key()` deleted. -- Correctness bugs (5, 6, 7, 8, 9, 10, 14): IME UTF-16 conversion fixed, combobox chevron lists all options, palette modal deferred + dead bounds machinery removed, code_view click honors scroll, tree-sitter incremental misuse deleted (`highlight_with_tree` folded into `highlight`), resize drag ends on capture loss, `Tab::new(key, label)` with stable element ids. -- Regression tests added: two-sibling independence for slider / dropdown / scrollbar, resize capture-loss, multibyte UTF-16 conversions. - -### P1 — done (in working tree, uncommitted) - -- Registry (11): keyed by `(WindowId, ElementId)`, mutated in place via `cx.default_global` — no whole-map clone, no cross-window bleed. `anchor_bounds` takes the window id now. -- Caret blink (12): `with_max_fps(4.0)`; `field_bench` updated to probe the capped contract (executor-timer redraw, no next-frame demand). -- widest_row (13): cached in `set_rows` (called by `new`/`set_versions`). -- `debug_selector` batch: all eager formats moved into closures (switch, field ×3, combobox ×2, command_palette ×2, tabs ×2, sidebar, code_view, markdown, dock ×2 reusing the `.id()` format). -- Per-frame clone batch: segmented options are `Rc<[SegmentedOption]>` (closures capture bumps; `selectable` Vec → index math), select trigger text computed in render, menu/dropdown items passed as `Rc<[MenuItem]>`, alert borrows entries, code_view borrows the row `Line`. -- Skipped deliberately: the caret animation-id `format!` (removing it needs id plumbing through `InputState` for a ~30-byte alloc — over-engineering). -- Decision: registry stays a gpui `Global`, not a `OnceLock`. `WindowId`s come from a per-`App` slotmap, so a process-static map would leak state across tests (every `TestAppContext` mints the same early window ids); a Global dies with its `App` and needs no locking. - -Validation after P1: `cargo check --workspace --all-targets` clean, `cargo test --workspace` 64 binaries 0 failures, `cargo fmt --check` clean. - -### P2 — done (in working tree, uncommitted) - -Re-verified the whole P2 list against the tree first. Policy recorded in the plan below: visual/a11y surface stays even when dead; only non-UI dead code was deleted. - -**1a — deleted (non-UI dead API):** `Slider::value()`, `ComboboxChanged` + its `EventEmitter` impl + emit, `CodeView::language()`, `CodeView::clear_selection()`, `Input::submit_on()` getter, `MarkdownTableCell.column` (+ bookkeeping + test assertion), `Side::toggle_icon`/`toggle_label` (left values inlined at the call site), `render_tab`'s duplicate `selected` param (derived from `selected_index`), `register_panel`'s first-insert-only notify and its `cx` parameter (all call sites updated), `Menu checked` field → plain `bool`, `refilter` → `()`. Visibility narrowed: `scrollbar_state_key` → `pub(crate)`, `maximize_on_double_click` → private, `into_bindings` → `pub(crate)`. - -**1b — kept (UI/a11y surface, dead is fine):** `PanelDescriptor::icon`, `DropdownMenu::icon()`/`.tab_stop()`, `Alert::icon()`/`.no_icon()`, `markdown plain_code()`, the whole `accessibility_label` convention, `ResizeHandle::disabled()`, `Resizable::sizes()`/`set_sizes()`, `Separator::orientation()`, `data` closure `&mut Window` params. One remaining fix moved to follow-ups: `Segmented`'s unlabeled aria fallback announcing internal ids. - -**2 — over-engineering deletions:** dialog's `BoundsCell` + capture canvas + bounds check deleted (panel `stop_propagation` is the one mechanism); scroll_area's ~150-line drifted painter deleted — it now composes the real `Scrollbar` (`ScrollbarVisibility`, `Autohide`, and the geometry helpers moved into `scrollbar.rs`), and `ScrollAxes::Both` now renders a horizontal thumb too, resolving the promised-but-unimplemented `Horizontal`; input's `Rc>>` → plain `Option` field, both always-`Some` handler `Option`s → non-optional (two `expect`s deleted), `caret_epoch_started: Option` → `Instant`; markdown's `CodeBuilder` and `TableBuilder` deleted (`String` / `Option>`), the unreachable footnote arm folded into the render-nothing group; avatar's `PixelAvatar.style` + `Styled` impl + duplicate clipping deleted; combobox `refilter` clamps the highlight and the identity round-trip bounds check became `highlight.filter(...)`; code_view's two no-op `.max(0)`s deleted; layout `Cargo.toml`: `dock` now depends on `resize_handle` directly instead of `resizable`. - -**3 — theme-token hoists:** new theme consts: `POPUP_MAX_HEIGHT` (the three `px(320.)` popup sites), `TOOLTIP_MAX_WIDTH`/`TOOLTIP_OFFSET`, `CARET_WIDTH`, `UNDERLINE_WIDTH`, `WHEEL_LINE_HEIGHT`, `RESIZE_KEYBOARD_STEP`, `WINDOW_RESIZE_INSET`, `TRAFFIC_LIGHT_PAD` (moved out of window_controls; dock/shell/tests import it from the theme), `WINDOWS_CAPTION_WIDTH`, `WINDOWS_CLOSE_HOVER_HEX`, `MONO_ADVANCE_RATIO` (the duplicated `0.62` estimate), `PIXEL_AVATAR_*` tint/shade factors. Button's Ghost border now paints `transparent_black()` instead of raw `hsla(0,0,0,0)` (kept painted for uniform control metrics), dropdown fallback uses `transparent_black()`, separator hairline uses `BORDER_WIDTH`. - -**4 — smaller cleanups:** input early-returns when a cursor/selection/wheel change is a no-op (no notify, no IME re-query, no blink-epoch reset); IME pre-edit steps merge into the previous undo entry unconditionally while a composition is active; `line.paint` errors logged instead of `.ok()`; `caret_visible` takes `&self`; `Alerts::clear` runs every entry's `on_dismiss` handler (matches `Toasts::clear`); dock `close_others`/`close_pane` now reconcile the displayed right view (`close_panel` already did); combobox arms row 0 unconditionally on open, so empty results show "No matches" instead of closing (palette parity); palette rows keyed `("command", command.id)` — stable domain ids, not per-keystroke indexes; shell element ids composed from the shell id (`(id, "strip")`, `(id, "strip-drag")`, `(id, "sidebar-toggle")`, `(id, "window-controls")`); sidebar a11y sets count sibling rows only (headers excluded); tree a11y position/size computed among same-depth siblings in one pass. - -Validation after P2: `cargo fmt --check` clean, `cargo check --workspace --all-targets` 0 errors/0 warnings, `cargo test --workspace` 64 binaries 0 failures. - ---- - -## Next step — `signed` migration - -The audit backlog (P0-P2) is complete. Remaining small follow-ups, none blocking: `Segmented`'s unlabeled aria fallback; `markdown plain_code()`'s double parse if a consumer ever uses it; `Separator::orientation()` fold if strictness is ever wanted. - -Kickoff: - -1. Clone `signed` (git.reya.info/reya/signed) beside this workspace. -2. Inventory its views against the component list; mark each usage as: direct fit / needs a small adapter / needs a new component. -3. Bring missing components in-tree here (per repo rules: components are written here, not ported), then migrate view-by-view. -- 2.54.0