From c71a874fe0d21f2ddf627002552f683302c2923d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 19:38:00 +0000 Subject: [PATCH] Select Color: reuse helpers instead of repeated code - book::add_or_replace adds a colour book or replaces one of the same name, for both reading the folders and loading a file. - style.rs: the Plot Style and View Background colour targets use math::color_rgb (and hex_string) instead of their own conversions. - Ribbon: one layer_color lookup for Ribbon::active_layer_color and the ribbon's ByLayer swatch. - Picker: one swatch() for the Index and Document swatches, fill_of() for "colour or gray" and divider() for the 1 px separators. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013rGmDUNr1hUf4zo1X3dak8 --- src/app/update/mod.rs | 20 ++---- src/app/update/style.rs | 24 +++----- src/ui/color_select/book.rs | 14 +++++ src/ui/color_select/picker.rs | 113 ++++++++++++++++++++-------------- src/ui/ribbon/mod.rs | 13 ++-- src/ui/ribbon/widgets.rs | 6 +- 6 files changed, 104 insertions(+), 86 deletions(-) diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index eee1f3f01..cd8f968d0 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -12004,17 +12004,10 @@ impl OpenCADStudio { } Message::ColorBookLoaded(Ok(book)) => { // A book loaded again replaces the earlier copy. - let books = std::sync::Arc::make_mut(&mut self.color_books); - let index = match books.iter().position(|b| b.name == book.name) { - Some(index) => { - books[index] = book; - index - } - None => { - books.push(book); - books.len() - 1 - } - }; + let index = crate::ui::color_select::book::add_or_replace( + std::sync::Arc::make_mut(&mut self.color_books), + book, + ); if let Some(picker) = self.color_picker.as_mut() { picker.show_book(std::sync::Arc::clone(&self.color_books), index); } @@ -12162,10 +12155,7 @@ impl OpenCADStudio { .filter_map(|path| std::fs::read_to_string(path).ok()) .filter_map(|text| book::parse(&text).ok()) { - match books.iter_mut().find(|b| b.name == loaded.name) { - Some(same) => *same = loaded, - None => books.push(loaded), - } + book::add_or_replace(&mut books, loaded); } self.color_books = std::sync::Arc::new(books); } diff --git a/src/app/update/style.rs b/src/app/update/style.rs index b1cd0da91..b3daf77d8 100644 --- a/src/app/update/style.rs +++ b/src/app/update/style.rs @@ -1171,14 +1171,8 @@ impl OpenCADStudio { ) { self.color_picker = None; - let rgb = match color { - codec::types::Color::Rgb { r, g, b } => Some((r, g, b)), - codec::types::Color::Index(index) => codec::types::aci_table::aci_to_rgb(index), - _ => None, - }; - - if let Some((r, g, b)) = rgb { - self.ps_color_buf = format!("#{r:02X}{g:02X}{b:02X}"); + if let Some((r, g, b)) = crate::ui::color_select::math::color_rgb(color) { + self.ps_color_buf = crate::ui::color_select::math::hex_string(r, g, b); } return self.on_plot_style_panel_apply(); @@ -1191,15 +1185,11 @@ impl OpenCADStudio { ))) } Some(crate::app::ColorPickTarget::ViewBackground(slot)) => { - let rgb = match color { - codec::types::Color::Rgb { r, g, b } => Some([r, g, b]), - codec::types::Color::Index(index) => { - codec::types::aci_table::aci_to_rgb(index).map(|(r, g, b)| [r, g, b]) - } - _ => None, - }; - rgb.map(|rgb| { - Message::PdfDialog(crate::ui::window::pdf_dialogs::PdfDialogMsg::BgColor(slot, rgb)) + crate::ui::color_select::math::color_rgb(color).map(|(r, g, b)| { + Message::PdfDialog(crate::ui::window::pdf_dialogs::PdfDialogMsg::BgColor( + slot, + [r, g, b], + )) }) } Some(crate::app::ColorPickTarget::GraphicAttributesLine) => Some( diff --git a/src/ui/color_select/book.rs b/src/ui/color_select/book.rs index 6a9a74f2f..cd2009ca1 100644 --- a/src/ui/color_select/book.rs +++ b/src/ui/color_select/book.rs @@ -57,6 +57,20 @@ impl ColorBook { } } +/// Add `book`, replacing one of the same name; returns where it now is. +pub fn add_or_replace(books: &mut Vec, book: ColorBook) -> usize { + match books.iter().position(|b| b.name == book.name) { + Some(index) => { + books[index] = book; + index + } + None => { + books.push(book); + books.len() - 1 + } + } +} + /// The colour book that ships with the program; its licence is in the /// file's header comment. pub fn builtin() -> impl Iterator { diff --git a/src/ui/color_select/picker.rs b/src/ui/color_select/picker.rs index 40f003ef4..8c0333820 100644 --- a/src/ui/color_select/picker.rs +++ b/src/ui/color_select/picker.rs @@ -434,28 +434,22 @@ impl ColorPicker { (None, None) => math::describe(self.pending), }; let error = self.status.is_some(); - body = body - .push( + body = body.push(divider()).push( + row![ container(Space::new()) - .width(Length::Fill) - .height(1) - .style(|theme: &Theme| filled(theme.palette().background.weak.color)), - ) - .push( - row![ - container(Space::new()) - .width(12) - .height(12) - .style(move |_theme: &Theme| swatch_style(fill)), - text(info).size(11).font(iced::Font::MONOSPACE).style( - move |theme: &Theme| text::Style { - color: error.then(|| theme.palette().danger.base.color), - } - ), - ] - .spacing(8) - .align_y(iced::Center), - ); + .width(12) + .height(12) + .style(move |_theme: &Theme| swatch_style(fill)), + text(info) + .size(11) + .font(iced::Font::MONOSPACE) + .style(move |theme: &Theme| text::Style { + color: error.then(|| theme.palette().danger.base.color), + }), + ] + .spacing(8) + .align_y(iced::Center), + ); } let body = container(body) .padding(12) @@ -590,14 +584,14 @@ impl ColorPicker { let section = |label: String| text(label).size(11); let swatches = |range: std::ops::RangeInclusive| { range.fold(row![].spacing(5), |line, index| { - line.push(self.swatch_button(index, 30.0, 26.0)) + line.push(self.index_swatch(index, 30.0, 26.0)) }) }; // ACI 10–249 come in groups of ten: 24 hue families across, their ten // shades down. let grid = (0u8..10).fold(column![].spacing(2), |grid, shade| { grid.push((1u8..=24).fold(row![].spacing(2), |line, family| { - line.push(self.swatch_button(family * 10 + shade, 13.0, 13.0)) + line.push(self.index_swatch(family * 10 + shade, 13.0, 13.0)) })) }); column![ @@ -617,14 +611,16 @@ impl ColorPicker { .into() } - fn swatch_button(&self, index: u8, width: f32, height: f32) -> Element<'_, Message> { - let fill = rgb_color(math::aci_rgb(index).unwrap_or((128, 128, 128))); - let selected = self.pending == AcadColor::Index(index); - button(Space::new().width(width).height(height)) - .on_press(picker_msg(ColorPickerMsg::Pick(AcadColor::Index(index)))) - .padding(0) - .style(move |theme: &Theme, status| swatch_button_style(theme, status, fill, selected)) - .into() + /// One index colour of the Index tab. + fn index_swatch(&self, index: u8, width: f32, height: f32) -> Element<'_, Message> { + let color = AcadColor::Index(index); + swatch( + fill_of(color), + self.pending == color, + ColorPickerMsg::Pick(color), + Length::Fixed(width), + Length::Fixed(height), + ) } fn document_tab(&self) -> Element<'_, Message> { @@ -675,8 +671,7 @@ impl ColorPicker { shown .iter() .fold(column![].spacing(2), |list, &(i, entry)| { - let fill = math::color_rgb(entry.color) - .map_or(Color::from_rgb8(128, 128, 128), rgb_color); + let fill = fill_of(entry.color); let selected = self.pending == entry.color && self.pending_name == entry.name; list.push( color_bar( @@ -747,15 +742,13 @@ impl ColorPicker { } fn doc_swatch<'a>(&self, i: usize, entry: &'a DocColor) -> Element<'a, Message> { - let fill = math::color_rgb(entry.color).map_or(Color::from_rgb8(128, 128, 128), rgb_color); - let selected = self.pending == entry.color && self.pending_name == entry.name; - button(Space::new()) - .on_press(picker_msg(ColorPickerMsg::PickDocument(i))) - .padding(0) - .width(Length::Fill) - .height(30) - .style(move |theme: &Theme, status| swatch_button_style(theme, status, fill, selected)) - .into() + swatch( + fill_of(entry.color), + self.pending == entry.color && self.pending_name == entry.name, + ColorPickerMsg::PickDocument(i), + Length::Fill, + Length::Fixed(30.0), + ) } fn books_tab(&self) -> Element<'_, Message> { @@ -915,10 +908,7 @@ impl ColorPicker { channel(t!("Red"), chosen.map(|c| c.rgb.0)), channel(t!("Green"), chosen.map(|c| c.rgb.1)), channel(t!("Blue"), chosen.map(|c| c.rgb.2)), - container(Space::new()) - .width(Length::Fill) - .height(1) - .style(|theme: &Theme| filled(theme.palette().background.weak.color)), + divider(), text(format!("{} / {}", self.book_page + 1, book.page_count())) .size(11) .style(muted_text), @@ -1185,6 +1175,37 @@ fn tab_icon<'a>(handle: svg::Handle) -> svg::Svg<'a> { }) } +/// A clickable colour swatch, ringed when selected or hovered. +fn swatch<'a>( + fill: Color, + selected: bool, + on_press: ColorPickerMsg, + width: Length, + height: Length, +) -> Element<'a, Message> { + button(Space::new()) + .on_press(picker_msg(on_press)) + .padding(0) + .width(width) + .height(height) + .style(move |theme: &Theme, status| swatch_button_style(theme, status, fill, selected)) + .into() +} + +/// How a concrete colour looks; gray for one without RGB. +fn fill_of(color: AcadColor) -> Color { + math::color_rgb(color).map_or(Color::from_rgb8(128, 128, 128), rgb_color) +} + +/// A 1 px separator line across the panel. +fn divider<'a>() -> Element<'a, Message> { + container(Space::new()) + .width(Length::Fill) + .height(1) + .style(|theme: &Theme| filled(theme.palette().background.weak.color)) + .into() +} + /// A short gray note in the middle of the free space. fn empty_note<'a>(note: String) -> Element<'a, Message> { container(text(note).size(12).style(muted_text)) diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index 2172218cb..7504828d1 100644 --- a/src/ui/ribbon/mod.rs +++ b/src/ui/ribbon/mod.rs @@ -148,6 +148,14 @@ pub struct LayerInfo { pub vp_frozen: Option<(usize, bool)>, } +/// The colour of layer `name` in the ribbon's layer list. +pub(super) fn layer_color(layers: &[LayerInfo], name: &str) -> Option { + layers + .iter() + .find(|layer| layer.name == name) + .map(|layer| layer.color) +} + /// Full-screen backdrop for an open ribbon dropdown: closes the dropdown when /// the user clicks outside the panel. (Cursor motion leaking to the viewport /// beneath is blocked in `on_viewport_move`, not here — iced 0.14's mouse_area @@ -188,10 +196,7 @@ fn position_ribbon_dropdown<'a>( impl Ribbon { /// The current layer's colour: what ByLayer looks like for new objects. pub fn active_layer_color(&self) -> Option { - self.layer_infos - .iter() - .find(|layer| layer.name == self.active_layer) - .map(|layer| layer.color) + layer_color(&self.layer_infos, &self.active_layer) } /// The colour-book identity of the active colour, while the active colour diff --git a/src/ui/ribbon/widgets.rs b/src/ui/ribbon/widgets.rs index 41b81ac71..a141408a7 100644 --- a/src/ui/ribbon/widgets.rs +++ b/src/ui/ribbon/widgets.rs @@ -1174,11 +1174,9 @@ pub(super) fn render_large<'a>(item: &RibbonItem, ctx: &RenderCtx<'_>) -> Elemen }; // ByLayer shows the current layer's colour. - let color_swatch = layer_infos - .iter() - .find(|layer| layer.name == active_layer) + let color_swatch = super::layer_color(layer_infos, active_layer) .filter(|_| active_color == AcadColor::ByLayer) - .map_or_else(|| acad_color_display(active_color).0, |layer| layer.color); + .unwrap_or_else(|| acad_color_display(active_color).0); let color_row = prop_row( crate::ui::color_select::color_display_name(active_color), PROP_COLOR_ID,