diff --git a/crates/moon-ui-components/src/moon/color_picker.rs b/crates/moon-ui-components/src/moon/color_picker.rs index e4301c1..8b50bf2 100644 --- a/crates/moon-ui-components/src/moon/color_picker.rs +++ b/crates/moon-ui-components/src/moon/color_picker.rs @@ -1,40 +1,183 @@ use gpui::prelude::FluentBuilder; use gpui::*; +use regex::Regex; use super::{ + foundation::v_flex, + input::{MoonInput, MoonInputEvent, MoonInputState}, popover::{MoonPopover, MoonPopoverPlacement}, text::MoonText, theme::MoonTheme, - tokens::{MoonPalette, rgba_from}, + tokens::{MoonPalette, MoonTone, rgba_from}, }; +/// Cap on remembered custom colours (most-recent-first). A larger stored/seeded list is silently +/// trimmed on the next `custom_colors`/`set_custom_colors` call — callers that persist a longer +/// list would see it quietly shrink, so keep any persisted cap at or below this one. +const MAX_CUSTOM_COLORS: usize = 20; + +/// Height budget for the swatch grid, in design-reference (unscaled) units — roughly 5 rows at +/// the current swatch size/gap. `MoonPopover` renders its content at intrinsic height with no cap +/// of its own (`popover.rs`), so a caller-supplied palette plus up to `MAX_CUSTOM_COLORS` custom +/// entries (e.g. the Badges tab's 65-swatch palette: up to 17 rows) can otherwise run the popup +/// off the bottom of the window in a long scrolling settings list. The grid scrolls past this +/// budget instead of growing without limit. +const GRID_MAX_HEIGHT_UI: f32 = 150.0; + +/// Charset gate for the hex field, applied per keystroke via `MoonInputState::pattern`. +/// +/// Deliberately permissive (any prefix of 0-6 hex digits, optional leading `#`) rather than a +/// full `#RRGGBB` match: a full-value pattern would reject every partial keystroke while typing +/// (e.g. `#F`), which is not what `is_valid_input` is for. Full-value validation happens only at +/// commit, in [`parse_hex_rgb`]. +const HEX_CHARSET: &str = r"^#?[0-9a-fA-F]{0,6}$"; + +/// Rounding (not truncating) sRGB bytes for a picker colour. +/// +/// The single source both `hex_label` and [`parse_hex_rgb`] go through, so the two are exact +/// inverses of each other. The naive `u32::from(rgba) >> 8` conversion this replaces truncates +/// instead of rounding, which used to only mis-label a swatch by up to 1/255 — harmless while the +/// label was read-only, but a value that round-trips through an editable field on every commit +/// must not drift. +fn rgb8_of(color: Hsla) -> [u8; 3] { + let c: Rgba = color.into(); + [ + (c.r * 255.0).round() as u8, + (c.g * 255.0).round() as u8, + (c.b * 255.0).round() as u8, + ] +} + +/// Parse a committed hex field value into a colour. +/// +/// Accepts `#RRGGBB` or bare `RRGGBB`, case-insensitive, with surrounding whitespace trimmed. +/// Deliberately rejects an 8-digit `#RRGGBBAA` (unlike the base Longbridge picker): this widget's +/// readout and every consumer are 6-digit/no-alpha, so an accepted 8-digit value could not +/// round-trip back through `hex_label`. +fn parse_hex_rgb(text: &str) -> Option { + let text = text.trim(); + let digits = text.strip_prefix('#').unwrap_or(text); + if digits.len() != 6 || !digits.bytes().all(|b| b.is_ascii_hexdigit()) { + return None; + } + let value = u32::from_str_radix(digits, 16).ok()?; + Some(rgb(value).into()) +} + +/// Push a newly-committed colour to the front of a most-recent-first custom-colour list. +/// +/// De-duplicates by [`rgb8_of`] (a re-typed colour moves to the front rather than appearing +/// twice) and caps the list at [`MAX_CUSTOM_COLORS`], dropping the oldest entries. +/// +/// Returns: +/// Whether the list's content or order actually changed — `false` when `color` was already +/// the front entry, so a caller mirroring this list into persisted state does not write on a +/// no-op commit. +fn push_custom(list: &mut Vec, color: Hsla) -> bool { + let bytes = rgb8_of(color); + let already_front = list.first().map(|c| rgb8_of(*c) == bytes).unwrap_or(false); + if already_front { + return false; + } + list.retain(|c| rgb8_of(*c) != bytes); + list.insert(0, color); + list.truncate(MAX_CUSTOM_COLORS); + true +} + +/// Seed/replace `list` from an already most-recent-first ORDERED source (a persisted history, or +/// another picker's current `custom()`), preserving that order. +/// +/// `push_custom` inserts one colour at the front, so feeding it a MRU-ordered sequence +/// front-to-back would reverse it (the first, newest element ends up pushed deepest). Processing +/// the source in REVERSE — oldest first — restores the intended order: each push moves the +/// correct next-newest colour back to the front. +fn push_all_custom(list: &mut Vec, colors: impl IntoIterator) { + let ordered: Vec = colors.into_iter().collect(); + for color in ordered.into_iter().rev() { + push_custom(list, color); + } +} + pub enum MoonColorPickerEvent { Change(Hsla), + /// A colour typed into the hex field and committed, newly added to (or moved to the front + /// of) `custom`. Callers that persist a reuse palette react to this; others may ignore it. + CustomAdded(Hsla), } pub struct MoonColorPickerState { value: Hsla, + /// Colours committed through the hex field, most-recent-first. Session-local unless a caller + /// seeds/persists it via `custom_colors`/`set_custom_colors` and `CustomAdded`. + custom: Vec, + /// Owned rather than transient `window.use_keyed_state`: reacting to its `PressEnter`/`Blur` + /// needs `cx.subscribe_in`, which only a `Context` can set up — never available inside + /// the stateless `MoonColorPicker::render`. + hex_input: Entity, + /// `default_value`/`set_value` cannot write `hex_input`'s displayed text directly — neither + /// has a `Window` in hand. They raise this flag instead; `MoonColorPicker::render` drains it + /// via `sync_hex_input` on every render, where a `Window` is available. + hex_dirty: bool, + _subscriptions: Vec, } impl EventEmitter for MoonColorPickerState {} impl MoonColorPickerState { - pub fn new(_window: &mut Window, cx: &mut Context) -> Self { + pub fn new(window: &mut Window, cx: &mut Context) -> Self { + let value = rgb(MoonPalette::active(cx).blue).into(); + let hex_input = cx.new(|cx| { + MoonInputState::new(window, cx) + .pattern(Regex::new(HEX_CHARSET).expect("HEX_CHARSET is a valid regex")) + }); + let _subscriptions = vec![cx.subscribe_in(&hex_input, window, Self::on_hex_event)]; Self { - value: rgb(MoonPalette::active(cx).blue).into(), + value, + custom: Vec::new(), + hex_input, + hex_dirty: true, + _subscriptions, } } pub fn default_value(mut self, value: Hsla) -> Self { self.value = value; + self.hex_dirty = true; + self + } + + /// Seed the reuse palette shown in the popover (most-recent-first), e.g. from persisted + /// config. Runs each entry through the same dedupe/cap as a live hex commit. + pub fn custom_colors(mut self, colors: impl IntoIterator) -> Self { + push_all_custom(&mut self.custom, colors); self } + /// Replace the reuse palette post-construction (e.g. after a sibling picker's hex commit was + /// persisted and fanned out to this one). Normalizes through the same dedupe/cap; emits no + /// event — this is the caller pushing state IN, not the widget producing a new colour. + pub fn set_custom_colors( + &mut self, + colors: impl IntoIterator, + cx: &mut Context, + ) { + self.custom.clear(); + push_all_custom(&mut self.custom, colors); + cx.notify(); + } + + /// The current reuse palette, most-recent-first. + pub fn custom(&self) -> &[Hsla] { + &self.custom + } + pub fn value(&self) -> Hsla { self.value } fn set_value(&mut self, value: Hsla, cx: &mut Context) { + self.hex_dirty = true; if self.value == value { return; } @@ -42,6 +185,50 @@ impl MoonColorPickerState { cx.emit(MoonColorPickerEvent::Change(value)); cx.notify(); } + + /// React to a committed hex-field edit (Enter or blur). The one validation: a value that + /// parses becomes the picker's value and joins the reuse palette; a value that does not + /// parse is rejected WITHOUT touching `self.value`/`self.custom` — only the field's own + /// displayed text reverts (via the `hex_dirty` flag) to whatever colour is still live. + fn on_hex_event( + &mut self, + input: &Entity, + event: &MoonInputEvent, + window: &mut Window, + cx: &mut Context, + ) { + if !matches!( + event, + MoonInputEvent::PressEnter { .. } | MoonInputEvent::Blur + ) { + return; + } + let text = input.read(cx).value(); + match parse_hex_rgb(&text) { + Some(color) => { + let added = push_custom(&mut self.custom, color); + self.set_value(color, cx); + if added { + cx.emit(MoonColorPickerEvent::CustomAdded(color)); + } + } + None => self.hex_dirty = true, + } + self.sync_hex_input(window, cx); + cx.notify(); + } + + /// Write the live value's hex text into the field when `hex_dirty` is set — after a + /// `default_value`/`set_value` call, or after a rejected commit reverts the draft. + fn sync_hex_input(&mut self, window: &mut Window, cx: &mut Context) { + if !self.hex_dirty { + return; + } + self.hex_dirty = false; + let text = hex_label(self.value); + self.hex_input + .update(cx, |state, cx| state.set_value(text, window, cx)); + } } #[derive(IntoElement)] @@ -76,21 +263,27 @@ impl MoonColorPicker { self.colors = colors.into_iter().collect(); self } +} - fn hex_label(color: Hsla) -> SharedString { - let rgba = color.to_rgb(); - let rgb = u32::from(rgba) >> 8; - SharedString::from(format!("#{rgb:06X}")) - } +/// Render a colour as the `#RRGGBB` text the hex field shows and [`parse_hex_rgb`] accepts back. +fn hex_label(color: Hsla) -> SharedString { + let [r, g, b] = rgb8_of(color); + SharedString::from(format!("#{r:02X}{g:02X}{b:02X}")) } impl RenderOnce for MoonColorPicker { - fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + // Drain a pending `default_value`/rejected-commit sync before reading anything else, so + // the trigger label and the hex field never show stale text for one frame. + self.state + .update(cx, |state, cx| state.sync_hex_input(window, cx)); + let p = MoonPalette::active(cx); let tokens = MoonTheme::active_tokens(cx); let value = self.state.read(cx).value(); + let custom = self.state.read(cx).custom().to_vec(); let state = self.state.clone(); - let colors = if self.colors.is_empty() { + let fixed = if self.colors.is_empty() { vec![ rgb(p.blue).into(), rgb(p.green).into(), @@ -106,6 +299,18 @@ impl RenderOnce for MoonColorPicker { } else { self.colors }; + // Custom (typed) colours lead, so a just-committed hex is immediately reachable without + // re-typing; a fixed swatch already covered by a custom one is skipped rather than shown + // twice. + let custom_bytes: Vec<[u8; 3]> = custom.iter().map(|c| rgb8_of(*c)).collect(); + let colors: Vec = custom + .into_iter() + .chain( + fixed + .into_iter() + .filter(|c| !custom_bytes.contains(&rgb8_of(*c))), + ) + .collect(); let trigger = div() .id(ElementId::from(SharedString::from(format!( @@ -138,7 +343,7 @@ impl RenderOnce for MoonColorPicker { .bg(value), ) .child( - MoonText::new(Self::hex_label(value)) + MoonText::new(hex_label(value)) .color(p.text_soft) .alpha(if self.disabled { 0.45 } else { 1.0 }) .font_size(10.0) @@ -156,7 +361,9 @@ impl RenderOnce for MoonColorPicker { )))) .grid() .grid_cols(5) - .gap(px(tokens.ui(6.0))); + .gap(px(tokens.ui(6.0))) + .max_h(px(tokens.ui(GRID_MAX_HEIGHT_UI))) + .overflow_y_scroll(); for (ix, color) in colors.into_iter().enumerate() { let state = state.clone(); @@ -194,11 +401,30 @@ impl RenderOnce for MoonColorPicker { ); } + let hex_input_state = self.state.read(cx).hex_input.clone(); + let hex_is_invalid = { + let text = hex_input_state.read(cx).value(); + !text.is_empty() && parse_hex_rgb(&text).is_none() + }; + let hex_row = div().w_full().child( + MoonInput::new(SharedString::from(format!("{}:hex", self.id))) + .state(&hex_input_state) + .small() + .mono(true) + .disabled(self.disabled) + .when(hex_is_invalid, |this| this.tone(MoonTone::Danger)), + ); + + let content = v_flex().gap(px(tokens.ui(6.0))).child(hex_row).child(grid); + MoonPopover::new(self.id) .trigger(trigger) - .content(grid) + .content(content) .width(156.0) .placement(MoonPopoverPlacement::BottomStart) .disabled(self.disabled) } } + +#[cfg(test)] +mod tests; diff --git a/crates/moon-ui-components/src/moon/color_picker/tests.rs b/crates/moon-ui-components/src/moon/color_picker/tests.rs new file mode 100644 index 0000000..8fd831a --- /dev/null +++ b/crates/moon-ui-components/src/moon/color_picker/tests.rs @@ -0,0 +1,119 @@ +//! Regression coverage for the Moon colour-picker hex field and reuse palette. + +use super::{MAX_CUSTOM_COLORS, hex_label, parse_hex_rgb, push_all_custom, push_custom, rgb8_of}; +use gpui::{Hsla, hsla, rgb}; + +/// Builds an opaque colour from its independently chosen six-digit RGB value. +fn color(value: u32) -> Hsla { + rgb(value).into() +} + +/// Reads a list in the user-visible hexadecimal form instead of comparing floating-point HSLA. +fn labels(colors: &[Hsla]) -> Vec { + colors + .iter() + .map(|color| hex_label(*color).to_string()) + .collect() +} + +/// Catches removing trimming or case-insensitive ASCII hex acceptance from `color_picker.rs:parse_hex_rgb`. +/// +/// Pasted lower-case or padded hex input would otherwise be rejected even though it names the +/// same orange swatch as the canonical readout. +#[test] +fn hex_parser_accepts_case_and_surrounding_whitespace() { + let expected = Some([0xFF, 0x80, 0x00]); + for text in ["#ff8000", "FF8000", " #Ff8000 "] { + assert_eq!(parse_hex_rgb(text).map(rgb8_of), expected, "{text:?}"); + } +} + +/// Catches changing `color_picker.rs:parse_hex_rgb` to accept eight digits as an RGB value. +/// +/// Invalid or alpha-bearing input would otherwise commit a colour that cannot round-trip through +/// the six-digit picker field. +#[test] +fn hex_parser_rejects_every_non_six_digit_input() { + for text in ["", "#FFF", "#GGGGGG", "#FFFFFFFF", "12345", "#1234567"] { + assert_eq!(parse_hex_rgb(text), None, "{text:?} must be rejected"); + } +} + +/// Catches replacing `color_picker.rs:push_custom` front insertion with appending. +/// +/// A newly committed colour must be the first reusable swatch rather than being hidden behind +/// older entries. +#[test] +fn new_custom_colour_becomes_the_front_entry() { + let mut list = vec![color(0x112233)]; + + assert!(push_custom(&mut list, color(0xA5A5A5))); + assert_eq!(labels(&list), ["#A5A5A5", "#112233"]); +} + +/// Catches removing the `already_front` early return from `color_picker.rs:push_custom`. +/// +/// Recommitting the current front colour would otherwise produce a redundant persisted custom- +/// colour event even though the visible list did not change. +#[test] +fn recommitting_the_front_custom_colour_is_a_no_op() { + let mut list = vec![color(0x808080), color(0x112233)]; + let before = labels(&list); + + assert!(!push_custom(&mut list, color(0x808080))); + assert_eq!(labels(&list), before); +} + +/// Catches removing RGB-byte de-duplication from `color_picker.rs:push_custom`. +/// +/// Reusing an older swatch would otherwise show it twice instead of moving that same colour to +/// the first reusable position. +#[test] +fn an_existing_custom_colour_moves_to_front_without_a_duplicate() { + let mut list = vec![color(0x112233), color(0xA5A5A5)]; + + assert!(push_custom(&mut list, color(0xA5A5A5))); + assert_eq!(labels(&list), ["#A5A5A5", "#112233"]); +} + +/// Catches removing `color_picker.rs:push_custom`'s `MAX_CUSTOM_COLORS` truncation. +/// +/// An unbounded history would keep growing the picker and push its reusable-swatch grid beyond +/// its intended fixed memory and visual budget. +#[test] +fn custom_colour_history_is_capped_with_the_newest_entries_first() { + let mut list = Vec::new(); + for value in 1..=(MAX_CUSTOM_COLORS as u32 + 3) { + assert!(push_custom(&mut list, color(value))); + } + + assert_eq!(list.len(), MAX_CUSTOM_COLORS); + assert_eq!(labels(&list).first(), Some(&"#000017".to_string())); + assert_eq!(labels(&list).last(), Some(&"#000004".to_string())); +} + +/// Catches removing the reverse iteration in `color_picker.rs:push_all_custom`. +/// +/// A persisted or shared most-recent-first reuse palette would otherwise show its custom +/// swatches oldest-first after it is seeded or replaced. +#[test] +fn seeded_custom_colours_preserve_most_recent_first_order() { + let most_recent_first = vec![color(0xA5A5A5), color(0x8A2BE2), color(0x3A6EA5)]; + let mut list = Vec::new(); + + push_all_custom(&mut list, most_recent_first.iter().copied()); + + assert_eq!(list, most_recent_first); +} + +/// Catches reverting `color_picker.rs:rgb8_of` to truncate `c.r * 255.0` with `as u8` instead +/// of rounding each channel. +/// +/// The picker palette's 50% gray swatch would otherwise display and round-trip as `#7F7F7F` +/// instead of the nearest-byte `#808080`. +#[test] +fn hsl_middle_gray_rounds_to_the_picker_hex_byte() { + let picker_middle_gray = hsla(0.0, 0.0, 0.5, 1.0); + + assert_eq!(rgb8_of(picker_middle_gray), [0x80, 0x80, 0x80]); +} diff --git a/crates/moon-ui-gallery/src/gallery.rs b/crates/moon-ui-gallery/src/gallery.rs index fec2747..57ce557 100644 --- a/crates/moon-ui-gallery/src/gallery.rs +++ b/crates/moon-ui-gallery/src/gallery.rs @@ -161,8 +161,11 @@ impl Gallery { .step(1.0) .default_value((18.0, 74.0)) }); - let color_state = - cx.new(|cx| MoonColorPickerState::new(window, cx).default_value(rgb(0xFFB347).into())); + let color_state = cx.new(|cx| { + MoonColorPickerState::new(window, cx) + .default_value(rgb(0xFFB347).into()) + .custom_colors([rgb(0x3A6EA5).into(), rgb(0x8A2BE2).into()]) + }); let data_table_state = cx.new(|_| MoonDataTableState::new()); let tooltip_view = cx.new(|_| MoonTooltipView::new("MoonTooltipView entity").max_width(220.0)); diff --git a/docs/component-api-baseline.json b/docs/component-api-baseline.json index 9b121f2..95e491a 100644 --- a/docs/component-api-baseline.json +++ b/docs/component-api-baseline.json @@ -609,6 +609,14 @@ "file": "crates/moon-ui-components/src/moon/color_picker.rs", "signature": "pub fn colors(mut self, colors: impl IntoIterator) -> Self" }, + { + "file": "crates/moon-ui-components/src/moon/color_picker.rs", + "signature": "pub fn custom(&self) -> &[Hsla]" + }, + { + "file": "crates/moon-ui-components/src/moon/color_picker.rs", + "signature": "pub fn custom_colors(mut self, colors: impl IntoIterator) -> Self" + }, { "file": "crates/moon-ui-components/src/moon/color_picker.rs", "signature": "pub fn default_value(mut self, value: Hsla) -> Self" @@ -623,11 +631,15 @@ }, { "file": "crates/moon-ui-components/src/moon/color_picker.rs", - "signature": "pub fn new(_window: &mut Window, cx: &mut Context) -> Self" + "signature": "pub fn new(state: &Entity) -> Self" }, { "file": "crates/moon-ui-components/src/moon/color_picker.rs", - "signature": "pub fn new(state: &Entity) -> Self" + "signature": "pub fn new(window: &mut Window, cx: &mut Context) -> Self" + }, + { + "file": "crates/moon-ui-components/src/moon/color_picker.rs", + "signature": "pub fn set_custom_colors( &mut self, colors: impl IntoIterator, cx: &mut Context, )" }, { "file": "crates/moon-ui-components/src/moon/color_picker.rs",