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 <[email protected]> Claude-Session: https://claude.ai/code/session_013rGmDUNr1hUf4zo1X3dak8
2026-10-14 19:38:00 +00:00
1 parent 345f836e91
commit c71a874fe0
@@ -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);
    }
@@ -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(
@@ -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<ColorBook>, 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<Item = ColorBook> {
@@ -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<u8>| {
            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))
@@ -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<Color> {
+    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<Color> {
-        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
@@ -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,