diff --git a/src/app/properties.rs b/src/app/properties.rs index 8f2dda280..daf941511 100644 --- a/src/app/properties.rs +++ b/src/app/properties.rs @@ -13,6 +13,31 @@ use codec::{Entity, EntityType, Handle}; /// shows a count-only summary instead. Bulk edits still go through the ribbon. const MAX_PROP_AGGREGATE: usize = 2_000; +fn property_choice_combos( + sections: &[crate::scene::model::object::PropSection], +) -> rustc_hash::FxHashMap< + String, + iced::widget::combo_box::State, +> { + sections + .iter() + .flat_map(|section| section.props.iter()) + .filter_map(|prop| match &prop.value { + PropValue::Choice { options, .. } => Some(( + prop.field.to_string(), + iced::widget::combo_box::State::new( + options + .iter() + .cloned() + .map(ui::properties::LocalizedChoice::new) + .collect(), + ), + )), + _ => None, + }) + .collect() +} + /// Cap the total number of selection grips at /// [`crate::app::settings::MAX_SELECTED_GRIPS`], keeping entity order with /// non-midpoint (vertex/stretch) grips first. Below the cap both vecs are @@ -2292,25 +2317,7 @@ impl OpenCADStudio { block_definition: tab .active_block_edit_session() .map(|session| session.br_handle), - choice_combos: sections - .iter() - .flat_map(|section| section.props.iter()) - .filter_map(|prop| match &prop.value { - crate::scene::model::object::PropValue::Choice { - options, .. - } => Some(( - prop.field.to_string(), - iced::widget::combo_box::State::new( - options - .iter() - .cloned() - .map(ui::properties::LocalizedChoice::new) - .collect(), - ), - )), - _ => None, - }) - .collect(), + choice_combos: property_choice_combos(§ions), sections, layer_combo: iced::widget::combo_box::State::new(layer_names.clone()), linetype_combo: iced::widget::combo_box::State::new(linetype_items.clone()), @@ -2332,25 +2339,7 @@ impl OpenCADStudio { prop_vertex, ); ui::PropertiesPanel { - choice_combos: sections - .iter() - .flat_map(|section| section.props.iter()) - .filter_map(|prop| match &prop.value { - crate::scene::model::object::PropValue::Choice { - options, .. - } => Some(( - prop.field.to_string(), - iced::widget::combo_box::State::new( - options - .iter() - .cloned() - .map(ui::properties::LocalizedChoice::new) - .collect(), - ), - )), - _ => None, - }) - .collect(), + choice_combos: property_choice_combos(§ions), sections, title, layer_combo: iced::widget::combo_box::State::new(layer_names.clone()), @@ -2457,25 +2446,7 @@ filter={:.1} local={:.1} aggregate={:.1} entities={}", retain_compact_solid_sections(&mut sections); } ui::PropertiesPanel { - choice_combos: sections - .iter() - .flat_map(|section| section.props.iter()) - .filter_map(|prop| match &prop.value { - crate::scene::model::object::PropValue::Choice { - options, .. - } => Some(( - prop.field.to_string(), - iced::widget::combo_box::State::new( - options - .iter() - .cloned() - .map(ui::properties::LocalizedChoice::new) - .collect(), - ), - )), - _ => None, - }) - .collect(), + choice_combos: property_choice_combos(§ions), sections, title: t!("%{count} objects selected", count = selected.len()).into_owned(), selection_group_combo: iced::widget::combo_box::State::new(groups.clone()), @@ -2948,23 +2919,36 @@ handles={handles_ms:.1} panel={:.1} ribbon={ribbon_ms:.1} tail={:.1} selected={} pub(super) fn property_target_handles(&self, i: usize) -> Vec { // The node graph edits its own node's entity through the panel's // handlers, independent of the drawing selection. - if let Some(handle) = self.property_target_override { - return vec![handle]; - } - let mut handles = self.tabs[i].properties.selected_handles(); - if handles.is_empty() { - handles = self.tabs[i].properties.source_handles.clone(); - } - if handles.is_empty() { - handles.extend(self.tabs[i].selected_handle); - } - handles.retain(|handle| !self.tabs[i].scene.is_layer_locked(*handle)); - handles + self.resolve_property_targets(i, true, |scene, handle| { + !scene.is_layer_locked(handle) + }) } /// The point clouds the Locked row applies to: locked ones too, unless /// their layer is locked. pub(super) fn point_cloud_lock_targets(&self, i: usize) -> Vec { + self.resolve_property_targets(i, false, |scene, handle| { + scene.locked_layer_name(handle).is_none() + }) + } + + fn resolve_property_targets( + &self, + i: usize, + include_override: bool, + mut include: impl FnMut(&crate::scene::Scene, Handle) -> bool, + ) -> Vec { + if include_override { + if let Some(handle) = self.property_target_override { + let scene = &self.tabs[i].scene; + return scene + .document + .get_entity(handle) + .filter(|_| include(scene, handle)) + .map(|_| vec![handle]) + .unwrap_or_default(); + } + } let mut handles = self.tabs[i].properties.selected_handles(); if handles.is_empty() { handles = self.tabs[i].properties.source_handles.clone(); @@ -2972,7 +2956,10 @@ handles={handles_ms:.1} panel={:.1} ribbon={ribbon_ms:.1} tail={:.1} selected={} if handles.is_empty() { handles.extend(self.tabs[i].selected_handle); } - handles.retain(|handle| self.tabs[i].scene.locked_layer_name(*handle).is_none()); + let scene = &self.tabs[i].scene; + handles.retain(|handle| { + scene.document.get_entity(*handle).is_some() && include(scene, *handle) + }); handles } @@ -4689,6 +4676,45 @@ mod chprop_integration_tests { ); } + #[test] + fn property_target_resolvers_discard_stale_handles() { + let mut app = OpenCADStudio::new_for_test(); + let i = app.active_tab; + let existing = line_handle(&mut app); + let stale = app.tabs[i].scene.document.allocate_handle(); + app.tabs[i].properties.source_handles = vec![existing, stale]; + + assert_eq!(app.property_target_handles(i), vec![existing]); + assert_eq!(app.point_cloud_lock_targets(i), vec![existing]); + + app.tabs[i].properties.source_handles = vec![stale]; + assert!(app.property_target_handles(i).is_empty()); + assert!(app.point_cloud_lock_targets(i).is_empty()); + + app.finish_pending_history(i); + let undo_before = app.tabs[i].history.undo_stack.len(); + let color_before = app.tabs[i] + .scene + .document + .header + .current_entity_color; + app.tabs[i].dirty = false; + let _ = app.update(crate::app::Message::PropColorChanged( + codec::types::Color::Index(9), + )); + app.finish_pending_history(i); + assert_eq!(app.tabs[i].history.undo_stack.len(), undo_before); + assert_eq!( + app.tabs[i] + .scene + .document + .header + .current_entity_color, + color_before + ); + assert!(!app.tabs[i].dirty); + } + /// Regression test for the CLI `CHPROP` command. Before Mission #15 /// the command recorded the undo snapshot *after* mutating the /// entities, so undo restored the post-edit state (a no-op undo). diff --git a/src/app/view/modal.rs b/src/app/view/modal.rs index a6de51409..537686067 100644 --- a/src/app/view/modal.rs +++ b/src/app/view/modal.rs @@ -128,109 +128,8 @@ impl OpenCADStudio { super::super::ModalKind::XrefHelp => { automatic_flow(ex, crate::ui::window::xref_help::view_window) } - super::super::ModalKind::Shortcuts => { - // Keys claimed by two rows — the cells turn red and a - // persistent warning names the command already using each - // key; the last row wins in the binding map on Apply. - let (duplicate_set, duplicate_conflicts) = duplicate_rows( - &self.shortcut_editor_rows, - crate::app::shortcuts::normalize_key, - ); - // Commands the dispatcher can't run: not a registered - // command, plugin command, alias, or input action. - let valid: rustc_hash::FxHashSet = - crate::command::all_registered_command_names() - .into_iter() - .map(str::to_uppercase) - .chain( - self.command_line - .dynamic_commands - .iter() - .map(|cmd| cmd.to_uppercase()), - ) - .chain(self.command_aliases.keys().cloned()) - .chain( - crate::app::shortcuts::INPUT_ACTIONS - .iter() - .map(|action| action.to_string()), - ) - .collect(); - let unknown_commands: Vec = self - .shortcut_editor_rows - .iter() - .filter_map(|(_, command)| { - let command = command.trim(); - (!command.is_empty() && !valid.contains(command)) - .then(|| command.to_string()) - }) - .collect(); - let unknown_set: rustc_hash::FxHashSet = - unknown_commands.into_iter().collect(); - sized_flow(ex, 720, 520, |flow| { - crate::ui::window::shortcuts::view_window( - &self.shortcut_editor_rows, - self.shortcut_capture_row, - self.shortcut_pending_add, - self.shortcut_reset_confirm, - &duplicate_set, - &duplicate_conflicts, - &unknown_set, - self.shortcut_close_confirm, - flow, - ) - }) - } - super::super::ModalKind::Aliases => { - // Aliases claimed by two rows — the cells turn red and a - // persistent warning names the command already using each - // alias; the last row wins in the alias map on Apply. - let (duplicate_set, duplicate_conflicts) = duplicate_rows( - &self.alias_editor_rows, - |alias| alias.trim().to_uppercase(), - ); - // Commands the dispatcher can't run: not a registered - // command, plugin command, or input action. - let valid: rustc_hash::FxHashSet = - crate::command::all_registered_command_names() - .into_iter() - .map(str::to_uppercase) - .chain( - self.command_line - .dynamic_commands - .iter() - .map(|cmd| cmd.to_uppercase()), - ) - .chain( - crate::app::shortcuts::INPUT_ACTIONS - .iter() - .map(|action| action.to_string()), - ) - .collect(); - let unknown_commands: Vec = self - .alias_editor_rows - .iter() - .filter_map(|(_, command)| { - let command = command.trim(); - (!command.is_empty() && !valid.contains(command)) - .then(|| command.to_string()) - }) - .collect(); - let unknown_set: rustc_hash::FxHashSet = - unknown_commands.into_iter().collect(); - sized_flow(ex, 480, 520, |flow| { - crate::ui::window::alias_editor::view_window( - &self.alias_editor_rows, - &self.alias_import_report, - self.alias_pending_add, - self.alias_reset_confirm, - &duplicate_set, - &duplicate_conflicts, - &unknown_set, - self.alias_close_confirm, - flow, - ) - }) - } + super::super::ModalKind::Shortcuts => self.shortcuts_modal_content(ex), + super::super::ModalKind::Aliases => self.aliases_modal_content(ex), super::super::ModalKind::NamedParameters => { let scene = &self.tabs[self.active_tab].scene; sized_flow(ex, 820, 520, |flow| { @@ -1901,15 +1800,109 @@ impl OpenCADStudio { } }) } + + fn shortcuts_modal_content<'s>(&'s self, ex: iced::Vector) -> Element<'s, Message> { + // Keys claimed by two rows — the cells turn red and a persistent + // warning names the command already using each key. + let (duplicate_set, duplicate_conflicts) = duplicate_rows( + &self.shortcut_editor_rows, + crate::app::shortcuts::normalize_key, + ); + // Commands the dispatcher can't run: not a registered command, + // plugin command, alias, or input action. + let valid: rustc_hash::FxHashSet = + crate::command::all_registered_command_names() + .into_iter() + .map(str::to_uppercase) + .chain( + self.command_line + .dynamic_commands + .iter() + .map(|cmd| cmd.to_uppercase()), + ) + .chain(self.command_aliases.keys().cloned()) + .chain( + crate::app::shortcuts::INPUT_ACTIONS + .iter() + .map(|action| action.to_string()), + ) + .collect(); + let unknown_set: rustc_hash::FxHashSet = self + .shortcut_editor_rows + .iter() + .filter_map(|(_, command)| { + let command = command.trim(); + (!command.is_empty() && !valid.contains(command)).then(|| command.to_string()) + }) + .collect(); + sized_flow(ex, 720, 520, |flow| { + crate::ui::window::shortcuts::view_window( + &self.shortcut_editor_rows, + self.shortcut_capture_row, + self.shortcut_pending_add, + self.shortcut_reset_confirm, + &duplicate_set, + &duplicate_conflicts, + &unknown_set, + self.shortcut_close_confirm, + flow, + ) + }) + } + + fn aliases_modal_content<'s>(&'s self, ex: iced::Vector) -> Element<'s, Message> { + // Aliases claimed by two rows — the cells turn red and a persistent + // warning names the command already using each alias. + let (duplicate_set, duplicate_conflicts) = duplicate_rows( + &self.alias_editor_rows, + |alias| alias.trim().to_uppercase(), + ); + // Commands the dispatcher can't run: not a registered command, + // plugin command, or input action. + let valid: rustc_hash::FxHashSet = + crate::command::all_registered_command_names() + .into_iter() + .map(str::to_uppercase) + .chain( + self.command_line + .dynamic_commands + .iter() + .map(|cmd| cmd.to_uppercase()), + ) + .chain( + crate::app::shortcuts::INPUT_ACTIONS + .iter() + .map(|action| action.to_string()), + ) + .collect(); + let unknown_set: rustc_hash::FxHashSet = self + .alias_editor_rows + .iter() + .filter_map(|(_, command)| { + let command = command.trim(); + (!command.is_empty() && !valid.contains(command)).then(|| command.to_string()) + }) + .collect(); + sized_flow(ex, 480, 520, |flow| { + crate::ui::window::alias_editor::view_window( + &self.alias_editor_rows, + &self.alias_import_report, + self.alias_pending_add, + self.alias_reset_confirm, + &duplicate_set, + &duplicate_conflicts, + &unknown_set, + self.alias_close_confirm, + flow, + ) + }) + } } fn duplicate_rows( rows: &[(String, String)], normalize: impl Fn(&str) -> String, -) -> ( - rustc_hash::FxHashSet, - Vec<(String, String)>, -) { +) -> (rustc_hash::FxHashSet, Vec<(String, String)>) { let mut grouped: std::collections::BTreeMap> = std::collections::BTreeMap::new(); for row in rows { @@ -1937,6 +1930,9 @@ fn duplicate_rows( (duplicate_set, duplicates) } +/// Build a dialog with an explicit initial size cap. Use this for dense forms +/// and tables that contain their own scrollable regions; the cap keeps their +/// intrinsic first layout bounded, while `extra` records user resizing. fn sized_flow<'a>( extra: iced::Vector, max_width: u16, @@ -1951,6 +1947,8 @@ fn sized_flow<'a>( ) } +/// Build a dialog at its content's natural size. Use this for compact prompts +/// and small forms whose contents should determine the initial frame. fn automatic_flow<'a>( extra: iced::Vector, mut build: impl FnMut(crate::ui::modal::ModalSizing) -> Element<'a, Message>, diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index b256e07b0..5934ceaeb 100644 --- a/src/ui/ribbon/mod.rs +++ b/src/ui/ribbon/mod.rs @@ -734,13 +734,17 @@ impl Ribbon { } } - /// Returns the index of the Layout module in the modules list. - #[allow(dead_code)] // layout-tab helpers; not yet wired + /// Returns the index of the Layout module in the modules list. Kept as a + /// routing helper for when the currently suppressed Layout ribbon tab is + /// enabled; it has no effect while that module is absent from the tabs. + #[allow(dead_code)] pub fn layout_module_index(&self) -> Option { self.modules.iter().position(|m| m.id() == "layout") } - /// Returns true if the currently active tab is the Layout module. + /// Returns true if the currently active tab is the Layout module. The + /// predicate remains useful to the future Layout tab, which is currently + /// suppressed by the view layer. #[allow(dead_code)] pub fn active_is_layout(&self) -> bool { self.modules diff --git a/src/ui/window/layers.rs b/src/ui/window/layers.rs index a2aeb23c4..4b064d4f1 100644 --- a/src/ui/window/layers.rs +++ b/src/ui/window/layers.rs @@ -119,8 +119,6 @@ impl Layer { #[derive(Clone)] pub struct LayerPanel { pub layers: Vec, - #[allow(dead_code)] - pub visible: bool, /// Anchor row: the last row clicked. Drives the editable combos and the /// Shift-range origin. Its layer is always part of `selected_multi`. pub selected: Option, @@ -152,7 +150,6 @@ pub struct LayerPanel { impl Default for LayerPanel { fn default() -> Self { Self { - visible: false, layers: vec![Layer::new("0", AcadColor::Index(7))], selected: None, selected_multi: Vec::new(),