diff --git a/AGENTS.md b/AGENTS.md index 922e8c25..df8b559d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,10 @@ * Prioritize code correctness and clarity. Speed and efficiency are secondary priorities unless otherwise specified. * Do not write organizational or comments that summarize the code. Comments should only be written in order to explain "why" the code is written in some way in the case there is a reason that is tricky / non-obvious. +* Keep doc comments and comments simple, one line, straightforward, and unbloated. * Prefer implementing functionality in existing files unless it is a new logical component. Avoid creating many small files. +* Prefer `impl` blocks over standalone functions: place functionality as methods or associated functions on the relevant type, extending the type's existing `impl` block instead of creating a duplicate one in another file. +* Bind a global store handle to a local before using it (`let state = GlobalState::global(cx);` then `state.update(...)`) instead of chaining on `GlobalState::global(cx)` inline. * Avoid using functions that panic like `unwrap()`, instead use mechanisms like `?` to propagate errors. * Be careful with operations like indexing which may panic if the indexes are out of bounds. * Never silently discard errors with `let _ =` on fallible operations. Always handle errors appropriately: @@ -81,6 +84,8 @@ Both `cx.spawn` and `cx.background_spawn` return a `Task`, which is a future A task which doesn't do anything but provide a value can be created with `Task::ready(value)`. +Prefer keeping a task in a field over `.detach()` when the work belongs to a view or store. A detached task outlives the view that started it (for example, a repository panel the user has closed), while a task stored in a field is cancelled when that struct drops. A finished `Task` held in a container is not reaped by GPUI - it stays alive until its handle is dropped - so if the container can accumulate many runs, either drop the finished handles before pushing a new one, or hold a single `Option>` and replace it. + ## Elements The `Render` trait is used to render some state into an element tree that is laid out using flexbox layout. An `Entity` where `T` implements `Render` is sometimes called a "view". diff --git a/Cargo.lock b/Cargo.lock index 276799ab..e228ae33 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7958,7 +7958,7 @@ dependencies = [ "gpui-base", "gpui-pre", "instant", - "serde", + "log", "smallvec", "theme", "uuid", diff --git a/crates/ui/Cargo.toml b/crates/ui/Cargo.toml index 05f32c64..3fd75c4a 100644 --- a/crates/ui/Cargo.toml +++ b/crates/ui/Cargo.toml @@ -10,7 +10,7 @@ theme = { path = "../theme" } gpui.workspace = true gpui-base.workspace = true instant.workspace = true -serde.workspace = true smallvec.workspace = true +log.workspace = true uuid = "1.10" diff --git a/crates/ui/src/animation.rs b/crates/ui/src/animation.rs index afb19265..d60dd5d9 100644 --- a/crates/ui/src/animation.rs +++ b/crates/ui/src/animation.rs @@ -1,8 +1,4 @@ -/// A cubic bezier function like CSS `cubic-bezier`. -/// -/// Builder: -/// -/// https://cubic-bezier.com +/// A cubic bezier easing function like CSS `cubic-bezier` (see https://cubic-bezier.com). pub fn cubic_bezier(x1: f32, y1: f32, x2: f32, y2: f32) -> impl Fn(f32) -> f32 { move |t: f32| { let one_t = 1.0 - t; diff --git a/crates/ui/src/avatar.rs b/crates/ui/src/avatar.rs index 52e6f422..979f51a4 100644 --- a/crates/ui/src/avatar.rs +++ b/crates/ui/src/avatar.rs @@ -1,4 +1,3 @@ -use gpui::prelude::FluentBuilder; use gpui::{ AbsoluteLength, AnyElement, App, Bounds, Div, Hsla, ImageSource, InteractiveElement, Interactivity, IntoElement, ObjectFit, ParentElement, PathBuilder, Pixels, Point, RenderOnce, @@ -24,7 +23,7 @@ const FALLBACK_SEED: &str = "coop"; const CIRCLE_SEGMENTS: usize = 32; /// Returns the size of the avatar based on the given [`Size`]. -pub(super) fn avatar_size(size: Size) -> AbsoluteLength { +fn avatar_size(size: Size) -> AbsoluteLength { match size { Size::Large => px(64.).into(), Size::Medium => px(32.).into(), @@ -36,7 +35,7 @@ pub(super) fn avatar_size(size: Size) -> AbsoluteLength { /// A deterministic, offline pixel-art avatar derived from a seed. #[derive(IntoElement)] -pub(crate) struct PixelAvatar { +struct PixelAvatar { seed: u64, size: Size, style: StyleRefinement, @@ -44,7 +43,7 @@ pub(crate) struct PixelAvatar { impl PixelAvatar { /// Creates a pixel avatar from `seed`. - pub fn new(seed: impl AsRef) -> Self { + fn new(seed: impl AsRef) -> Self { Self { seed: fnv1a(seed.as_ref().as_bytes()), size: Size::Medium, @@ -188,8 +187,7 @@ fn paint_polygons<'a>( } } -/// Approximates the circle of `radius` around `center` as a convex polygon, -/// wound so that its interior is on the left of every directed edge. +/// Approximates the circle of `radius` around `center` as a convex polygon wound left-interior. fn circle_polygon(center: Point, radius: f32) -> Vec> { let center_x = center.x.as_f32(); let center_y = center.y.as_f32(); @@ -354,11 +352,9 @@ fn generated_avatar(seed: Option<&str>, size: Pixels) -> AnyElement { pub struct Avatar { base: Div, picture: Option, - grayscale: bool, seed: Option, style: StyleRefinement, size: Size, - border_color: Option, selected: bool, } @@ -377,11 +373,9 @@ impl Avatar { Avatar { base: div(), picture, - grayscale: false, seed: None, style: StyleRefinement::default(), size: Size::Medium, - border_color: None, selected: false, } } @@ -425,25 +419,16 @@ impl InteractiveElement for Avatar { impl RenderOnce for Avatar { fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let border_width = if self.border_color.is_some() { - px(2.) - } else { - px(0.) - }; - let image_size = avatar_size(self.size).to_pixels(window.rem_size()); - let container_size = image_size + border_width * 2.; let content = match self.picture { Some(picture) => { let seed = self.seed; - let grayscale = self.grayscale; img(picture) .size(image_size) .rounded_full() .object_fit(ObjectFit::Cover) - .grayscale(grayscale) .bg(cx.theme().ghost_element_background) .with_fallback(move || generated_avatar(seed.as_deref(), image_size)) .into_any_element() @@ -453,12 +438,9 @@ impl RenderOnce for Avatar { div() .flex_shrink_0() - .size(container_size) + .size(image_size) .rounded_full() .overflow_hidden() - .when_some(self.border_color, |this, color| { - this.border(border_width).border_color(color) - }) .child(content) } } diff --git a/crates/ui/src/button.rs b/crates/ui/src/button.rs index 5ed81266..67c60f01 100644 --- a/crates/ui/src/button.rs +++ b/crates/ui/src/button.rs @@ -70,42 +70,34 @@ pub enum ButtonVariant { pub trait ButtonVariants: Sized { fn with_variant(self, variant: ButtonVariant) -> Self; - /// With the primary style for the Button. fn primary(self) -> Self { self.with_variant(ButtonVariant::Primary) } - /// With the secondary style for the Button. fn secondary(self) -> Self { self.with_variant(ButtonVariant::Secondary) } - /// With the danger style for the Button. fn danger(self) -> Self { self.with_variant(ButtonVariant::Danger) } - /// With the warning style for the Button. fn warning(self) -> Self { self.with_variant(ButtonVariant::Warning) } - /// With the ghost style for the Button. fn ghost(self) -> Self { self.with_variant(ButtonVariant::Ghost { alt: false }) } - /// With the ghost style for the Button. fn ghost_alt(self) -> Self { self.with_variant(ButtonVariant::Ghost { alt: true }) } - /// With the transparent style for the Button. fn transparent(self) -> Self { self.with_variant(ButtonVariant::Transparent) } - /// With the custom style for the Button. fn custom(self, style: ButtonCustomVariant) -> Self { self.with_variant(ButtonVariant::Custom(style)) } @@ -127,13 +119,9 @@ pub struct Button { rounded: bool, compact: bool, caret: bool, - indicator: bool, on_click: Option>, - on_hover: Option>, - tab_index: isize, tab_stop: bool, - - pub(crate) selected: bool, + selected: bool, } impl From