Merge pull request #1353 from HakanSeven12/ui/options-ok-apply-close

feat(options): OK / Apply / Close with a discard guard

# Conflicts:
#	src/app/view/modal.rs
#	src/ui/window/options.rs

src/app/mod.rs

@@ -18,6 +18,7 @@ mod dimension_preview_tests;
 mod document;
 mod drafting_settings;
 pub(crate) mod expr_eval;
+mod options_session;
 mod find_replace;
 pub(crate) mod helpers;
 mod history;
@@ -570,6 +571,10 @@ pub(super) struct OpenCADStudio {
     dyn_input: bool,
     /// Currently visible page in the application Options dialog.
     options_tab: crate::ui::window::options::OptionsTab,
+    /// The Options window's commit point (see `options_session`).
+    options_saved: Option<options_session::OptionsSnapshot>,
+    /// Close was pressed with unapplied changes; the discard guard is up.
+    options_close_confirm: bool,
     spacemouse: crate::input::spacemouse::Service,
     spacemouse_preferences: crate::input::spacemouse::Preferences,
     spacemouse_paused: bool,
@@ -2860,6 +2865,14 @@ pub enum Message {
     DraftingSettingsClose,
     DraftingSettingsCloseDiscard,
     DraftingSettingsCloseKeep,
+    /// Options window: commit the changes made so far.
+    OptionsApply,
+    /// Options window: commit and close.
+    OptionsOk,
+    /// Options window: close, asking first when changes would be lost.
+    OptionsClose,
+    OptionsCloseDiscard,
+    OptionsCloseKeep,
     AutoConstrainSelectRow(usize),
     AutoConstrainToggleKind(settings::AutoConstraintKind),
     AutoConstrainMoveUp,
@@ -3900,6 +3913,8 @@ impl OpenCADStudio {
             grid_beyond_limits: true,
             dyn_input: true,
             options_tab: crate::ui::window::options::OptionsTab::General,
+            options_saved: None,
+            options_close_confirm: false,
             spacemouse: {
                 let service = crate::input::spacemouse::Service::default();
                 service.set_actions(navigation::actions());

src/app/options_session.rs

@@ -0,0 +1,225 @@
+//! The Options window's OK / Apply / Close. Settings still change the moment
+//! a control is touched, so a theme, a cursor size or a language is seen at
+//! once — but nothing is committed until Apply or OK. Close (and the window's
+//! × or Esc) puts everything back the way it was when the window opened, or
+//! when Apply was last pressed, asking first when something would be lost.
+//!
+//! The commit point is a snapshot of the persisted configuration plus the
+//! few drawing variables the window edits (the 3D Modeling page writes into
+//! the active drawing's header); "dirty" is simply "the current snapshot
+//! differs from the saved one", so every control the window gains later is
+//! covered without registering it here.
+
+use super::OpenCADStudio;
+
+/// The drawing-header variables the Options window edits, for the active
+/// tab, with the tab's modified flag so a revert leaves it as it found it.
+#[derive(Clone, PartialEq)]
+struct HeaderVars {
+    isolines: i16,
+    display_silhouette: bool,
+    surface_u_density: i16,
+    surface_v_density: i16,
+    surface_type: i16,
+    record_solid_history: bool,
+    show_solid_history: i16,
+    tab_dirty: bool,
+}
+
+/// What the Options window can change, as it stood at the last commit.
+#[derive(Clone, PartialEq)]
+pub(super) struct OptionsSnapshot {
+    config: super::config::AppConfig,
+    header: Option<HeaderVars>,
+    textfill: bool,
+}
+
+impl OpenCADStudio {
+    fn options_snapshot(&self) -> OptionsSnapshot {
+        let header = self.tabs.get(self.active_tab).map(|tab| {
+            let h = &tab.scene.document.header;
+            HeaderVars {
+                isolines: h.isolines,
+                display_silhouette: h.display_silhouette,
+                surface_u_density: h.surface_u_density,
+                surface_v_density: h.surface_v_density,
+                surface_type: h.surface_type,
+                record_solid_history: h.record_solid_history,
+                show_solid_history: h.show_solid_history,
+                tab_dirty: tab.dirty,
+            }
+        });
+        OptionsSnapshot {
+            config: self.current_config(),
+            header,
+            textfill: crate::scene::text::sdf_atlas::textfill(),
+        }
+    }
+
+    /// The window opens: remember the state to come back to.
+    pub(super) fn options_open(&mut self) {
+        self.active_modal = Some(super::ModalKind::Options);
+        self.options_saved = Some(self.options_snapshot());
+        self.options_close_confirm = false;
+    }
+
+    /// Whether anything changed since the window opened or Apply was pressed.
+    pub(super) fn options_dirty(&self) -> bool {
+        match &self.options_saved {
+            Some(saved) => *saved != self.options_snapshot(),
+            None => false,
+        }
+    }
+
+    /// Apply: the current state becomes the state to come back to, and is
+    /// written to disk.
+    pub(super) fn options_apply(&mut self) {
+        self.options_saved = Some(self.options_snapshot());
+        self.options_close_confirm = false;
+        self.save_config();
+    }
+
+    /// Close: put everything back to the last commit. The caller closes the
+    /// window afterwards.
+    pub(super) fn options_discard(&mut self) {
+        self.options_close_confirm = false;
+        if let Some(saved) = self.options_saved.take() {
+            self.options_revert_to(saved);
+        }
+    }
+
+    /// Forget the commit point without reverting (OK, or a close that had
+    /// nothing to lose).
+    pub(super) fn options_forget(&mut self) {
+        self.options_saved = None;
+        self.options_close_confirm = false;
+    }
+
+    fn options_revert_to(&mut self, saved: OptionsSnapshot) {
+        let current = self.options_snapshot();
+        if current == saved {
+            return;
+        }
+        if current.config != saved.config {
+            // `apply_config` distributes every persisted preference, the
+            // theme and the language the way start-up does.
+            self.apply_config(saved.config.clone());
+        }
+        if current.textfill != saved.textfill {
+            // The glyph atlas was re-baked for the other setting.
+            self.invalidate_text_everywhere();
+        }
+        if let (Some(now), Some(then)) = (&current.header, &saved.header) {
+            if now != then {
+                let i = self.active_tab;
+                if let Some(tab) = self.tabs.get_mut(i) {
+                    let h = &mut tab.scene.document.header;
+                    h.isolines = then.isolines;
+                    h.display_silhouette = then.display_silhouette;
+                    h.surface_u_density = then.surface_u_density;
+                    h.surface_v_density = then.surface_v_density;
+                    h.surface_type = then.surface_type;
+                    h.record_solid_history = then.record_solid_history;
+                    h.show_solid_history = then.show_solid_history;
+                    tab.dirty = then.tab_dirty;
+                    if now.show_solid_history != then.show_solid_history
+                        || now.display_silhouette != then.display_silhouette
+                    {
+                        tab.scene.bump_geometry();
+                    }
+                }
+                if now.isolines != then.isolines {
+                    self.isolines_awaiting_regen = false;
+                    self.regenerate_meshes();
+                }
+            }
+        }
+        self.save_config();
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::app::{Message, OpenCADStudio};
+
+    fn app() -> OpenCADStudio {
+        let mut app = OpenCADStudio::new_for_test();
+        app.automation_op(r#"{"op":"new"}"#);
+        app
+    }
+
+    #[test]
+    fn changes_show_at_once_but_close_puts_them_back() {
+        let mut app = app();
+        let _ = app.update(Message::OptionsOpen);
+        assert!(!app.options_dirty());
+        let before = app.zoom_factor;
+        let _ = app.update(Message::ZoomFactorChanged(before + 7));
+        assert_eq!(app.zoom_factor, before + 7, "the change is live");
+        assert!(app.options_dirty());
+        // The first Close asks; the discard puts the value back.
+        let _ = app.update(Message::OptionsClose);
+        assert!(app.options_close_confirm);
+        assert_eq!(app.active_modal, Some(crate::app::ModalKind::Options));
+        let _ = app.update(Message::OptionsCloseDiscard);
+        assert_eq!(app.zoom_factor, before);
+        assert_eq!(app.active_modal, None);
+        assert!(!app.options_dirty());
+    }
+
+    #[test]
+    fn apply_makes_the_change_the_new_baseline() {
+        let mut app = app();
+        let _ = app.update(Message::OptionsOpen);
+        let before = app.zoom_factor;
+        let _ = app.update(Message::ZoomFactorChanged(before + 7));
+        let _ = app.update(Message::OptionsApply);
+        assert!(!app.options_dirty(), "Apply commits");
+        let _ = app.update(Message::ZoomFactorChanged(before + 9));
+        let _ = app.update(Message::OptionsClose);
+        let _ = app.update(Message::OptionsCloseDiscard);
+        assert_eq!(app.zoom_factor, before + 7, "back to the applied value, not the original");
+    }
+
+    #[test]
+    fn ok_commits_and_closes_and_a_clean_close_asks_nothing() {
+        let mut app = app();
+        let _ = app.update(Message::OptionsOpen);
+        let before = app.zoom_factor;
+        let _ = app.update(Message::ZoomFactorChanged(before + 7));
+        let _ = app.update(Message::OptionsOk);
+        assert_eq!(app.active_modal, None);
+        assert_eq!(app.zoom_factor, before + 7);
+        let _ = app.update(Message::OptionsOpen);
+        let _ = app.update(Message::OptionsClose);
+        assert_eq!(app.active_modal, None, "nothing to lose, nothing to ask");
+        assert!(!app.options_close_confirm);
+        // Keep editing returns to the window with the change intact.
+        let _ = app.update(Message::OptionsOpen);
+        let _ = app.update(Message::ZoomFactorChanged(before + 11));
+        let _ = app.update(Message::CloseModal);
+        assert!(app.options_close_confirm, "the × asks like Close does");
+        let _ = app.update(Message::OptionsCloseKeep);
+        assert_eq!(app.active_modal, Some(crate::app::ModalKind::Options));
+        assert_eq!(app.zoom_factor, before + 11);
+    }
+
+    #[test]
+    fn drawing_variables_of_the_modeling_page_are_reverted_too() {
+        let mut app = app();
+        let i = app.active_tab;
+        let isolines = app.tabs[i].scene.document.header.isolines;
+        let silhouette = app.tabs[i].scene.document.header.display_silhouette;
+        let _ = app.update(Message::OptionsOpen);
+        let _ = app.update(Message::IsolinesChanged(isolines + 4));
+        let _ = app.update(Message::DispSilhChanged(!silhouette));
+        assert!(app.options_dirty());
+        assert!(app.tabs[i].dirty);
+        let _ = app.update(Message::OptionsClose);
+        let _ = app.update(Message::OptionsCloseDiscard);
+        let h = &app.tabs[i].scene.document.header;
+        assert_eq!(h.isolines, isolines);
+        assert_eq!(h.display_silhouette, silhouette);
+        assert!(!app.tabs[i].dirty, "the drawing is as unmodified as it was");
+    }
+}

src/app/update/mod.rs

@@ -177,6 +177,10 @@ impl OpenCADStudio {
             self.drafting_settings_state = None;
             self.drafting_settings_saved = None;
         }
+        if self.active_modal == Some(Options) {
+            self.options_saved = None;
+            self.options_close_confirm = false;
+        }
         #[cfg(not(target_arch = "wasm32"))]
         if self.active_modal == Some(FileInUse) {
             self.pending_save_failure = None;
@@ -7065,7 +7069,35 @@ impl OpenCADStudio {
             }
             // ── Options / About windows ───────────────────────────────────
             Message::OptionsOpen => {
-                self.active_modal = Some(super::ModalKind::Options);
+                self.options_open();
+                Task::none()
+            }
+            Message::OptionsApply => {
+                self.options_apply();
+                Task::none()
+            }
+            Message::OptionsOk => {
+                self.options_apply();
+                self.options_forget();
+                self.close_active_modal();
+                Task::none()
+            }
+            Message::OptionsClose => {
+                if !self.options_close_confirm && self.options_dirty() {
+                    self.options_close_confirm = true;
+                    return Task::none();
+                }
+                self.options_discard();
+                self.close_active_modal();
+                Task::none()
+            }
+            Message::OptionsCloseDiscard => {
+                self.options_discard();
+                self.close_active_modal();
+                Task::none()
+            }
+            Message::OptionsCloseKeep => {
+                self.options_close_confirm = false;
                 Task::none()
             }
 
@@ -7682,6 +7714,10 @@ impl OpenCADStudio {
                 if self.active_modal == Some(super::ModalKind::RecoveryPrompt) {
                     return self.update(Message::RecoveryDecline);
                 }
+                // The Options window's × and Esc behave like its Close button.
+                if self.active_modal == Some(super::ModalKind::Options) {
+                    return self.update(Message::OptionsClose);
+                }
                 // Closing the shortcut editor with un-applied rows needs an
                 // explicit discard; a second close attempt (or the overlay's
                 // Discard button) goes through.

src/app/view/modal.rs

@@ -265,7 +265,10 @@ impl OpenCADStudio {
                     },
                 )
             }
-            super::super::ModalKind::Options => sized_flow(
+            super::super::ModalKind::Options => {
+                let dirty = self.options_dirty();
+                let close_confirm = self.options_close_confirm;
+                sized_flow(
                 ex,
                 880,
                 620,
@@ -361,10 +364,13 @@ impl OpenCADStudio {
                         &self.paper_bg_input,
                         &self.desk_bg_input,
                         self.bg_picker,
+                        dirty,
+                        close_confirm,
                         flow,
                     )
                 },
-            ),
+                )
+            }
             super::super::ModalKind::DraftingSettings => {
                 let state = self.drafting_settings_state.as_ref();
                 let dirty = self.drafting_settings_dirty();

src/ui/modal.rs

@@ -244,6 +244,60 @@ pub fn backdrop<'a>(
     stack![base.into(), opaque(shield)].into()
 }
 
+/// A dialog's "unsaved changes" guard: `main` dimmed behind a small panel
+/// that offers to discard the changes and close, or to keep editing. Clicking
+/// the dim area keeps editing. Callers show it in place of the plain dialog
+/// once the user asked to close with something to lose.
+pub fn discard_guard<'a>(
+    main: impl Into<Element<'a, Message>>,
+    on_discard: Message,
+    on_keep: Message,
+) -> Element<'a, Message> {
+    let shield = mouse_area(
+        container(Space::new())
+            .width(Length::Fill)
+            .height(Length::Fill)
+            .style(|theme: &Theme| container::Style {
+                background: Some(Background::Color(
+                    theme
+                        .palette()
+                        .background
+                        .strongest
+                        .color
+                        .scale_alpha(0.55),
+                )),
+                ..Default::default()
+            }),
+    )
+    .on_press(on_keep.clone());
+    let panel = container(
+        column![
+            iced::widget::text(crate::t!("Unsaved changes will be discarded.")).size(13.5),
+            Space::new().height(12),
+            row![
+                button(iced::widget::text(crate::t!("Discard && close")).size(12))
+                    .on_press(on_discard)
+                    .padding([5, 14])
+                    .style(button::danger),
+                button(iced::widget::text(crate::t!("Keep editing")).size(12))
+                    .on_press(on_keep)
+                    .padding([5, 14])
+                    .style(button::secondary),
+            ]
+            .spacing(10),
+        ]
+        .spacing(0),
+    )
+    .padding([18, 22])
+    .style(container::rounded_box);
+    stack![
+        main.into(),
+        shield,
+        container(panel).center_x(Length::Fill).center_y(Length::Fill)
+    ]
+    .into()
+}
+
 /// Stack `content` over `base` behind a dimmed backdrop, framed with a title bar
 /// (the ✕ close button at its right end). The backdrop only dims and blocks
 /// clicks from reaching the view beneath — it does **not** dismiss the dialog;

src/ui/window/drafting_settings.rs

@@ -2,7 +2,7 @@ use crate::app::settings::IsoPlane;
 use crate::app::Message;
 use crate::snap::{SnapType, ALL_3D_SNAP_MODES, ALL_SNAP_MODES};
 use iced::widget::{button, checkbox, column, container, row, scrollable, text, text_input, Space};
-use iced::{Background, Border, Element, Fill, Length, Theme};
+use iced::{Background, Border, Element, Fill, Theme};
 use std::borrow::Cow;
 
 /// Active tab in the Drafting Settings dialog.
@@ -717,51 +717,10 @@ pub fn view_window<'a>(
     if !close_confirm {
         return main_content.into();
     }
-
-    let shield = iced::widget::mouse_area(
-        container(Space::new())
-            .width(Length::Fill)
-            .height(Length::Fill)
-            .style(|theme: &Theme| container::Style {
-                background: Some(Background::Color(
-                    theme
-                        .palette()
-                        .background
-                        .strongest
-                        .color
-                        .scale_alpha(0.55),
-                )),
-                ..Default::default()
-            }),
-    )
-    .on_press(Message::DraftingSettingsCloseKeep);
-
-    let warning_panel = container(
-        column![
-            text(crate::t!("Unsaved changes will be discarded.")).size(13.5),
-            Space::new().height(12),
-            row![
-                button(text(crate::t!("Discard && close")).size(12))
-                    .on_press(Message::DraftingSettingsCloseDiscard)
-                    .padding([5, 14])
-                    .style(button::danger),
-                button(text(crate::t!("Keep editing")).size(12))
-                    .on_press(Message::DraftingSettingsCloseKeep)
-                    .padding([5, 14])
-                    .style(button::secondary),
-            ]
-            .spacing(10),
-        ]
-        .spacing(0),
-    )
-    .padding([18, 22])
-    .style(container::rounded_box);
-
-    iced::widget::stack![
+    crate::ui::modal::discard_guard(
         main_content,
-        shield,
-        container(warning_panel).center_x(Fill).center_y(Fill)
-    ]
-    .into()
+        Message::DraftingSettingsCloseDiscard,
+        Message::DraftingSettingsCloseKeep,
+    )
 }
 

src/ui/window/options.rs

@@ -212,6 +212,8 @@ pub fn view_window<'a>(
     paper_bg_input: &'a str,
     desk_bg_input: &'a str,
     bg_picker: Option<crate::app::BgTarget>,
+    dirty: bool,
+    close_confirm: bool,
     sizing: crate::ui::modal::ModalSizing,
 ) -> Element<'a, Message> {
     let selected_format = crate::io::SAVE_FORMAT_OPTIONS
@@ -311,8 +313,18 @@ pub fn view_window<'a>(
         );
     }
 
+    // Changes show at once but are committed by OK / Apply; Close puts them
+    // back (asking first when there is something to lose).
+    let ok = button(text(crate::t!("OK")).size(12))
+        .on_press(Message::OptionsOk)
+        .padding([6, 18])
+        .style(button::primary);
+    let apply = button(text(crate::t!("Apply")).size(12))
+        .on_press_maybe(dirty.then_some(Message::OptionsApply))
+        .padding([6, 18])
+        .style(if dirty { button::secondary } else { button::text });
     let close = button(text(crate::tr!("action", "close")).size(12))
-        .on_press(Message::CloseModal)
+        .on_press(Message::OptionsClose)
         .padding([6, 18])
         .style(button::secondary);
 
@@ -1668,7 +1680,7 @@ pub fn view_window<'a>(
         // controls at the trailing edge of the Options content.
         scrollable(content).spacing(8).height(Fill),
         Space::new().height(12),
-        row![Space::new().width(Fill), close],
+        row![Space::new().width(Fill), ok, apply, close].spacing(8),
     ]
     .width(Fill)
     .height(sizing.height);
@@ -1678,10 +1690,13 @@ pub fn view_window<'a>(
         .height(sizing.height);
 
     let intrinsic = sizing.width == crate::ui::modal::ModalSizing::INTRINSIC.width;
-    container(body)
+    let main = container(body)
         .style(container::rounded_box)
         .padding([16, 18])
         .width(if intrinsic { iced::Length::Fixed(DIALOG_WIDTH) } else { sizing.width })
-        .height(if intrinsic { iced::Length::Fixed(DIALOG_HEIGHT) } else { sizing.height })
-        .into()
+        .height(if intrinsic { iced::Length::Fixed(DIALOG_HEIGHT) } else { sizing.height });
+    if !close_confirm {
+        return main.into();
+    }
+    crate::ui::modal::discard_guard(main, Message::OptionsCloseDiscard, Message::OptionsCloseKeep)
 }

tests/fixtures/plot/.page-setups-metric.dxf.ocs.lock

@@ -1 +1 @@
-Open CAD Studio pid=854778 path=/home/panos/OpenCADStudio/tests/fixtures/plot/page-setups-metric.dxf
+Open CAD Studio pid=898490 path=/home/panos/OpenCADStudio/tests/fixtures/plot/page-setups-metric.dxf