From fbc4f24d1692b721c002d29baf07730330f3abd4 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Tue, 10 Nov 2026 07:43:30 +0700 Subject: [PATCH] feat: replace sidebar row icon with prefix and suffix slots --- AGENTS.md | 2 +- crates/data/src/sidebar.rs | 122 ++++++++++++++++++----------------- docs/CHECKLIST.md | 7 ++ examples/gallery/src/main.rs | 21 ++++-- 4 files changed, 88 insertions(+), 64 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f41bc05..7152f6f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,7 +64,7 @@ GPUI is a UI framework which also provides primitives for state and concurrency ## Rendering and state - `Render` builds an element tree from entity state; `RenderOnce` (with `#[derive(IntoElement)]`) is for value-like components whose `render` takes `self` and `&mut App`. -- Keep `render` declarative; use `.when(...)` / `.when_some(...)` for small conditional refinements. +- Keep `render` declarative: build elements as one chained expression, using `.when(...)` / `.when_some(...)` / `.map(...)` for conditional refinements instead of `let mut` element variables reassigned under `if` / `if let`. - Controlled components: the owner passes the current value, callbacks report intent, the owner updates state and calls `cx.notify()`. No second source of truth. - Call `cx.notify()` after a mutation that changes rendering; notify the narrowest owning entity. Never notify from `render`. - `render` is side-effect free: no `cx.notify()`, no `entity.update(...)`, no writing derived text or state back into other entities during render. A write there forces another frame, and a write that emits an event re-enters the event handler — an unbounded redraw loop. Reconcile state in event handlers instead, with silent setters (`set_value_silent`-style) so a control's own write does not re-enter the events it mirrors. diff --git a/crates/data/src/sidebar.rs b/crates/data/src/sidebar.rs index e15103f..c2ba650 100644 --- a/crates/data/src/sidebar.rs +++ b/crates/data/src/sidebar.rs @@ -10,7 +10,6 @@ use gpui::{ Styled as _, Window, div, }; use gpui_native_core::scroll_area::ScrollArea; -use gpui_native_icons::Icon; use gpui_native_theme::ActiveTheme as _; use gpui_native_utils::{nav_key, paint_selectable_row, selectable_label_color}; @@ -19,15 +18,15 @@ type ChangeHandler = Rc, &mut Window, &mut App)>; type EntriesFn = Box Vec>; /// One entry of a [`Sidebar`], a section header or a selectable row. -#[derive(Clone)] pub enum SidebarEntry { /// A non-selectable section label. Header { label: SharedString }, /// A selectable row. Row { label: SharedString, - icon: Option, detail: Option, + prefix: Option, + suffix: Option, disabled: bool, }, } @@ -50,23 +49,13 @@ impl SidebarEntry { { Self::Row { label: label.into(), - icon: None, detail: None, + prefix: None, + suffix: None, disabled: false, } } - /// Shows an icon leading the label, a role or any custom [`IconNamed`]. - pub fn icon(mut self, icon: I) -> Self - where - I: Into, - { - if let Self::Row { icon: slot, .. } = &mut self { - *slot = Some(icon.into()); - } - self - } - /// Shows muted trailing text. pub fn detail(mut self, detail: D) -> Self where @@ -81,6 +70,28 @@ impl SidebarEntry { self } + /// Shows an element leading the row. + pub fn prefix(mut self, prefix: E) -> Self + where + E: IntoElement, + { + if let Self::Row { prefix: slot, .. } = &mut self { + *slot = Some(prefix.into_any_element()); + } + self + } + + /// Shows an element trailing the row. + pub fn suffix(mut self, suffix: E) -> Self + where + E: IntoElement, + { + if let Self::Row { suffix: slot, .. } = &mut self { + *slot = Some(suffix.into_any_element()); + } + self + } + /// Renders the row dimmed; clicks and keyboard skip it. pub fn disabled(mut self, disabled: bool) -> Self { if let Self::Row { @@ -207,7 +218,7 @@ impl RenderOnce for Sidebar { let mut row_position = 0usize; let children: Vec = entries - .iter() + .into_iter() .enumerate() .map(|(index, entry)| { let is_row = matches!(entry, SidebarEntry::Row { .. }); @@ -274,7 +285,7 @@ impl RenderOnce for Sidebar { #[allow(clippy::too_many_arguments)] fn render_entry( cx: &App, - entry: &SidebarEntry, + entry: SidebarEntry, index: usize, position_in_set: usize, size_of_set: usize, @@ -293,17 +304,18 @@ fn render_entry( .text_sm() .font_weight(gpui::FontWeight::MEDIUM) .text_color(theme.colors.muted_foreground) - .child(label.clone()) + .child(label) .into_any_element(), SidebarEntry::Row { label, - icon: row_icon, detail, + prefix, + suffix, disabled: row_disabled, } => { - let label_color = selectable_label_color(cx, selected, *row_disabled); + let label_color = selectable_label_color(cx, selected, row_disabled); - let mut row = div() + div() .id(("sidebar-row", index)) .flex() .items_center() @@ -316,51 +328,45 @@ fn render_entry( .aria_position_in_set(position_in_set) .aria_size_of_set(size_of_set) .map(|row| { - if material && !*row_disabled { + if material && !row_disabled { if selected { row.bg(theme.material.selection) } else { row.hover(|row| row.bg(theme.material.hover)) } } else { - paint_selectable_row(cx, row, selected, *row_disabled) + paint_selectable_row(cx, row, selected, row_disabled) } - }); - - if !*row_disabled { - row = row.cursor_pointer().on_click({ - let on_change = on_change.clone(); - let focus = focus.clone(); - move |_, window, cx| { - window.focus(&focus, cx); - if let Some(on_change) = &on_change { - on_change(Some(index), window, cx); + }) + .when(!row_disabled, |row| { + row.cursor_pointer().on_click({ + let on_change = on_change.clone(); + let focus = focus.clone(); + move |_, window, cx| { + window.focus(&focus, cx); + if let Some(on_change) = &on_change { + on_change(Some(index), window, cx); + } } - } - }); - } - - row = row.when_some(row_icon.clone(), |row, icon| { - row.child(icon.size(theme.sizes.medium.icon_size).color(label_color)) - }); - - row = row.child(div().flex_1().text_color(label_color).child(label.clone())); - - if let Some(detail) = detail { - row = row.child( - div() - .flex_none() - .text_sm() - .text_color(if selected { - theme.colors.primary_foreground - } else { - theme.colors.muted_foreground - }) - .child(detail.clone()), - ); - } - - row.into_any_element() + }) + }) + .when_some(prefix, |row, prefix| row.child(prefix)) + .child(div().flex_1().text_color(label_color).child(label)) + .when_some(detail, |row, detail| { + row.child( + div() + .flex_none() + .text_sm() + .text_color(if selected { + theme.colors.primary_foreground + } else { + theme.colors.muted_foreground + }) + .child(detail), + ) + }) + .when_some(suffix, |row, suffix| row.child(suffix)) + .into_any_element() } } } diff --git a/docs/CHECKLIST.md b/docs/CHECKLIST.md index dc81754..0f65713 100644 --- a/docs/CHECKLIST.md +++ b/docs/CHECKLIST.md @@ -9,6 +9,13 @@ is the first downstream consumer; its screens drive the order. ## Next +- [ ] **Declarative render audit** — run the AGENTS.md render rule over the + other landed components: replace `let mut` element variables reassigned + under `if` / `if let` with one chained expression using `.when(...)` / + `.when_some(...)` / `.map(...)`. The sidebar row is the model; sweep + `crates/core`, `crates/data`, `crates/editor`, `crates/layout`, and + re-run tests plus a gallery pass after each crate. + ## Queue — to follow Platform metric adoption continues one component at a time; each stop goes diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index 522b358..0b5ad73 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -3739,16 +3739,27 @@ impl Render for SidebarDemo { cx.notify(); }); }) - .entries(|_, _| { + .entries(|_, cx| { + let theme = cx.theme(); + let row_icon = || { + icon(IconRole::Folder).size(theme.sizes.medium.icon_size) + }; vec![ SidebarEntry::header("Favorites"), - SidebarEntry::row("Documents").icon(IconRole::Folder), + SidebarEntry::row("Documents").prefix(row_icon()), SidebarEntry::row("Pictures") - .icon(IconRole::Folder) - .detail("24"), + .prefix(row_icon()) + .detail("24") + .suffix( + Badge::new("new").variant(BadgeVariant::Accent), + ), SidebarEntry::row("Archive").disabled(true), SidebarEntry::header("Locations"), - SidebarEntry::row("iCloud").icon(IconRole::Folder), + SidebarEntry::row("iCloud").prefix(row_icon()).suffix( + StatusDot::new("sidebar-demo-icloud-status") + .small() + .variant(StatusDotVariant::Success), + ), SidebarEntry::row("Trash"), ] }),