From 9f68034a6026ab2e737be29f5bad97be408de59d Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Wed, 12 Aug 2026 23:23:50 +0100 Subject: [PATCH] feat(props): select whole value when a property field is clicked Clicking an editable property value field now selects its entire value instead of only placing the caret, so typing immediately replaces it. The click is observed through a global left-press subscription (PropPointerPressed); a focus sweep reports which widget took focus and PropSyncActive maps it back to the active property row, then issues a select-all operation only when focus landed on a different field. The active row stays highlighted while its text input holds keyboard focus and clears when focus moves elsewhere. --- src/app/mod.rs | 11 ++ src/app/properties.rs | 10 ++ src/app/update/command.rs | 2 + src/app/update/mod.rs | 67 +++++++++- src/app/view/mod.rs | 3 + src/ui/properties.rs | 261 ++++++++++++++++++++++++++++++++++++-- 6 files changed, 336 insertions(+), 18 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index a4d83243f..ade3398b0 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -2362,6 +2362,17 @@ pub enum Message { PropAttrInput { tag: String, value: String }, /// User committed a block-attribute value edit (Enter pressed). PropAttrCommit(String), + /// Reports the currently keyboard-focused widget (if any) after a + /// [`sync_active_field_task`](crate::ui::properties::sync_active_field_task) + /// sweep, so the update handler can keep the active-row marker reconciled + /// against real focus instead of pointer hover. + PropSyncActive(Option), + /// A left mouse button was pressed anywhere. The clicked widget has already + /// been given focus by the time this arrives (the widget tree processes the + /// event before the runtime broadcasts it to subscriptions), so a focus + /// sweep — which resolves as `PropSyncActive` — reveals whether a property + /// value field was clicked and can select its whole value. + PropPointerPressed, /// Toggle the inline color picker dropdown open/closed. PropColorPickerToggle, /// Toggle the MTEXT background-colour picker dropdown open/closed. diff --git a/src/app/properties.rs b/src/app/properties.rs index b233c5614..5ad86cd88 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -42,6 +42,7 @@ impl OpenCADStudio { // closes, matching the deselect / reselect / click-away expectation. let color_palette_open = self.tabs[i].properties.color_palette_open; let edit_buf = std::mem::take(&mut self.tabs[i].properties.edit_buf); + let active_field = std::mem::take(&mut self.tabs[i].properties.active_field); // Expanded coordinate groups persist across rebuilds AND selection // changes — it's a per-user view preference, not per-entity state. let expanded_groups = std::mem::take(&mut self.tabs[i].properties.expanded_groups); @@ -1441,6 +1442,14 @@ impl OpenCADStudio { } else { Default::default() }; + // The active-row highlight only survives a rebuild for the same + // selection (like the edit buffer); a selection change clears it so + // an old row isn't marked active against new content. + panel.active_field = if prev_handles == new_handles { + active_field + } else { + None + }; panel.expanded_groups = expanded_groups; panel.source_handles = new_handles; panel.prop_vertex = prop_vertex; @@ -1458,6 +1467,7 @@ impl OpenCADStudio { if locked_only { make_sections_read_only(&mut panel.sections); panel.edit_buf.clear(); + panel.active_field = None; panel.color_picker_open = false; panel.color_palette_open = false; panel.bg_color_picker_open = false; diff --git a/src/app/update/command.rs b/src/app/update/command.rs index 72eec976b..2e4a07573 100644 --- a/src/app/update/command.rs +++ b/src/app/update/command.rs @@ -2120,6 +2120,7 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { pub(super) fn on_prop_geom_commit(&mut self, field: &'static str) -> Task { let i = self.active_tab; + self.tabs[i].properties.active_field = None; let handles = self.property_target_handles(i); if !handles.is_empty() { if let Some(raw_val) = self.tabs[i].properties.edit_buf.remove(field) { @@ -2348,6 +2349,7 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { /// it is not run through the expression evaluator like numeric fields). pub(super) fn on_prop_attr_commit(&mut self, tag: String) -> Task { let i = self.active_tab; + self.tabs[i].properties.active_field = None; let key = crate::ui::properties::attr_edit_key(&tag); let handles = self.property_target_handles(i); if handles.is_empty() { diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index eaec47f33..a21d463d9 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -217,6 +217,20 @@ impl OpenCADStudio { self.reset_modal_geometry(); } + /// Fire the focus-sweep once when a property field is active, so a + /// click-away that moved focus off the field (viewport, toolbar, command + /// line) clears the active-row highlight. Idle (`Task::none()`) unless a + /// field is active, and the sweep itself keeps the marker whenever the + /// active field still holds focus — re-firing it on unrelated messages is + /// cheap and safe. + fn sync_active_field_if_any(&self) -> Task { + if self.tabs[self.active_tab].properties.active_field.is_some() { + crate::ui::properties::sync_active_field_task() + } else { + Task::none() + } + } + pub fn update(&mut self, msg: Message) -> Task { let perf_started = crate::perf::enabled().then(Instant::now); let perf_label = perf_message_label(&msg); @@ -1158,7 +1172,8 @@ impl OpenCADStudio { } Message::RibbonToolClick { tool_id, event } => { - self.on_ribbon_tool_click(tool_id, event) + let sweep = self.sync_active_field_if_any(); + Task::batch(vec![sweep, self.on_ribbon_tool_click(tool_id, event)]) } Message::PluginFileDialogResult { command, path } => { if let Some(path) = path { @@ -1401,14 +1416,15 @@ impl OpenCADStudio { // the submit path, which tokenises multi-token lines — so a typed // token, a pasted `LINE 0,0 10,10`, or API-fed text all run their // spaces as step separators. A leading `>` keeps spaces literal. + let sweep = self.sync_active_field_if_any(); if !self.command_line.literal_spaces && !s.starts_with('>') && s.contains(' ') { self.command_line.input = s; - return self.update(Message::CommandSubmit); + return Task::batch(vec![sweep, self.update(Message::CommandSubmit)]); } self.command_line.input = s; self.command_line.autocomplete_cursor = None; self.command_line.cancel_history_navigation(); - Task::none() + Task::batch(vec![sweep, Task::none()]) } Message::CommandAppendChar(s) => self.on_command_append_char(s), @@ -2591,10 +2607,13 @@ impl OpenCADStudio { Task::none() } Message::PanePress(idx) => { + // A click-away into the drawing area re-syncs the active-row + // highlight against real focus before the pick runs. + let sweep = self.sync_active_field_if_any(); // A fresh press ends any stale (un-dropped) pane move. self.pane_move_from = None; self.focus_model_pane(idx); - self.on_viewport_left_press() + Task::batch(vec![sweep, self.on_viewport_left_press()]) } Message::PaneRelease(idx) => { // Finishing a pane-move drag: swap the source pane with the one @@ -2629,7 +2648,10 @@ impl OpenCADStudio { self.update(Message::ViewportScroll(d)) } - Message::ViewportLeftPress => self.on_viewport_left_press(), + Message::ViewportLeftPress => { + let sweep = self.sync_active_field_if_any(); + Task::batch(vec![sweep, self.on_viewport_left_press()]) + } Message::ViewportLeftRelease => self.on_viewport_left_release(), @@ -4610,6 +4632,41 @@ impl OpenCADStudio { Message::PropAttrCommit(tag) => self.on_prop_attr_commit(tag), + Message::PropPointerPressed => { + if self.tabs[self.active_tab].is_start || self.clean_screen { + return Task::none(); + } + crate::ui::properties::sync_active_field_task() + } + + Message::PropSyncActive(focused) => { + let panel = &mut self.tabs[self.active_tab].properties; + if let Some(id) = focused.as_ref() { + if let Some(key) = crate::ui::properties::prop_field_key_for_id( + &panel.sections, + id, + ) { + let changed = panel.active_field.as_deref() != Some(key.as_str()); + panel.active_field = Some(key); + // Only select the whole value when focus landed on a + // field that wasn't already the active one; re-focusing + // the same field (or sweeping after a click elsewhere + // left its focus in place) must keep caret placement. + if changed { + return iced::widget::operation::select_all(id.clone()); + } + return Task::none(); + } + } + if !crate::ui::properties::active_key_focused( + panel.active_field.as_deref(), + focused.as_ref(), + ) { + panel.active_field = None; + } + Task::none() + } + Message::PropColorPickerToggle => { let i = self.active_tab; self.tabs[i].properties.color_picker_open = diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index fe100032b..7a5929f8c 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -2116,6 +2116,9 @@ impl OpenCADStudio { event::listen_with(|ev, status, win_id| { use iced::event::Status; match ev { + iced::Event::Mouse(iced::mouse::Event::ButtonPressed( + iced::mouse::Button::Left, + )) => Some(Message::PropPointerPressed), iced::Event::Window(window::Event::CloseRequested) => { Some(Message::WindowCloseRequested(win_id)) } diff --git a/src/ui/properties.rs b/src/ui/properties.rs index 77bc5f99d..7d2852c9b 100644 --- a/src/ui/properties.rs +++ b/src/ui/properties.rs @@ -249,6 +249,106 @@ pub fn attr_edit_key(tag: &str) -> String { format!("\x01attr\x01{tag}") } +/// Text-input widget id for a geometry/common editable value field. The same +/// id (built from the `&'static str` key) is used by the row renderer to tag +/// the input and by the update handler to focus / select it on click. +pub fn prop_geom_field_id(field: &str) -> iced::widget::Id { + iced::widget::Id::from(format!("props-geom-field-{field}")) +} + +/// Text-input widget id for a block attribute value field, keyed by the tag. +pub fn prop_attr_field_id(tag: &str) -> iced::widget::Id { + iced::widget::Id::from(format!("props-attr-field-{tag}")) +} + +/// Returns the text-input widget id that backs an active-field key (the shape +/// used by the row renderer and the `PropSyncActive` handler). Geometry keys map +/// straight onto their field id; attribute keys carry the `\x01attr\x01…` +/// prefix (see [`attr_edit_key`]) so they map onto the tag's field id. +fn active_key_id(key: &str) -> iced::widget::Id { + if let Some(tag) = key.strip_prefix("\x01attr\x01") { + prop_attr_field_id(tag) + } else { + prop_geom_field_id(key) + } +} + +/// Keeps the active-row highlight keyed to real keyboard focus: returns true +/// only while the currently-focused widget IS the text input of the active +/// field. Focus on any other widget (another property field, a non-property +/// widget, or nothing at all) clears the marker. Setting the key for a newly +/// focused property field is handled by the `PropSyncActive` handler before +/// this runs. +pub fn active_key_focused(active_key: Option<&str>, focused: Option<&iced::widget::Id>) -> bool { + match (active_key, focused) { + (Some(key), Some(focused_id)) => focused_id == &active_key_id(key), + _ => false, + } +} + +/// Maps a focused text-input widget id back to the active-field key for that +/// property row, if the id belongs to an editable value field currently shown +/// in the panel. Geometry rows key on the row's `&'static str` field name; +/// attribute rows key on the sentinel-prefixed tag key (see [`attr_edit_key`]). +/// The block-Name caret-dropdown (`EditChoice`) is editable but deliberately +/// carries no id and is excluded here, mirroring the renderer. +pub fn prop_field_key_for_id( + sections: &[PropSection], + id: &iced::widget::Id, +) -> Option { + for section in sections { + for prop in §ion.props { + match &prop.value { + PropValue::EditText(_) => { + if &prop_geom_field_id(prop.field) == id { + return Some(prop.field.to_string()); + } + } + PropValue::AttrText { tag, .. } => { + if &prop_attr_field_id(tag) == id { + return Some(attr_edit_key(tag)); + } + } + _ => {} + } + } + } + None +} + +/// Returns a task that sweeps the widget tree for the currently-focused widget +/// and reports its `Id` via [`Message::PropSyncActive`], which the update +/// handler uses to reconcile the active-row marker against real focus. Mirrors +/// the `WebFieldCopy` operation idiom: `focusable` fires for every focusable +/// widget, and we record the one that `is_focused()`. +pub fn sync_active_field_task() -> iced::Task { + use iced::advanced::widget::operation::{Focusable, Outcome}; + use iced::advanced::widget::{Id, Operation}; + #[derive(Default)] + struct FocusedId { + focused: Option, + } + impl Operation> for FocusedId { + fn focusable( + &mut self, + id: Option<&Id>, + _bounds: iced::Rectangle, + state: &mut dyn Focusable, + ) { + if state.is_focused() && self.focused.is_none() { + self.focused = id.cloned(); + } + } + fn traverse(&mut self, operate: &mut dyn FnMut(&mut dyn Operation>)) { + operate(self); + } + fn finish(&self) -> Outcome> { + Outcome::Some(self.focused.clone()) + } + } + iced::advanced::widget::operate(FocusedId::default()).map(Message::PropSyncActive) +} + /// A translated choice label paired with the unchanged value stored in the /// drawing and emitted by the properties panel. #[derive(Clone, Debug, PartialEq, Eq)] @@ -324,6 +424,11 @@ pub struct PropertiesPanel { pub expanded_groups: HashSet, /// Whether the editable-dropdown (block Name) option list is open. pub edit_choice_open: bool, + /// Field key of the value row currently being edited, or `None`. Marked when + /// the user focuses an editable value field and cleared on commit / when the + /// selection changes, so the active row can be highlighted. Geometry fields + /// use the raw `&'static str` key; attribute rows use `attr_edit_key(tag)`. + pub active_field: Option, } impl Default for PropertiesPanel { @@ -352,6 +457,7 @@ impl Default for PropertiesPanel { prop_vertex_indicator_active: false, expanded_groups: HashSet::default(), edit_choice_open: false, + active_field: None, } } } @@ -939,7 +1045,9 @@ impl PropertiesPanel { .map(|s| s.as_str()) .unwrap_or(entity_val); + let active = self.active_field.as_deref() == Some(field); let ti = text_input("", display) + .id(prop_geom_field_id(field)) .on_input(move |v| Message::PropGeomInput { field, value: v }) .on_submit(Message::PropGeomCommit(field)) .size(FONT_SZ) @@ -947,7 +1055,7 @@ impl PropertiesPanel { .padding([3, 6]) .width(Length::Fill); - prop_row_widget(label, ti.into()) + prop_row_with_active(label, ti.into(), active) } /// Editable dropdown row (block reference Name): a text field with a caret @@ -965,6 +1073,10 @@ impl PropertiesPanel { let typed = self.edit_buf.get(field); let display = typed.map(|s| s.as_str()).unwrap_or(entity_val); + // NOTE: unlike `render_edit_row` / `render_attr_row`, this input is + // deliberately NOT wrapped in a click-to-focus mouse_area. It is a + // caret-dropdown: clicking should place the caret / open the list, and + // the auto Select-all-on-focus would fight that. Don't "fix" it. let input = text_input("", display) .on_input(move |v| Message::PropGeomInput { field, value: v }) .on_submit(Message::PropGeomCommit(field)) @@ -1078,8 +1190,10 @@ impl PropertiesPanel { .map(|s| s.as_str()) .unwrap_or(entity_val); + let active = self.active_field.as_deref() == Some(key.as_str()); let tag_for_input = tag.to_string(); let ti = text_input("", display) + .id(prop_attr_field_id(tag)) .on_input(move |v| Message::PropAttrInput { tag: tag_for_input.clone(), value: v, @@ -1090,7 +1204,7 @@ impl PropertiesPanel { .padding([3, 6]) .width(Length::Fill); - prop_row_widget(tag, ti.into()) + prop_row_with_active(tag, ti.into(), active) } fn render_hatch_pattern_row<'a>( @@ -1683,15 +1797,28 @@ fn render_ro_row<'a>(label: &'a str, value: &'a str) -> Element<'a, Message> { /// Build a label | widget property row. fn prop_row_widget<'a>(label: &'a str, widget: Element<'a, Message>) -> Element<'a, Message> { + prop_row_with_active(label, widget, false) +} + +/// Like [`prop_row_widget`] but tints the row as the currently-active edit +/// row when `active` is true, using the palette's `primary.weak` highlight so +/// it reads as "selected" without shouting. +fn prop_row_with_active<'a>( + label: &'a str, + widget: Element<'a, Message>, + active: bool, +) -> Element<'a, Message> { let label_col = container( text(crate::ui::text_util::elide(label, 18)) .size(FONT_SZ) .style(muted_text_style), ) - .style(|theme: &Theme| container::Style { - background: Some(Background::Color( - theme.palette().background.weakest.color, - )), + .style(move |theme: &Theme| container::Style { + background: Some(Background::Color(if active { + theme.palette().primary.weak.color + } else { + theme.palette().background.weakest.color + })), ..Default::default() }) .width(Length::FillPortion(5)) @@ -1704,10 +1831,12 @@ fn prop_row_widget<'a>(label: &'a str, widget: Element<'a, Message>) -> Element< right: 6.0, }); let value_col = container(widget) - .style(|theme: &Theme| container::Style { - background: Some(Background::Color( - theme.palette().background.base.color, - )), + .style(move |theme: &Theme| container::Style { + background: Some(Background::Color(if active { + theme.palette().primary.weak.color + } else { + theme.palette().background.base.color + })), ..Default::default() }) .width(Length::FillPortion(6)) @@ -1721,9 +1850,13 @@ fn prop_row_widget<'a>(label: &'a str, widget: Element<'a, Message>) -> Element< }); container(row![label_col, value_col]) .height(Length::Fixed(ROW_H)) - .style(|theme: &Theme| container::Style { + .style(move |theme: &Theme| container::Style { border: Border { - color: theme.palette().background.neutral.color, + color: if active { + theme.palette().primary.base.color + } else { + theme.palette().background.neutral.color + }, width: 1.0, radius: 0.0.into(), }, @@ -1871,7 +2004,109 @@ fn hint_text_style(theme: &Theme) -> iced::widget::text::Style { #[cfg(test)] mod tests { - use super::{hatch_pattern_matches, hatch_preview_scale}; + use super::{ + active_key_focused, hatch_pattern_matches, hatch_preview_scale, prop_attr_field_id, + prop_field_key_for_id, prop_geom_field_id, + }; + + fn sample_sections() -> Vec { + use crate::scene::model::object::{Property, PropValue}; + vec![ + crate::scene::model::object::PropSection { + title: "Geometry".into(), + props: vec![Property { + label: "X".into(), + field: "pos_x", + value: PropValue::EditText("1.0".into()), + }], + }, + crate::scene::model::object::PropSection { + title: "Attributes".into(), + props: vec![Property { + label: "TITLE".into(), + field: "0", + value: PropValue::AttrText { + tag: "TITLE".into(), + value: "hello".into(), + }, + }], + }, + crate::scene::model::object::PropSection { + title: "Reference".into(), + props: vec![Property { + label: "Name".into(), + field: "name", + value: PropValue::EditChoice { + value: "Door".into(), + options: vec!["Door".into(), "Window".into()], + }, + }], + }, + ] + } + + #[test] + fn focus_transition_updates_the_active_field_highlight() { + // The highlight is keyed to real keyboard focus: it persists only while + // the focused widget is THAT field's text input, and clears when focus + // moves to any other widget (or nowhere). + // + // Focus on the active field's own input → row stays marked active. + let focused = prop_geom_field_id("pos_x"); + assert!(active_key_focused(Some("pos_x"), Some(&focused))); + + // Focus moves to another property field → superseded (cleared here; the + // manual focus-setter then marks the new field active). + assert!(!active_key_focused(Some("pos_x"), Some(&prop_geom_field_id("pos_y")))); + + // Focus moves to a non-property widget / nothing → cleared. + assert!(!active_key_focused(Some("pos_x"), Some(&prop_attr_field_id("TITLE")))); + assert!(!active_key_focused(Some("pos_x"), None)); + + // No active field (or one whose input isn't focused) never reads active. + assert!(!active_key_focused(None, Some(&prop_geom_field_id("pos_x")))); + } + + #[test] + fn field_ids_are_distinct_across_geometries_and_attributes() { + assert_ne!(prop_geom_field_id("pos_x"), prop_geom_field_id("pos_y")); + assert_ne!(prop_geom_field_id("pos_x"), prop_attr_field_id("TITLE")); + assert_eq!(prop_attr_field_id("TITLE"), prop_attr_field_id("TITLE")); + } + + #[test] + fn prop_field_key_for_id_maps_focus_ids_to_active_keys() { + let sections = sample_sections(); + + // A focused geometry edit field maps back to its `&'static str` key, + // and round-trips through `active_key_focused`. + let geom_id = prop_geom_field_id("pos_x"); + let key = prop_field_key_for_id(§ions, &geom_id); + assert_eq!(key.as_deref(), Some("pos_x")); + assert!(active_key_focused(key.as_deref(), Some(&geom_id))); + + // A focused attribute edit field maps back to its sentinel-prefixed key. + let attr_id = prop_attr_field_id("TITLE"); + let key = prop_field_key_for_id(§ions, &attr_id); + assert_eq!(key.as_deref(), Some("\x01attr\x01TITLE")); + assert!(active_key_focused(key.as_deref(), Some(&attr_id))); + + // The block-Name caret-dropdown is editable but deliberately excluded. + assert_eq!( + prop_field_key_for_id(§ions, &prop_geom_field_id("name")), + None + ); + + // Unknown or non-property ids map to nothing. + assert_eq!( + prop_field_key_for_id(§ions, &prop_geom_field_id("nope")), + None + ); + assert_eq!( + prop_field_key_for_id(§ions, &iced::widget::Id::new("elsewhere")), + None + ); + } #[test] fn hatch_picker_filters_names_and_descriptions() {