diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs index 227779c159..db2eb9cfdb 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs @@ -1127,7 +1127,8 @@ public void GhostRow_WatermarkClearsOnFocus_AndRestoresWhenLeftEmpty() isEditable: true, indent: 0, ghostPrompt: "Click here to add Lexeme Form"); var model = new DetailModel("LexEntry", "Normal", new List { ghost }, new List()); - var view = new DataTree(model, new FakeDetailEditContext()); + var context = new FakeDetailEditContext(); + var view = new DataTree(model, context); var window = new Window { Content = view, Width = 480, Height = 200 }; window.Show(); Dispatcher.UIThread.RunJobs(); @@ -1144,6 +1145,9 @@ public void GhostRow_WatermarkClearsOnFocus_AndRestoresWhenLeftEmpty() Dispatcher.UIThread.RunJobs(); Assert.That(box.Watermark, Is.EqualTo("Click here to add Lexeme Form"), "leaving without typing restores the prompt"); + Assert.That(context.TextEdits, Is.Empty, + "focusing and leaving the ghost row stages nothing, so nothing is created"); + Assert.That(context.CommitCount, Is.Zero, "with nothing staged there is nothing to save"); } // A search-backed reference vector (SearchOptions diff --git a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs index 809100f393..6e3e020906 100644 --- a/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs +++ b/Src/xWorks/Avalonia/Composer/ComposedDetailEditContext.cs @@ -7,7 +7,6 @@ using System.Linq; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.LCModel; -using SIL.LCModel.Core.KernelInterfaces; namespace SIL.FieldWorks.XWorks { @@ -82,8 +81,15 @@ public override bool TrySetText(DetailField field, string ws, string value) var setter = Handler(field)?.Text; if (setter == null) return false; + + if (string.IsNullOrEmpty(value) && field.GhostPrompt != null + && IsSessionOwnedBy(field.StableId) && CanRollBackSession) + { + Cancel(); + return true; + } // A single field's edit names the undo label (e.g. "Undo change to Gloss"). - return Stage(() => setter(ws, value), FieldLabelFor(field)); + return Stage(() => setter(ws, value), FieldLabelFor(field), field.StableId); } public override bool TrySetRichText(DetailField field, string ws, DetailRichTextValue value) diff --git a/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs b/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs index e2b51b175b..84f7a3ba07 100644 --- a/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs +++ b/Src/xWorks/Avalonia/Composer/LcmDetailEditSession.cs @@ -43,6 +43,12 @@ public LcmDetailEditSession(LcmCache cache, string undoLabel, string redoLabel) /// public bool IsOpen { get; private set; } + /// + /// Whether would roll back this session's writes: it is open and + /// owns its undo task, which is still the open one. + /// + public bool CanRollBack => IsOpen && _ownsTask && TaskStillOpen; + /// public void Commit() { diff --git a/Src/xWorks/Avalonia/DetailEditContextBase.cs b/Src/xWorks/Avalonia/DetailEditContextBase.cs index 721d201110..8b3802eed9 100644 --- a/Src/xWorks/Avalonia/DetailEditContextBase.cs +++ b/Src/xWorks/Avalonia/DetailEditContextBase.cs @@ -21,6 +21,7 @@ namespace SIL.FieldWorks.XWorks public abstract class DetailEditContextBase : IDetailEditContext { private LcmDetailEditSession _session; + private string _sessionOwnerKey; private readonly Dictionary> _pendingEditFlushes = new Dictionary>(StringComparer.Ordinal); @@ -154,15 +155,31 @@ public bool FlushPendingEdits() /// public void Commit() { + _sessionOwnerKey = null; _session?.Commit(); } /// public void Cancel() { + _sessionOwnerKey = null; _session?.Cancel(); } + /// + /// Whether every write in the open session was staged under . + /// False when no session is open or the key is null. + /// + protected bool IsSessionOwnedBy(string ownerKey) + => IsOpen && ownerKey != null + && string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal); + + /// + /// Whether would roll back the open session's writes. False when the + /// session joined an undo task opened elsewhere, which only that task's opener can end. + /// + protected bool CanRollBackSession => _session != null && _session.CanRollBack; + /// /// Opens the fenced session on the first staged edit; later calls are no-ops. The generic /// "Undo/Redo Edit Entry" label is used -- the batch/bulk path that stages several fields @@ -230,6 +247,15 @@ public bool Stage(Func setter) /// already-open session leaves that session's label untouched. /// public bool Stage(Func setter, string fieldLabel) + { + return Stage(setter, fieldLabel, null); + } + + /// + /// As , recording a write that stages under + /// for . + /// + protected bool Stage(Func setter, string fieldLabel, string ownerKey) { if (setter == null) return false; @@ -238,14 +264,25 @@ public bool Stage(Func setter, string fieldLabel) try { var staged = setter(); - if (!staged && !wasOpen) + if (staged) + { + if (!wasOpen) + _sessionOwnerKey = ownerKey; + else if (!string.Equals(_sessionOwnerKey, ownerKey, StringComparison.Ordinal)) + _sessionOwnerKey = null; + } + else if (!wasOpen) + { Cancel(); + } return staged; } catch { if (!wasOpen) Cancel(); + else + _sessionOwnerKey = null; // a throwing setter may have written before it threw throw; } } diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index c87ba390d5..09e914f471 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs @@ -1555,6 +1555,203 @@ public void Compose_GhostWithoutGhostField_IsNotEditable_SoTypingCannotCreateAnd "no bare object was created behind the user's back"); } + private static DetailField ExampleGhost(ComposedDetail composed, ILexSense sense) + => composed.Model.Fields.FirstOrDefault(f => f.Field == "Examples" + && f.StableId.EndsWith(DetailField.GhostStableIdSuffix) && f.ObjectHvo == sense.Hvo); + + // The layout gives SubSenses no ghost= attribute; LT-22690 approves its ghost row. + private static DetailField SubsenseGhost(ComposedDetail composed, ILexSense sense) + => composed.Model.Fields.FirstOrDefault(f => f.Field == "Senses" + && f.StableId.EndsWith(DetailField.GhostStableIdSuffix) && f.ObjectHvo == sense.Hvo); + + // One stage per keystroke, as the view does. + private static void TypeInto(ComposedDetail composed, DetailField field, string text) + { + for (var length = 1; length <= text.Length; length++) + { + Assert.That(composed.EditContext.TrySetText(field, field.Values[0].WsAbbrev, + text.Substring(0, length)), Is.True, "each keystroke stages"); + } + } + + [Test] + public void Compose_GhostExample_TypingCreatesTheExample_AndRecomposesIntoItsRows() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = ExampleGhost(composed, sense); + Assert.That(ghost, Is.Not.Null, "a sense with no examples shows the Example ghost row"); + Assert.That(ghost.IsEditable, Is.True); + Assert.That(ghost.GhostPrompt, Does.Contain("Example")); + + TypeInto(composed, ghost, "una casa"); + composed.EditContext.Commit(); + + Assert.That(sense.ExamplesOS.Count, Is.EqualTo(1), "typing created the example"); + var example = sense.ExamplesOS[0]; + Assert.That(example.Example.get_String(Cache.DefaultVernWs).Text, Is.EqualTo("una casa"), + "the typed text landed in the Example field, in the vernacular writing system"); + + var recomposed = DetailComposer.Compose(m_entry, Cache); + Assert.That(ExampleGhost(recomposed, sense), Is.Null, + "with an example present the ghost row gives way to the example's own rows"); + var exampleRow = recomposed.Model.Fields.FirstOrDefault(f => + f.Field == "Example" && f.ObjectHvo == example.Hvo); + Assert.That(exampleRow, Is.Not.Null, "the new example composes its Example row"); + Assert.That(exampleRow.Values.Any(v => v.Value == "una casa"), Is.True, + "the Example row shows the typed text"); + } + + [Test] + public void Compose_GhostExample_CreationAndTyping_AreOneUndoStep() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var stepsBefore = Cache.ActionHandlerAccessor.UndoableSequenceCount; + + TypeInto(composed, ExampleGhost(composed, sense), "una casa"); + composed.EditContext.Commit(); + Assert.That(Cache.ActionHandlerAccessor.UndoableSequenceCount, Is.EqualTo(stepsBefore + 1), + "creating the example and every keystroke typed into it are one undo step"); + + Cache.ActionHandlerAccessor.Undo(); + Assert.That(sense.ExamplesOS.Count, Is.Zero, "one undo removes the example"); + Assert.That(ExampleGhost(DetailComposer.Compose(m_entry, Cache), sense), Is.Not.Null, + "after the undo the sense shows its Example ghost row again"); + } + + [Test] + public void Compose_GhostExample_TypedThenCleared_CreatesNothing() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = ExampleGhost(composed, sense); + var stepsBefore = Cache.ActionHandlerAccessor.UndoableSequenceCount; + + TypeInto(composed, ghost, "un"); + Assert.That(sense.ExamplesOS.Count, Is.EqualTo(1), "the first keystroke creates the example"); + Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, "u"), Is.True); + Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""), Is.True, + "clearing the row is accepted"); + var sessionOpenAfterClear = composed.EditContext.IsOpen; + var examplesAfterClear = sense.ExamplesOS.Count; + // Close the session first, so a failure cannot break the next test's setup. + composed.EditContext.Commit(); + + Assert.That(sessionOpenAfterClear, Is.False, "clearing the row ends its edit session"); + Assert.That(examplesAfterClear, Is.Zero, "clearing the row removes the example it created"); + Assert.That(Cache.ActionHandlerAccessor.UndoableSequenceCount, Is.EqualTo(stepsBefore), + "nothing reaches the undo stack, not even a step that changes nothing"); + + TypeInto(composed, ghost, "b"); + composed.EditContext.Commit(); + Assert.That(sense.ExamplesOS.Count, Is.EqualTo(1), "typing again creates the example afresh"); + Assert.That(sense.ExamplesOS[0].Example.get_String(Cache.DefaultVernWs).Text, Is.EqualTo("b")); + } + + [Test] + public void Compose_GhostExample_ClearedAfterAnotherRowsEdit_KeepsThatEdit() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var gloss = composed.Model.Fields.First(f => f.Field == "Gloss" && f.ObjectHvo == sense.Hvo); + var ghost = ExampleGhost(composed, sense); + + var glossStaged = composed.EditContext.TrySetText(gloss, gloss.Values[0].WsAbbrev, "home"); + var typedStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, "u"); + var clearStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""); + var sessionOpenAfterClear = composed.EditContext.IsOpen; + // Close the session first, so a failure cannot break the next test's setup. + composed.EditContext.Commit(); + + Assert.That(glossStaged && typedStaged && clearStaged, Is.True, "every edit stages"); + Assert.That(sessionOpenAfterClear, Is.True, + "the session holds another row's edit, so clearing the ghost row does not cancel it"); + Assert.That(sense.Gloss.get_String(Cache.DefaultAnalWs).Text, Is.EqualTo("home"), + "the other row's edit is saved"); + } + + [Test] + public void Compose_GhostExample_ClearedAfterAnotherRowsRejectedEdit_CreatesNothing() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var gloss = composed.Model.Fields.First(f => f.Field == "Gloss" && f.ObjectHvo == sense.Hvo); + var ghost = ExampleGhost(composed, sense); + + TypeInto(composed, ghost, "u"); + var glossStaged = composed.EditContext.TrySetText(gloss, "no-such-ws", "home"); + var clearStaged = composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""); + var sessionOpenAfterClear = composed.EditContext.IsOpen; + var examplesAfterClear = sense.ExamplesOS.Count; + // Close the session first, so a failure cannot break the next test's setup. + composed.EditContext.Commit(); + + Assert.That(glossStaged, Is.False, "precondition: the gloss row rejects an unknown writing system"); + Assert.That(clearStaged, Is.True); + Assert.That(sessionOpenAfterClear, Is.False, + "a rejected edit wrote nothing, so the session still holds only the ghost row's edits"); + Assert.That(examplesAfterClear, Is.Zero, "clearing the row removes the example it created"); + } + + // An undo task opened before the row is typed into belongs to its opener, and cancelling + // the edit session cannot roll it back. + [Test] + public void Compose_GhostExample_ClearedInsideAnOuterUndoTask_KeepsNoTypedText() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = ExampleGhost(composed, sense); + + Cache.DomainDataByFlid.BeginUndoTask("outer", "outer"); + try + { + TypeInto(composed, ghost, "u"); + Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, ""), Is.True, + "clearing the row is accepted"); + composed.EditContext.Commit(); + } + finally + { + Cache.DomainDataByFlid.EndUndoTask(); + } + + Assert.That(sense.ExamplesOS.Select(e => e.Example.get_String(Cache.DefaultVernWs).Text ?? ""), + Has.All.Empty, "the text the user cleared is not saved by the outer task"); + } + + [Test] + public void Compose_GhostSubsense_TypingCreatesASubsense_WithTheTypedGloss() + { + var sense = m_entry.SensesOS[0]; + var composed = DetailComposer.Compose(m_entry, Cache); + var ghost = SubsenseGhost(composed, sense); + Assert.That(ghost, Is.Not.Null, "a sense with no subsenses shows the SubSenses ghost row"); + Assert.That(ghost.IsEditable, Is.True); + var stepsBefore = Cache.ActionHandlerAccessor.UndoableSequenceCount; + + TypeInto(composed, ghost, "small house"); + composed.EditContext.Commit(); + + Assert.That(sense.SensesOS.Count, Is.EqualTo(1), "typing created the subsense"); + var subsense = sense.SensesOS[0]; + Assert.That(subsense.Gloss.get_String(Cache.DefaultAnalWs).Text, Is.EqualTo("small house"), + "the typed text became the subsense's gloss"); + Assert.That(Cache.ActionHandlerAccessor.UndoableSequenceCount, Is.EqualTo(stepsBefore + 1), + "creating the subsense and typing its gloss are one undo step"); + + var recomposed = DetailComposer.Compose(m_entry, Cache); + Assert.That(SubsenseGhost(recomposed, sense), Is.Null, + "with a subsense present the ghost row gives way to the subsense's own rows"); + Assert.That(recomposed.Model.Fields.Any(f => f.Field == "Gloss" && f.ObjectHvo == subsense.Hvo), + Is.True, "the new subsense composes its Gloss row"); + + Cache.ActionHandlerAccessor.Undo(); + Assert.That(sense.SensesOS.Count, Is.Zero, "one undo removes the subsense"); + Assert.That(SubsenseGhost(DetailComposer.Compose(m_entry, Cache), sense), Is.Not.Null, + "after the undo the sense shows its SubSenses ghost row again"); + } + // Conditional display: the real shipped variant/complex-form // divergence. LexEntryRef/Normal's VariantEntryTypes and ComplexEntryTypes parts are // twins (LexEntryParts.xml:1133-1162); exactly one may