From d7fe7fe33c2a71116d1d55685eba588e03ff3772 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Tue, 3 Nov 2026 10:43:49 -0500 Subject: [PATCH] feat(ui): mark unsaved nests and ask before closing them A nest window gave no sign that its nest had changed since the last save, and closing it (or the app) discarded the edits silently. The undo history already tracks a save point, so the title now ends in " *" while the nest is off it (or holds an edit no scope recorded), and closing asks to save, discard or cancel. Yes saves through the storage in use; a database save cancels the close and closes the window once the upload succeeds. Saves now commit any unrecorded edit first so the save point covers what was written. The window's cleanup moves from OnClosing to OnFormClosed: another window can still cancel closing the app, and a nest window must not lose its plate manager when that happens. Co-Authored-By: Claude Opus 5.5 --- .../Forms/UnsavedChangesTests.cs | 142 ++++++++++++++++++ .../IsolatedUserSettings.cs | 8 +- .../Forms/Commands/NestStorageController.cs | 4 +- OpenNest/Forms/EditNestForm.Undo.cs | 19 ++- OpenNest/Forms/EditNestForm.cs | 60 +++++++- OpenNest/Forms/MainForm.File.cs | 11 +- docs/undo-redo.md | 4 + 7 files changed, 239 insertions(+), 9 deletions(-) create mode 100644 OpenNest.WinForms.Tests/Forms/UnsavedChangesTests.cs diff --git a/OpenNest.WinForms.Tests/Forms/UnsavedChangesTests.cs b/OpenNest.WinForms.Tests/Forms/UnsavedChangesTests.cs new file mode 100644 index 00000000..4c315963 --- /dev/null +++ b/OpenNest.WinForms.Tests/Forms/UnsavedChangesTests.cs @@ -0,0 +1,142 @@ +using System.Windows.Forms; +using OpenNest.Forms; + +namespace OpenNest.WinForms.Tests.Forms; + +// The title marks unsaved edits and closing asks before discarding them. Shares the undo +// tests' collection, since it drives the same main-window guards. +[Collection("Fill operation lifetime")] +public class UnsavedChangesTests +{ + [Fact] + public void TheTitleIsMarkedWhileTheNestDiffersFromItsSavedState() => UndoTestEnv.Run(env => + { + var editor = env.Editor; + Assert.False(editor.HasUnsavedChanges); + Assert.Equal("undo", editor.Text); + + DeleteFirstPlatesParts(editor); + Assert.True(editor.HasUnsavedChanges); + Assert.Equal("undo *", editor.Text); + + Assert.True(env.Host.TryUndo(editor)); + Assert.False(editor.HasUnsavedChanges); + Assert.Equal("undo", editor.Text); + + Assert.True(env.Host.TryRedo(editor)); + Assert.Equal("undo *", editor.Text); + + var path = TempNestPath("saved"); + try + { + editor.SaveAs(path); + Assert.False(editor.HasUnsavedChanges); + Assert.Equal("saved", editor.Text); + } + finally + { + File.Delete(path); + } + }); + + [Fact] + public void AnEditNoScopeRecordedStillCountsAsUnsaved() => UndoTestEnv.Run(env => + { + var editor = env.Editor; + editor.Nest.Plates[0].Parts.RemoveAt(0); + + Assert.True(editor.HasUnsavedChanges); + }); + + [Fact] + public void ClosingAnUnchangedNestDoesNotAsk() => UndoTestEnv.Run(env => + { + var asked = 0; + WithPrompt(env.Editor, () => { asked++; return DialogResult.Cancel; }, () => + { + env.Editor.Close(); + + Assert.Equal(0, asked); + Assert.True(env.Editor.IsDisposed); + }); + }); + + [Fact] + public void CancelKeepsTheWindowOpenAndNoClosesWithoutSaving() => UndoTestEnv.Run(env => + { + var editor = env.Editor; + DeleteFirstPlatesParts(editor); + var answer = DialogResult.Cancel; + var asked = 0; + WithPrompt(editor, () => { asked++; return answer; }, () => + { + editor.Close(); + Assert.Equal(1, asked); + Assert.False(editor.IsDisposed); + Assert.True(editor.HasUnsavedChanges); + + answer = DialogResult.No; + editor.Close(); + Assert.Equal(2, asked); + Assert.True(editor.IsDisposed); + }); + }); + + // Without a main window (whose save follows the machine's storage mode) Yes saves to a file. + [Fact] + public void YesSavesToTheNestsFileAndCloses() => StaTestThread.Run(() => + { + using var editor = new EditNestForm(UndoTestEnv.BuildNest()); + editor.PlateView.SetOverlapAutoCheck(null); + editor.Show(); + var path = TempNestPath("yes"); + try + { + editor.SaveAs(path); + var savedAt = File.GetLastWriteTimeUtc(path); + DeleteFirstPlatesParts(editor); + File.SetLastWriteTimeUtc(path, savedAt.AddMinutes(-1)); + + WithPrompt(editor, () => DialogResult.Yes, () => + { + editor.Close(); + + Assert.True(editor.IsDisposed); + Assert.True(File.GetLastWriteTimeUtc(path) > savedAt.AddMinutes(-1)); + }); + } + finally + { + File.Delete(path); + } + }, TimeSpan.FromMinutes(1), "The STA test did not complete."); + + private static void DeleteFirstPlatesParts(EditNestForm editor) + { + editor.PlateManager.LoadAt(0); + editor.PlateView.SelectAll(); + editor.PlateView.Selection.DeleteSelected(); + } + + private static string TempNestPath(string name) + { + var directory = Path.Combine(Path.GetTempPath(), "OpenNest.WinForms.Tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(directory); + return Path.Combine(directory, name + ".nest"); + } + + // Answers for this test's window only; other windows closing meanwhile keep the default. + private static void WithPrompt(EditNestForm editor, Func answer, Action body) + { + var previous = EditNestForm.UnsavedChangesPrompt; + EditNestForm.UnsavedChangesPrompt = form => ReferenceEquals(form, editor) ? answer() : previous(form); + try + { + body(); + } + finally + { + EditNestForm.UnsavedChangesPrompt = previous; + } + } +} diff --git a/OpenNest.WinForms.Tests/IsolatedUserSettings.cs b/OpenNest.WinForms.Tests/IsolatedUserSettings.cs index 8c421137..864c2f15 100644 --- a/OpenNest.WinForms.Tests/IsolatedUserSettings.cs +++ b/OpenNest.WinForms.Tests/IsolatedUserSettings.cs @@ -1,4 +1,6 @@ using System.Runtime.CompilerServices; +using System.Windows.Forms; +using OpenNest.Forms; using OpenNest.Properties; namespace OpenNest.WinForms.Tests; @@ -7,8 +9,12 @@ internal static class IsolatedUserSettings { #pragma warning disable CA2255 // Must run before anything reads Settings.Default. [ModuleInitializer] - internal static void Redirect() => + internal static void Redirect() + { AppDataSettingsProvider.PathOverride = Path.Combine( Path.GetTempPath(), "OpenNest.WinForms.Tests", Guid.NewGuid().ToString("N"), "user-settings.json"); + // A closing nest window never waits on the unsaved-changes message box. + EditNestForm.UnsavedChangesPrompt = _ => DialogResult.No; + } #pragma warning restore CA2255 } diff --git a/OpenNest/Forms/Commands/NestStorageController.cs b/OpenNest/Forms/Commands/NestStorageController.cs index fd5d6f99..d6e71a54 100644 --- a/OpenNest/Forms/Commands/NestStorageController.cs +++ b/OpenNest/Forms/Commands/NestStorageController.cs @@ -136,11 +136,13 @@ namespace OpenNest.Forms.Commands renamed = chosen != originalName; form.Nest.Name = chosen; } + // The save marks the history's current state saved, so it must include every edit. + form.RecordUnrecordedChange(); var saved = await form.Document.SaveToDatabaseAsync( repository, serverUrl, saveCopy); if (!form.IsDisposed) { - form.Text = form.Document.Name; + form.UpdateTitle(); _ = form.HoldStatus.RefreshAfterSaveAsync(); } if (!owner.IsDisposed) diff --git a/OpenNest/Forms/EditNestForm.Undo.cs b/OpenNest/Forms/EditNestForm.Undo.cs index 5ff02bd5..db61219b 100644 --- a/OpenNest/Forms/EditNestForm.Undo.cs +++ b/OpenNest/Forms/EditNestForm.Undo.cs @@ -98,10 +98,22 @@ namespace OpenNest.Forms RestoreNest(restored); } + /// + /// True when the nest differs from the state last saved (or opened): the history has + /// moved off its save point, or an edit no scope recorded is pending. + /// + public bool HasUnsavedChanges => + History.Current != null + && (!History.IsAtSavePoint || NestFingerprint.Compute(Nest) != recordedFingerprint); + + /// Shows the nest's name, marked with an asterisk while it has unsaved changes. + internal void UpdateTitle() => Text = HasUnsavedChanges ? $"{Nest.Name} *" : Nest.Name; + private void InitializeHistory() { History.Reset(new NestUndoState(NestSnapshot.Capture(Nest))); recordedFingerprint = NestFingerprint.Compute(Nest); + History.Changed += (_, _) => UpdateTitle(); #if DEBUG Application.Idle += CheckForUnrecordedChange; FormClosed += (_, _) => Application.Idle -= CheckForUnrecordedChange; @@ -123,13 +135,15 @@ namespace OpenNest.Forms { History.Commit(label, new NestUndoState(NestSnapshot.Capture(Nest)), coalesceKey); recordedFingerprint = NestFingerprint.Compute(Nest); + UpdateTitle(); } /// /// Commits an edit no scope recorded as its own entry, so a rollback, Undo or Redo - /// does not silently discard it. Detected by fingerprint, so best effort. + /// does not silently discard it, and a save marks it saved. Detected by fingerprint, so + /// best effort. /// - private void RecordUnrecordedChange() + internal void RecordUnrecordedChange() { if (History.Current != null && NestFingerprint.Compute(Nest) != recordedFingerprint) Commit("Change", null); @@ -210,6 +224,7 @@ namespace OpenNest.Forms } recordedFingerprint = NestFingerprint.Compute(Nest); + UpdateTitle(); NestRestored?.Invoke(this, EventArgs.Empty); } diff --git a/OpenNest/Forms/EditNestForm.cs b/OpenNest/Forms/EditNestForm.cs index 1ec08db5..702d1484 100644 --- a/OpenNest/Forms/EditNestForm.cs +++ b/OpenNest/Forms/EditNestForm.cs @@ -500,7 +500,7 @@ namespace OpenNest.Forms UpdateDrawingList(); UpdateRemovePlateButton(); - Text = Nest.Name; + UpdateTitle(); drawingListBox1.Units = Nest.Units; drawingListBox1.Plate = PlateManager.CurrentPlate; } @@ -599,6 +599,8 @@ namespace OpenNest.Forms public void SaveAs(string path) { + // The save marks the history's current state saved, so it must include every edit. + RecordUnrecordedChange(); try { Document.SaveAs(path); @@ -610,7 +612,7 @@ namespace OpenNest.Forms "Save Nest", MessageBoxButtons.OK, MessageBoxIcon.Error); return; } - Text = Document.Name; + UpdateTitle(); } public void Import() @@ -874,7 +876,7 @@ namespace OpenNest.Forms } if (form.UseForNewNests) MachineSettings.SaveNestDefaults(this, NestDefaults.FromNest(Nest), Nest.Machine); - Text = Nest.Name; + UpdateTitle(); drawingListBox1.Units = Nest.Units; drawingListBox1.Invalidate(); if (statusBefore != NestStatus.HasBeenCut && Nest.Status == NestStatus.HasBeenCut) @@ -1252,13 +1254,65 @@ namespace OpenNest.Forms } } base.OnClosing(e); + if (!e.Cancel && !ConfirmCloseWithUnsavedChanges()) + e.Cancel = true; + } + /// + /// Asks whether to save the nest's unsaved changes before closing (Yes, No or Cancel); + /// tests replace it so a closing window never waits on a message box. + /// + internal static Func UnsavedChangesPrompt = form => + MessageBox.Show(form, $"Save changes to '{form.Nest.Name}' before closing?", + "Unsaved Changes", MessageBoxButtons.YesNoCancel, MessageBoxIcon.Warning); + + // Whether the window may close now. A save that has to wait on the nest server cancels + // this close and closes the window once the nest is saved. + private bool ConfirmCloseWithUnsavedChanges() + { + if (!HasUnsavedChanges) + return true; + switch (UnsavedChangesPrompt(this)) + { + case DialogResult.No: + return true; + case DialogResult.Yes: + var save = SaveChangesAsync(); + if (save.IsCompleted) + return !HasUnsavedChanges; + _ = CloseWhenSavedAsync(save); + return false; + default: + return false; + } + } + + // The main window saves to the nest storage in use; a window without one saves to a file. + private Task SaveChangesAsync() + { + if (MdiParent is MainForm main) + return main.SaveNestAsync(this); + Save(); + return Task.CompletedTask; + } + + private async Task CloseWhenSavedAsync(Task save) + { + await save; + if (!IsDisposed && !HasUnsavedChanges) + Close(); + } + + protected override void OnFormClosed(FormClosedEventArgs e) + { + // Released here, not while closing: another window can still cancel closing the app. PlateManager.Dispose(); Settings.Default.PlateViewDrawBounds = PlateView.DrawBounds; Settings.Default.PlateViewDrawRapid = PlateView.DrawRapid; Settings.Default.SplitterDistance = splitContainer.SplitterDistance; Settings.Default.Save(); + base.OnFormClosed(e); } protected override void OnResize(EventArgs e) diff --git a/OpenNest/Forms/MainForm.File.cs b/OpenNest/Forms/MainForm.File.cs index fb5a5aed..5e6a7076 100644 --- a/OpenNest/Forms/MainForm.File.cs +++ b/OpenNest/Forms/MainForm.File.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Linq; +using System.Threading.Tasks; using System.Windows.Forms; using OpenNest.Data; using OpenNest.Engine.BestFit; @@ -230,8 +231,14 @@ namespace OpenNest.Forms private async void Save_Click(object sender, EventArgs e) { - var form = activeForm; - if (form == null || storage.IsSaving || RefuseSaveDuringRelease(form)) + if (activeForm != null) + await SaveNestAsync(activeForm); + } + + /// Saves 's nest to the nest storage in use. + internal async Task SaveNestAsync(EditNestForm form) + { + if (storage.IsSaving || RefuseSaveDuringRelease(form)) return; if (NestStorage.Settings.Mode == NestStorageMode.File) form.Save(); diff --git a/docs/undo-redo.md b/docs/undo-redo.md index 99b26adf..e75c53c4 100644 --- a/docs/undo-redo.md +++ b/docs/undo-redo.md @@ -48,6 +48,10 @@ Deliberately excluded: A save does not clear the history; it marks a save point (`UndoHistory.MarkSaved`, `IsAtSavePoint`). A database save marks it only when no edit was recorded while it ran. +A save first commits any edit no scope recorded, so the point covers what was written. +`EditNestForm.HasUnsavedChanges` is true off the save point or while such an edit is pending: +the window title then ends in ` *`, and closing asks Save, Don't Save or Cancel. Yes saves to +the storage in use; a database save cancels the close and closes the window once it succeeds. ## Recording rule