Select Color: small cleanups found in review

- Keep the modal keyboard comment with the code it describes and match
  shortcut keys without an extra closure.
- Drop unused ColorEditor::mode, EyedropperMsg::Cancel and a duplicate
  rgb helper; re-export nearest_aci instead of wrapping it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013rGmDUNr1hUf4zo1X3dak8

src/app/update/mod.rs

@@ -370,19 +370,11 @@ impl OpenCADStudio {
             let tab = &self.tabs[self.active_tab];
             (started, tab.id, tab.scene.geometry_epoch)
         });
-        // A modal dialog must capture the keyboard the same way it already
-        // captures the mouse. Otherwise keystrokes from the global key
-        // subscription leak past the modal into the command line and fire as
-        // commands once the dialog closes. While a modal is open, Escape
-        // closes it and every other keystroke-derived message is swallowed;
-        // the modal's own text fields keep working because they emit their own
-        // (non-blocked) messages. (#126)
         // The Select Color panel can open on top of another dialog, so it
         // takes the keyboard first: Esc cancels, Enter confirms, the arrows
         // walk the index palette, and every other keystroke is swallowed.
         if self.color_picker.is_some() {
             use crate::ui::color_select::ColorPickerMsg;
-            let last_key = |key: &str| key.rsplit('+').next().map(str::to_owned);
             match &msg {
                 Message::CommandEscape => {
                     return self.update(Message::ColorPicker(ColorPickerMsg::Cancel));
@@ -390,7 +382,7 @@ impl OpenCADStudio {
                 Message::CommandFinalize => {
                     return self.update(Message::ColorPicker(ColorPickerMsg::Ok));
                 }
-                Message::ShortcutPressed(key) => match last_key(key).as_deref() {
+                Message::ShortcutPressed(key) => match key.rsplit('+').next() {
                     Some("ESCAPE") => {
                         return self.update(Message::ColorPicker(ColorPickerMsg::Cancel));
                     }
@@ -413,6 +405,13 @@ impl OpenCADStudio {
                 return Task::none();
             }
         }
+        // A modal dialog must capture the keyboard the same way it already
+        // captures the mouse. Otherwise keystrokes from the global key
+        // subscription leak past the modal into the command line and fire as
+        // commands once the dialog closes. While a modal is open, Escape
+        // closes it and every other keystroke-derived message is swallowed;
+        // the modal's own text fields keep working because they emit their own
+        // (non-blocked) messages. (#126)
         if self.active_modal.is_some() {
             if matches!(msg, Message::CommandEscape)
                 || matches!(&msg, Message::ShortcutPressed(key) if key.rsplit('+').next() == Some("ESCAPE"))

src/ui/color_select/editor.rs

@@ -75,10 +75,6 @@ impl ColorEditor {
         self.rgb
     }
 
-    pub fn mode(&self) -> EditorMode {
-        self.mode
-    }
-
     pub fn set_rgb(&mut self, (r, g, b): (u8, u8, u8)) {
         let next = math::rgb_to_hsb(r, g, b);
         // A gray has no hue and black no saturation; keep the previous ones

src/ui/color_select/eyedropper.rs

@@ -5,6 +5,7 @@
 //! the window; a magnifier beside the cursor shows the pixels around it from
 //! that screenshot, and a click picks the centre pixel.
 
+use super::editor::rgb_color;
 use super::{math, ColorPickerMsg};
 use crate::app::Message;
 use iced::widget::{column, container, mouse_area, pin, row, stack, text, Space};
@@ -23,7 +24,6 @@ pub enum EyedropperMsg {
     Move(Point),
     Shot(Screenshot),
     Pick,
-    Cancel,
 }
 
 #[derive(Default)]
@@ -68,7 +68,7 @@ impl Eyedropper {
         for dy in -half..=half {
             let mut line = row![];
             for dx in -half..=half {
-                let fill = sample(shot, cursor, dx, dy).map_or(Color::TRANSPARENT, rgb);
+                let fill = sample(shot, cursor, dx, dy).map_or(Color::TRANSPARENT, rgb_color);
                 let centre = dx == 0 && dy == 0;
                 line = line.push(container(Space::new()).width(CELL).height(CELL).style(
                     move |_theme: &Theme| container::Style {
@@ -133,10 +133,6 @@ impl Eyedropper {
     }
 }
 
-fn rgb((r, g, b): (u8, u8, u8)) -> Color {
-    Color::from_rgb8(r, g, b)
-}
-
 /// The screenshot pixel `dx`, `dy` physical pixels away from a logical point.
 fn sample(shot: &Screenshot, at: Point, dx: i32, dy: i32) -> Option<(u8, u8, u8)> {
     let x = (at.x * shot.scale_factor) as i64 + i64::from(dx);

src/ui/color_select/math.rs

@@ -1,8 +1,8 @@
-//! Colour maths and text parsing for the Select Color panel.
+//! Colour maths and formatting for the Select Color panel.
 //!
 //! Everything here is pure so it can be unit tested without a UI: HSB/RGB
-//! conversion, hex formatting, nearest-index lookup and the parser behind the
-//! panel's Color field.
+//! conversion, hex formatting and parsing, nearest-index lookup and the
+//! short names the panel shows.
 
 use codec::types::Color as AcadColor;
 

src/ui/color_select/mod.rs

@@ -9,7 +9,8 @@ pub mod math;
 pub mod picker;
 pub mod widgets;
 
-pub use picker::{ColorPicker, ColorPickerMsg, ColorPickerTab, Outcome};
+pub use math::nearest_aci;
+pub use picker::{ColorPicker, ColorPickerMsg, Outcome};
 
 use crate::app::Message;
 use crate::t;
@@ -42,11 +43,6 @@ pub fn color_to_aci_string(c: AcadColor) -> String {
     }
 }
 
-/// Return the closest CAD Color Index for an RGB colour.
-pub fn nearest_aci(r: u8, g: u8, b: u8) -> u8 {
-    math::nearest_aci(r, g, b)
-}
-
 /// Decode an ACI integer string back into an `AcadColor`.
 pub fn aci_string_to_color(s: &str) -> AcadColor {
     match s.trim().parse::<i16>().unwrap_or(256) {

src/ui/color_select/picker.rs

@@ -2,7 +2,7 @@
 //!
 //! The panel edits a pending colour and only reports it back on OK, so every
 //! caller keeps applying colours through the same `ColorWindowPick` path.
-//! Opened for an interface colour (an Options background) it shows only the
+//! Opened for an interface colour (any colour in Options) it shows only the
 //! colour editor: no tabs, no index colours.
 
 use super::editor::{rgb_color, ColorEditor, EditorMsg};
@@ -91,7 +91,7 @@ impl ColorPicker {
         self.tab
     }
 
-    /// Interface colours (Options backgrounds) only get the colour editor.
+    /// Interface colours (Options) only get the colour editor.
     fn editor_only(&self) -> bool {
         matches!(self.target, ColorPickTarget::Interface(_))
     }
@@ -122,9 +122,8 @@ impl ColorPicker {
                         self.set_pending(AcadColor::Rgb { r, g, b });
                     }
                 }
-                ColorPickerMsg::Eyedropper(EyedropperMsg::Cancel) | ColorPickerMsg::Cancel => {
-                    self.eyedropper = None;
-                }
+                // Esc leaves the eyedropper but keeps the panel open.
+                ColorPickerMsg::Cancel => self.eyedropper = None,
                 _ => {}
             }
             return None;