From bbde1ed6939df208562c4e091270500afc8d7612 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Tue, 13 Oct 2026 10:47:48 -0400 Subject: [PATCH 1/2] LT-22691: Answer the Delete leaf of the Lexicon detail menus natively In the Avalonia detail view, Delete Translation and Delete Complex Form Info now run from the row itself: the Lexicon menu authority answers every DataTreeDelete leaf, so mnuDataTree-Translation and mnuDataTree-ComplexFormSpec no longer build the hidden DataTree. After the delete, the re-show gives keyboard focus to the nearest surviving row, the one before the deleted object's rows when it survives. Co-Authored-By: Claude Fable 5.1 --- Src/Common/Controls/DetailControls/Slice.cs | 20 +- Src/Common/FwAvalonia/Detail/DataTree.cs | 78 ++++- .../FwAvalonia/Detail/DetailFocusMemory.cs | 34 ++- Src/Common/FwAvalonia/Detail/DetailModel.cs | 7 + Src/Common/FwAvalonia/DetailHostControl.cs | 12 + .../FwAvaloniaTests/DetailFocusMemoryTests.cs | 57 ++++ Src/FdoUi/DetailRules/DetailObjectDeletion.cs | 59 ++++ Src/FdoUi/FdoUiCore.cs | 46 ++- .../DetailRules/DetailObjectDeletionTests.cs | 113 ++++++++ .../Hosting/RecordEditView.Avalonia.cs | 11 +- .../Hosting/RecordEditView.LexiconMenus.cs | 142 ++++++++++ Src/xWorks/DTMenuHandler.cs | 14 +- .../Hosting/LexiconMenuAuthorityTests.cs | 268 +++++++++++++++++- .../Hosting/LexiconMenuExecutionTests.cs | 225 +++++++++++++++ 14 files changed, 1010 insertions(+), 76 deletions(-) create mode 100644 Src/FdoUi/DetailRules/DetailObjectDeletion.cs create mode 100644 Src/FdoUi/FdoUiTests/DetailRules/DetailObjectDeletionTests.cs create mode 100644 Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs create mode 100644 Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuExecutionTests.cs diff --git a/Src/Common/Controls/DetailControls/Slice.cs b/Src/Common/Controls/DetailControls/Slice.cs index 4ac0328e7e..59c98a00f8 100644 --- a/Src/Common/Controls/DetailControls/Slice.cs +++ b/Src/Common/Controls/DetailControls/Slice.cs @@ -2478,25 +2478,7 @@ public bool GetCanDeleteNow() { CheckDisposed(); - var obj = GetObjectForMenusToOperateOn(); - if (obj == null) - return false; - - var owner = obj.Owner; - if (owner == null) // We can allow unowned objects to be deleted. - return true; - int flid = obj.OwningFlid; - if (!owner.IsFieldRequired(flid)) - return true; - - //now, if the field is required, then we do not allow this to be deleted if it is atomic - //futureTodo: this prevents the user from the deleting something in order to create something - //of a different class, or to paste in other object in this field. - if (!Cache.IsVectorProperty(flid)) - return false; - - // still OK to delete so long as it is not the last item. - return Cache.DomainDataByFlid.get_VecSize(owner.Hvo, flid) > 1; + return DetailObjectDeletion.CanDeleteNow(GetObjectForMenusToOperateOn()); } /// diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 1e9d05085b..365c5303d9 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -79,6 +79,56 @@ public sealed class DataTree : UserControl, IDetailPopupSink, IDisposable /// for a gesture (a menu-driven move) made while no editor had focus. /// public void RequestVectorFocusOnRebuild() => VectorFocusRequested = true; + + // Each row's focus targets by its automation id (a header row's "editor" is the + // header itself), for a focus requested across a rebuild. + private readonly Dictionary _rowFocusTargets = + new Dictionary(StringComparer.Ordinal); + + /// The automation ids of the rows the rebuild that follows should try to + /// focus, in order; null when none was requested. + internal IReadOnlyList RowFocusRequested { get; private set; } + + /// + /// Asks the rebuild that follows to give keyboard focus to the first of + /// it shows, for a gesture that removed the + /// focused row and may have removed its neighbours with it. + /// + public void RequestRowFocusOnRebuild(IReadOnlyList rowAutomationIds) + => RowFocusRequested = rowAutomationIds; + + /// + /// Focuses the first row of that is shown and + /// can take focus; false when none can. + /// + internal bool FocusFirstRow(IEnumerable rowAutomationIds) + { + if (rowAutomationIds == null) + return false; + foreach (var id in rowAutomationIds) + { + if (FocusRow(id)) + return true; + } + return false; + } + + /// + /// Focuses the row with as a label gesture does, + /// falling back to its field-menu button when the editor is hidden (a collapsed + /// section); false when no such row is shown or neither can take focus. + /// + internal bool FocusRow(string rowAutomationId) + { + if (rowAutomationId == null || !_rowFocusTargets.TryGetValue(rowAutomationId, out var row)) + return false; + var target = FocusTargetOf(row.Editor, row.Kebab); + if (target == null || !target.IsEffectivelyVisible) + target = row.Kebab; + if (target == null || !target.IsEffectivelyVisible) + return false; + return target.Focus(); + } private readonly IFwClipboard _clipboard; // Computed once per view (not per field/row) from the widest WS abbreviation across the @@ -571,7 +621,7 @@ private struct FieldContent private FieldContent AddField(int row, DetailField field) { - var automationId = string.IsNullOrEmpty(field.AutomationId) ? field.StableId : field.AutomationId; + var automationId = field.RowAutomationId; var indent = new Thickness(field.Indent * 12, 0, 0, 0); // Section/group headers from full-layout composition span the full item width @@ -631,6 +681,7 @@ private FieldContent AddField(int row, DetailField field) // "..." field-menu button (in a thin gutter to the left of the header) // opens the section menu/hotlinks. var headerCell = WrapWithFieldMenu(header, field, automationId, out var headerKebab); + _rowFocusTargets[automationId] = (header, headerKebab); // Discoverability parity (legacy SummaryCommandControl): a section header with hotlinks // shows its commands as an ALWAYS-VISIBLE inline command-link strip directly beneath the @@ -726,6 +777,7 @@ private FieldContent AddField(int row, DetailField field) // gutter "..." button; the editor's current item rides each request it raises. var labelCell = WrapWithFieldMenu(labelBlock, field, automationId, out var labelKebab, editor as IDetailItemSelection, editor); + _rowFocusTargets[automationId] = (editor, labelKebab); // Hover-reveal: the WHOLE row (label cell + editor) is the hover/focus // surface for the field-options "..." and any editor affordance (chooser @@ -855,17 +907,21 @@ private static void TakeRowFocus(Control cell, Control editor, Control kebab) var focused = TopLevel.GetTopLevel(cell)?.FocusManager?.GetFocusedElement(); if (IsOn(cell, focused) || IsOn(editor, focused)) return; - Control target; + FocusTargetOf(editor, kebab)?.Focus(); + } + + // The control a row's gesture focuses: a vector row its button (focusing an item would + // make it current), else the editor or its first focusable part; null when none. + private static Control FocusTargetOf(Control editor, Control kebab) + { + if (editor == null) + return null; if (editor is IDetailItemSelection) - target = kebab; - else if (editor.Focusable) - target = editor; - else - { - target = Avalonia.VisualTree.VisualExtensions.GetVisualDescendants(editor).OfType() - .FirstOrDefault(c => c.Focusable && c.IsEffectivelyEnabled && c.IsEffectivelyVisible); - } - target?.Focus(); + return kebab; + if (editor.Focusable) + return editor; + return Avalonia.VisualTree.VisualExtensions.GetVisualDescendants(editor).OfType() + .FirstOrDefault(c => c.Focusable && c.IsEffectivelyEnabled && c.IsEffectivelyVisible); } /// diff --git a/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs b/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs index e2b570ff36..45bfc7aaeb 100644 --- a/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs +++ b/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; using Avalonia.Automation; using Avalonia.Controls; using Avalonia; @@ -30,7 +31,7 @@ public sealed class Memento { public Memento(string automationId, int caretIndex, double verticalOffset = 0, string vectorAutomationId = null, string vectorItemKey = null, int vectorItemIndex = -1, - bool focusVectorItem = false) + bool focusVectorItem = false, IReadOnlyList rowAutomationIds = null) { AutomationId = automationId; CaretIndex = caretIndex; @@ -39,8 +40,16 @@ public Memento(string automationId, int caretIndex, double verticalOffset = 0, VectorItemKey = vectorItemKey; VectorItemIndex = vectorItemIndex; FocusVectorItem = focusVectorItem; + RowAutomationIds = rowAutomationIds; } + /// + /// The automation ids of the rows the outgoing view asked to try focusing after + /// the rebuild, in order (a gesture removed the focused row and maybe its + /// neighbours); null when none. + /// + public IReadOnlyList RowAutomationIds { get; } + /// /// True when the outgoing view asked for the vector row's current item to take /// keyboard focus after the rebuild (a gesture made from a menu, with no editor @@ -84,10 +93,11 @@ public static Memento Capture(Control root) var vectorKey = selectedVector?.SelectedItemKey; var vectorIndex = selectedVector?.SelectedItemIndex ?? -1; var focusVector = tree?.VectorFocusRequested ?? false; + var focusRow = tree?.RowFocusRequested; var focusManager = TopLevel.GetTopLevel(root)?.FocusManager; if (!(focusManager?.GetFocusedElement() is Control focused) || !root.IsVisualAncestorOf(focused)) - return new Memento(null, -1, verticalOffset, vectorId, vectorKey, vectorIndex, focusVector); + return new Memento(null, -1, verticalOffset, vectorId, vectorKey, vectorIndex, focusVector, focusRow); // The editor itself carries the stable id (e.g. "LexemeFormEditor.vern"); walk up in // case focus landed on an inner template part. @@ -97,11 +107,11 @@ public static Memento Capture(Control root) if (!string.IsNullOrEmpty(id)) { return new Memento(id, (focused as TextBox)?.CaretIndex ?? -1, verticalOffset, - vectorId, vectorKey, vectorIndex, focusVector); + vectorId, vectorKey, vectorIndex, focusVector, focusRow); } } - return new Memento(null, -1, verticalOffset, vectorId, vectorKey, vectorIndex, focusVector); + return new Memento(null, -1, verticalOffset, vectorId, vectorKey, vectorIndex, focusVector, focusRow); } // Whether the rebuilt view should focus the vector row's current item: the outgoing view @@ -207,9 +217,10 @@ public static bool TryRestoreScroll(Control root, Memento memento) /// /// Restores the memento once the incoming view has laid out: the scroll offset first (an /// offset set before layout is clamped to zero by the still-empty extent), then the - /// vector row's current item, then focus -- the remembered editor, or else that current - /// item when the memento asks for it, so a move or removal made from the row keeps the - /// keyboard in the row. Safe to call before the view is attached. + /// vector row's current item, then focus -- the remembered editor, else that current + /// item when the memento asks for it (a move or removal made from the row keeps the + /// keyboard in the row), else the first surviving row the outgoing view asked for (a + /// gesture removed the focused row). Safe to call before the view is attached. /// public static void RestoreAfterLayout(Control root, Memento memento) { @@ -229,8 +240,13 @@ public static void RestoreAfterLayout(Control root, Memento memento) if (needsScroll) TryRestoreScroll(root, memento); var vector = RestoreVectorSelection(root, memento); - if (!TryRestoreFocus(root, memento) && vector != null && WantsVectorFocus(memento)) - vector.FocusItemAt(vector.SelectedItemIndex); + if (!TryRestoreFocus(root, memento)) + { + if (vector != null && WantsVectorFocus(memento)) + vector.FocusItemAt(vector.SelectedItemIndex); + else if (memento.RowAutomationIds != null) + (root as DataTree)?.FocusFirstRow(memento.RowAutomationIds); + } // A bring-into-view request queued by a focus change is applied by the next // arrange; the remembered offset is re-asserted after it. if (needsScroll) diff --git a/Src/Common/FwAvalonia/Detail/DetailModel.cs b/Src/Common/FwAvalonia/Detail/DetailModel.cs index 464b9238fd..cda6f2b25e 100644 --- a/Src/Common/FwAvalonia/Detail/DetailModel.cs +++ b/Src/Common/FwAvalonia/Detail/DetailModel.cs @@ -1624,6 +1624,13 @@ public DetailField( public string GhostPrompt { get; } public string StableId { get; } + + /// + /// The automation id of the row built for this field: its own when the layout gives + /// one, else its stable id. + /// + public string RowAutomationId => string.IsNullOrEmpty(AutomationId) ? StableId : AutomationId; + public string Label { get; } public string Field { get; } public string WritingSystem { get; } diff --git a/Src/Common/FwAvalonia/DetailHostControl.cs b/Src/Common/FwAvalonia/DetailHostControl.cs index 66e7f5fba6..350a5b0682 100644 --- a/Src/Common/FwAvalonia/DetailHostControl.cs +++ b/Src/Common/FwAvalonia/DetailHostControl.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.Common.FwAvalonia.Seams; @@ -75,5 +76,16 @@ public void ShowDetail(DetailModel detail, IDetailEditContext editContext = null /// gesture (a menu-driven move) made while no editor had focus. /// public void FocusVectorItemOnNextShow() => (CurrentContent as DataTree)?.RequestVectorFocusOnRebuild(); + + /// + /// Asks the next re-show to give keyboard focus to the first of + /// it still shows, for a gesture that removed the + /// focused row. + /// + public void FocusRowOnNextShow(IReadOnlyList rowAutomationIds) + => (CurrentContent as DataTree)?.RequestRowFocusOnRebuild(rowAutomationIds); + + /// The detail model shown now; null before the first show. + public DetailModel ShownDetail => (CurrentContent as DataTree)?.Model; } } diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs index 41dc5defa0..9e3d108c5d 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs @@ -50,6 +50,63 @@ public IReadOnlyList GetValues(ViewNode fieldNode) private static DataTree NewView() => new DataTree(DetailModelProjector.FromViewDefinition(Definition(), new Provider())); + // The same view with its Gloss field gone, as a re-show after deleting that field's + // object would build it. + private static DataTree NewViewWithoutGloss() + { + var definition = Definition(); + var withoutGloss = new ViewDefinitionModel(definition.ClassName, definition.LayoutName, + definition.LayoutType, new List { definition.Roots[0] }, new List()); + return new DataTree(DetailModelProjector.FromViewDefinition(withoutGloss, new Provider())); + } + + // A gesture removed the focused row and named its neighbours, nearest first: with the + // remembered editor gone from the rebuilt view, the first surviving row takes focus. + [AvaloniaTest] + public void RequestedRowFocus_FocusesTheFirstSurvivingRow_WhenTheRememberedEditorIsGone() + { + var first = NewView(); + var window = new Window { Content = first, Width = 420, Height = 200 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + FindEditor(first, "GlossEditor.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + + first.RequestRowFocusOnRebuild(new[] { "RowThatAlsoVanishes", "LexemeFormEditor" }); + var memento = DetailFocusMemory.Capture(first); + Assert.That(memento.AutomationId, Is.EqualTo("GlossEditor.vern"), "precondition: the doomed editor had focus"); + Assert.That(memento.RowAutomationIds, Is.EqualTo(new[] { "RowThatAlsoVanishes", "LexemeFormEditor" })); + + var second = NewViewWithoutGloss(); + DetailFocusMemory.RestoreAfterLayout(second, memento); + window.Content = second; + Dispatcher.UIThread.RunJobs(); + + Assert.That(FindEditor(second, "GlossEditor.vern"), Is.Null, "precondition: the remembered editor is gone"); + Assert.That(FindEditor(second, "LexemeFormEditor.vern").IsFocused, Is.True, + "the requested row's editor takes focus"); + } + + // The remembered editor wins over a requested row when both survive the re-show. + [AvaloniaTest] + public void RequestedRowFocus_YieldsToTheRememberedEditor_WhenItSurvives() + { + var first = NewView(); + var window = new Window { Content = first, Width = 420, Height = 200 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + FindEditor(first, "GlossEditor.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + first.RequestRowFocusOnRebuild(new[] { "LexemeFormEditor" }); + + var second = NewView(); + DetailFocusMemory.RestoreAfterLayout(second, DetailFocusMemory.Capture(first)); + window.Content = second; + Dispatcher.UIThread.RunJobs(); + + Assert.That(FindEditor(second, "GlossEditor.vern").IsFocused, Is.True); + } + private static DataTree NewLongView() { var roots = new List(); diff --git a/Src/FdoUi/DetailRules/DetailObjectDeletion.cs b/Src/FdoUi/DetailRules/DetailObjectDeletion.cs new file mode 100644 index 0000000000..e95ba2301e --- /dev/null +++ b/Src/FdoUi/DetailRules/DetailObjectDeletion.cs @@ -0,0 +1,59 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using SIL.FieldWorks.Common.FwUtils; +using SIL.LCModel; + +// Rules both detail views share. Nothing here may depend on WinForms, Avalonia, or a slice +// or control type, so the rules outlive WinForms. DetailRulesBoundaryTests enforces this. +namespace SIL.FieldWorks.Common.DetailRules +{ + /// + /// The Delete command of a detail row: whether the row's object may be deleted now, and + /// the menu text that says so. Execution stays with CmObjectUi.DeleteUnderlyingObject. + /// + public static class DetailObjectDeletion + { + /// The strings-table key of the text appended to a disabled Delete + /// item. + public const string CannotDeleteKey = "(cannot delete this)"; + + /// + /// Whether may be deleted: always when unowned or in a field + /// its owner does not require; never from a required atomic field; from a required + /// vector only while it is not the last item. + /// + public static bool CanDeleteNow(ICmObject target) + { + if (target == null) + return false; + var owner = target.Owner; + if (owner == null) + return true; + var flid = target.OwningFlid; + if (!owner.IsFieldRequired(flid)) + return true; + if (!target.Cache.IsVectorProperty(flid)) + return false; + return target.Cache.DomainDataByFlid.get_VecSize(owner.Hvo, flid) > 1; + } + + /// + /// The Delete item's text: the configured label, with the cannot-delete note appended + /// when is false and {0} replaced by the target's + /// localized class name. + /// + /// The object to delete; null leaves any {0} in + /// place. + public static string DeleteLabel(string label, ICmObject target, bool canDelete) + { + var text = label ?? string.Empty; + if (!canDelete) + text += StringTable.Table.GetString(CannotDeleteKey); + if (text.Contains("{0}") && target != null) + text = string.Format(text, StringTable.Table.GetString(target.ClassName, "ClassNames")); + return text; + } + } +} diff --git a/Src/FdoUi/FdoUiCore.cs b/Src/FdoUi/FdoUiCore.cs index 015b30fa54..5c256bda7c 100644 --- a/Src/FdoUi/FdoUiCore.cs +++ b/Src/FdoUi/FdoUiCore.cs @@ -1022,8 +1022,22 @@ public virtual bool CanDelete(out string cannotDeleteMsg) /// Return true if deleted, false, if cancelled. /// public bool DeleteUnderlyingObject() + { + return DeleteUnderlyingObject(ConfirmDelete); + } + + /// + /// Deletes the object once says so. The clerk's current + /// record is instead handed to the DeleteRecord handler, confirmation included. + /// + /// Asks the user; in the + /// product. + /// True when the object was deleted here. + public bool DeleteUnderlyingObject(Func confirm) { CheckDisposed(); + if (confirm == null) + throw new ArgumentNullException(nameof(confirm)); ICmObject cmo = GetCurrentCmObject(); if (cmo != null && m_obj != null && cmo.Hvo == m_obj.Hvo) @@ -1038,24 +1052,34 @@ public bool DeleteUnderlyingObject() var mainWindow = m_propertyTable.GetValue
("window"); using (new WaitCursor(mainWindow)) { - using (var dlg = new ConfirmDeleteObjectDlg(m_propertyTable.GetValue("HelpTopicProvider"))) + if (confirm()) { - string cannotDeleteMsg; - if (CanDelete(out cannotDeleteMsg)) - dlg.SetDlgInfo(this, m_cache, Mediator, m_propertyTable); - else - dlg.SetDlgInfo(this, m_cache, Mediator, m_propertyTable, TsStringUtils.MakeString(cannotDeleteMsg, m_cache.DefaultUserWs)); - if (DialogResult.Yes == dlg.ShowDialog(mainWindow)) - { - ReallyDeleteUnderlyingObject(); - return true; // deleted it - } + ReallyDeleteUnderlyingObject(); + return true; // deleted it } } } return false; // didn't delete it. } + /// + /// Asks the user to confirm deleting the object, telling them when it cannot be + /// deleted; true when they confirm. + /// + public bool ConfirmDelete() + { + var mainWindow = m_propertyTable.GetValue("window"); + using (var dlg = new ConfirmDeleteObjectDlg(m_propertyTable.GetValue("HelpTopicProvider"))) + { + string cannotDeleteMsg; + if (CanDelete(out cannotDeleteMsg)) + dlg.SetDlgInfo(this, m_cache, Mediator, m_propertyTable); + else + dlg.SetDlgInfo(this, m_cache, Mediator, m_propertyTable, TsStringUtils.MakeString(cannotDeleteMsg, m_cache.DefaultUserWs)); + return DialogResult.Yes == dlg.ShowDialog(mainWindow); + } + } + /// /// Do any cleanup that involves interacting with the user, after the user has confirmed that our object should be /// deleted. diff --git a/Src/FdoUi/FdoUiTests/DetailRules/DetailObjectDeletionTests.cs b/Src/FdoUi/FdoUiTests/DetailRules/DetailObjectDeletionTests.cs new file mode 100644 index 0000000000..49a6815be4 --- /dev/null +++ b/Src/FdoUi/FdoUiTests/DetailRules/DetailObjectDeletionTests.cs @@ -0,0 +1,113 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using NUnit.Framework; +using SIL.FieldWorks.Common.DetailRules; +using SIL.FieldWorks.Common.FwUtils; +using SIL.LCModel; +using SIL.LCModel.Infrastructure; + +namespace SIL.FieldWorks.FdoUi +{ + /// + /// The Delete rule of a detail row: the required-field cases of + /// , and the item text. + /// + [TestFixture] + public class DetailObjectDeletionTests : MemoryOnlyBackendProviderTestBase + { + private ILexEntry _entry; + private ILexSense _sense; + private ICmTranslation _translation; + + // An entry with one sense, whose example has one translation. The entry has no entry + // refs, so LexEntry requires its Senses. + [SetUp] + public void MakeEntry() + { + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + _entry = Cache.ServiceLocator.GetInstance().Create(); + _sense = Cache.ServiceLocator.GetInstance().Create(); + _entry.SensesOS.Add(_sense); + var example = Cache.ServiceLocator.GetInstance().Create(); + _sense.ExamplesOS.Add(example); + var type = Cache.ServiceLocator.GetInstance() + .GetObject(CmPossibilityTags.kguidTranFreeTranslation); + _translation = Cache.ServiceLocator.GetInstance().Create(example, type); + }); + } + + [TearDown] + public void DeleteEntry() + { + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + if (_entry.IsValidObject) + _entry.Delete(); + }); + } + + [Test] + public void CanDeleteNow_AnUnownedObject() + { + Assert.That(_entry.Owner, Is.Null, "precondition"); + Assert.That(DetailObjectDeletion.CanDeleteNow(_entry), Is.True); + } + + [Test] + public void CanDeleteNow_AnObjectInAFieldItsOwnerDoesNotRequire() + { + Assert.That(_translation.Owner.IsFieldRequired(_translation.OwningFlid), Is.False, "precondition"); + Assert.That(DetailObjectDeletion.CanDeleteNow(_translation), Is.True); + } + + [Test] + public void CanDeleteNow_RefusesTheLastItemOfARequiredVector_AndAllowsOneOfTwo() + { + Assert.That(_entry.IsFieldRequired(LexEntryTags.kflidSenses), Is.True, + "precondition: an entry without entry refs requires its senses"); + Assert.That(DetailObjectDeletion.CanDeleteNow(_sense), Is.False, "the only sense stays"); + + ILexSense second = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + second = Cache.ServiceLocator.GetInstance().Create(); + _entry.SensesOS.Add(second); + }); + + Assert.That(DetailObjectDeletion.CanDeleteNow(_sense), Is.True); + Assert.That(DetailObjectDeletion.CanDeleteNow(second), Is.True); + } + + [Test] + public void CanDeleteNow_IsFalseForNoObject() + { + Assert.That(DetailObjectDeletion.CanDeleteNow(null), Is.False); + } + + [Test] + public void DeleteLabel_KeepsAnEnabledLabel_AndNotesADisabledOne() + { + var note = StringTable.Table.GetString(DetailObjectDeletion.CannotDeleteKey); + Assert.That(note, Is.Not.Empty, "precondition"); + + Assert.That(DetailObjectDeletion.DeleteLabel("Delete Translation", _translation, true), + Is.EqualTo("Delete Translation")); + Assert.That(DetailObjectDeletion.DeleteLabel("Delete Translation", _translation, false), + Is.EqualTo("Delete Translation" + note)); + Assert.That(DetailObjectDeletion.DeleteLabel(null, null, false), Is.EqualTo(note)); + } + + [Test] + public void DeleteLabel_NamesTheTargetsClass_WhereTheLabelAsksForIt() + { + var className = StringTable.Table.GetString("LexSense", "ClassNames"); + + Assert.That(DetailObjectDeletion.DeleteLabel("Delete {0}", _sense, true), Is.EqualTo("Delete " + className)); + Assert.That(DetailObjectDeletion.DeleteLabel("Delete {0}", null, true), Is.EqualTo("Delete {0}"), + "no target, nothing to name"); + } + } +} diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index dbf01a0b72..aa364dd393 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -751,16 +751,7 @@ internal IDetailMenuAuthority CreateMenuAuthority(DetailMenuRequest request) new EnvironmentInsertMenuAuthority(request, this), CreateMultiStringMenuAuthority(request.Field, objectMenu), objectMenu, - CreateLexiconMenuAuthority()); - } - - /// - /// The message-keyed authority for the Lexicon menus, owning by predicate. - /// - internal LexiconMenuAuthority CreateLexiconMenuAuthority() - { - var window = m_propertyTable.GetValue("window"); - return new LexiconMenuAuthority(id => XCoreMenuBridge.ResolveMenu(window, id)); + CreateLexiconMenuAuthority(request.Field)); } /// The native authority for a multi-writing-system row's label menu. diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs new file mode 100644 index 0000000000..5018627c3e --- /dev/null +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs @@ -0,0 +1,142 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System; +using System.Collections.Generic; +using SIL.FieldWorks.Common.DetailRules; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.FdoUi; +using SIL.LCModel; +using SIL.Reporting; +using XCore; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// The host half of the Lexicon detail menus: the answerers given to + /// and the commands they run. + /// + public partial class RecordEditView + { + /// The message every Delete leaf of the detail menus sends. + internal const string DeleteMessage = "DataTreeDelete"; + + /// + /// Asks the user to confirm a delete: the product shows the object's confirmation + /// dialog; a headless test answers for the user. + /// + internal Func DeleteConfirmation { get; set; } = ui => ui.ConfirmDelete(); + + /// + /// The message-keyed authority for the Lexicon menus, owning by predicate and + /// answering for ; with no row every answer is hidden. + /// + internal LexiconMenuAuthority CreateLexiconMenuAuthority(DetailField field) + { + var window = m_propertyTable.GetValue("window"); + var authority = new LexiconMenuAuthority(id => XCoreMenuBridge.ResolveMenu(window, id)); + authority.Answer(DeleteMessage, leaf => DeleteItem(leaf, field)); + return authority; + } + + /// + /// The Delete item for the row's object: absent on a row with no object or a ghost + /// row; disabled when the object is gone from the repository or + /// refuses it. + /// + internal DetailMenuItem DeleteItem(DetailMenuLeaf leaf, DetailField field) + { + if (field == null || field.ObjectHvo == 0 || field.GhostPrompt != null) + return null; + var label = XCoreMenuBridge.StripAccelerator(leaf.Label); + if (!Cache.ServiceLocator.ObjectRepository.TryGetObject(field.ObjectHvo, out var target)) + { + Logger.WriteEvent("Delete is disabled: the row's object " + field.ObjectHvo + " no longer exists."); + return DetailMenuItem.Disabled(DetailObjectDeletion.DeleteLabel(label, null, false)); + } + var canDelete = DetailObjectDeletion.CanDeleteNow(target); + var text = DetailObjectDeletion.DeleteLabel(label, target, canDelete); + return new DetailMenuItem(text, isEnabled: canDelete, isChecked: false, children: null, + execute: canDelete ? (Action)(() => DeleteDetailObject(field)) : null); + } + + /// + /// Deletes the row's object after confirmation: pending edits settle first, the + /// object's UI runs the delete, and the re-show focuses the nearest surviving row, the + /// one before the deleted object's rows when it survives. + /// + /// True when the object was deleted. + internal bool DeleteDetailObject(DetailField field) + { + SettleDetailEdits(); + if (!Cache.ServiceLocator.ObjectRepository.TryGetObject(field.ObjectHvo, out var target)) + { + Logger.WriteEvent("Delete did nothing: the row's object " + field.ObjectHvo + " no longer exists."); + return false; + } + var focusRows = RowsToFocusAfterDeleting(m_avaloniaEntryForm?.ShownDetail, target); + bool deleted; + using (var ui = CmObjectUi.MakeUi(Cache, target.Hvo)) + { + ui.Mediator = m_mediator; + ui.PropTable = m_propertyTable; + deleted = ui.DeleteUnderlyingObject(() => DeleteConfirmation(ui)); + } + if (!deleted) + return false; + if (focusRows.Count > 0) + m_avaloniaEntryForm?.FocusRowOnNextShow(focusRows); + OnAvaloniaDetailEditCompleted(this, EventArgs.Empty); + return true; + } + + /// + /// The rows to try focusing once is deleted, by automation + /// id: the rows before its rows (its own and those of the objects it owns), nearest + /// first, then the rows after them, nearest first. The re-show takes the first that + /// still exists; a row can vanish with the object, e.g. a section header whose + /// sequence it emptied. + /// + internal static IReadOnlyList RowsToFocusAfterDeleting(DetailModel shown, ICmObject target) + { + var candidates = new List(); + if (shown == null || target == null) + return candidates; + var fields = shown.Fields; + var first = -1; + var last = -1; + for (var i = 0; i < fields.Count; i++) + { + if (!ShowsPartOf(fields[i], target)) + continue; + if (first < 0) + first = i; + last = i; + } + if (first < 0) + return candidates; + for (var i = first - 1; i >= 0; i--) + candidates.Add(fields[i].RowAutomationId); + for (var i = last + 1; i < fields.Count; i++) + candidates.Add(fields[i].RowAutomationId); + return candidates; + } + + // Whether the row shows the object or one it owns, directly or through other objects. + private static bool ShowsPartOf(DetailField field, ICmObject target) + { + if (field.ObjectHvo == 0 + || !target.Cache.ServiceLocator.ObjectRepository.TryGetObject(field.ObjectHvo, out var shown)) + { + return false; + } + for (var owned = shown; owned != null; owned = owned.Owner) + { + if (owned.Hvo == target.Hvo) + return true; + } + return false; + } + } +} diff --git a/Src/xWorks/DTMenuHandler.cs b/Src/xWorks/DTMenuHandler.cs index bac167b9fc..8622818bd5 100644 --- a/Src/xWorks/DTMenuHandler.cs +++ b/Src/xWorks/DTMenuHandler.cs @@ -975,18 +975,8 @@ public virtual bool OnDisplayDataTreeDelete(object commandObject, // display.Text += Table.GetString("(Programming error: would delete this record.)"); // } // else - if(!display.Enabled) - display.Text += StringTable.Table.GetString("(cannot delete this)"); - - if (display.Text.Contains("{0}")) - { - // Insert the class name of the thing we will delete - var obj = current.GetObjectForMenusToOperateOn(); - if (obj != null) - { - display.Text = string.Format(display.Text, StringTable.Table.GetString(obj.ClassName, "ClassNames")); - } - } + display.Text = DetailObjectDeletion.DeleteLabel(display.Text, current.GetObjectForMenusToOperateOn(), + display.Enabled); return true;//we handled this, no need to ask anyone else. } diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuAuthorityTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuAuthorityTests.cs index d8ac7ae5aa..9e35741a64 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuAuthorityTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuAuthorityTests.cs @@ -6,11 +6,19 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Xml; using NUnit.Framework; +using SIL.FieldWorks.Common.Controls; +using SIL.FieldWorks.Common.DetailRules; using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; using SIL.FieldWorks.Common.FwUtils; +using SIL.LCModel; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Infrastructure; using XCore; +using LegacyDataTree = SIL.FieldWorks.Common.Framework.DetailControls.DataTree; namespace SIL.FieldWorks.XWorks { @@ -24,8 +32,27 @@ namespace SIL.FieldWorks.XWorks [Apartment(System.Threading.ApartmentState.STA)] public class LexiconMenuAuthorityTests : XWorksAppTestBase { - // The ids the product authority owns today. Every answerer a PR adds extends this. - private static readonly string[] PinnedOwnedIds = new string[0]; + // The ids owned today: every leaf of each sends DataTreeDelete. Ownership is by message + // over the whole window configuration, so other areas' ids and unbound ids count too. + private static readonly string[] PinnedOwnedIds = + { + // Lexicon rows: Translation, and Complex Form Type on a complex form. + "mnuDataTree-Translation", "mnuDataTree-ComplexFormSpec", + // Lexicon ids no part reaches (-CmMedia's pronunciation part does not exist). + "mnuDataTree-CmMedia", "mnuDataTree-Variant", "mnuDataTree-DeleteEtymology", + "mnuDataTree-DeletePronunciation", + // Grammar: two posEdit rows (Inflection Class Name, Stem Allomorph Label), then + // tools without an Avalonia detail view. + "mnuDataTree-POS-InflectionClass", "mnuDataTree-POS-StemName", + "mnuDataTree-ClosedFeature-Value", "mnuDataTree-MoStemName-Region", "mnuDataTree-Phoneme-Code", + "mnuDataTree-Delete-Adhoc-Allomorph", "mnuDataTree-Delete-Adhoc-Group", + "mnuDataTree-Delete-Adhoc-Morpheme", + // Lists: no Avalonia detail view. + "mnuDataTree-DeleteCustomItem", "mnuDataTree-DeletePossibility", "mnuDataTree-DeleteQuestion" + }; + + // The ids a Lexicon Edit row raises, the rows the contracts below are proved on. + private static readonly string[] LexiconRowIds = { "mnuDataTree-Translation", "mnuDataTree-ComplexFormSpec" }; // The messages of mnuDataTree-Object's six leaves, the test subject for the predicate. private static readonly string[] ObjectMenuMessages = @@ -35,12 +62,19 @@ public class LexiconMenuAuthorityTests : XWorksAppTestBase }; private PropertyTable m_propertyTable; + private List m_createdObjects; + private ILexEntry m_entry; + private ICmTranslation m_translation; private RecordEditView m_view; protected override void Init() { m_application = new MockFwXApp(new MockFwManager { Cache = Cache }, null, null); m_configFilePath = Path.Combine(FwDirectoryFinder.CodeDirectory, m_application.DefaultConfigurationPathname); + // The hidden DataTree the equivalence baseline drives loads the layout Inventory, + // which is keyed by the project path. + Cache.ProjectId.Path = Path.Combine(Path.GetTempPath(), Cache.ProjectId.Name, + Cache.ProjectId.Name + ".junk"); } [SetUp] @@ -51,17 +85,34 @@ public void SetUpWindow() m_propertyTable = m_window.PropTable; m_propertyTable.RemoveLocalAndGlobalSettings(); m_window.LoadUI(m_configFilePath); + TestLocalizationManagerBootstrap.EnsureInitialized(); + TestLocalizationManagerBootstrap.EnsureHelpTopicProvider(m_propertyTable); + LayoutCache.InitializePartInventories(Cache.ProjectId.Name, m_application, Cache.ProjectId.Path); + m_createdObjects = new List(); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, CreateTestEntry); m_propertyTable.SetProperty("UIMode", "New", true); m_propertyTable.SetPropertyPersistence("UIMode", false); LoadRecordEditView("lexiconEdit"); DrainMediatorAndIdleQueues(); m_view = m_propertyTable.GetValue("currentContentControlObject", null) as RecordEditView; Assert.That(m_view, Is.Not.Null, "expected the lexicon edit RecordEditView to load"); + m_view.Clerk.JumpToRecord(m_entry.Hvo); + DrainMediatorAndIdleQueues(); } [TearDown] public void TearDownWindow() { + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + foreach (var obj in Enumerable.Reverse(m_createdObjects)) + { + if (obj.IsValidObject) + obj.Delete(); + } + }); + m_createdObjects = null; + m_entry = null; m_view = null; m_propertyTable?.RemoveLocalAndGlobalSettings(); m_propertyTable = null; @@ -77,7 +128,7 @@ public void TearDownWindow() [Test] public void OwnsExactlyThePinnedIds_AndAnswersEveryLeafOfEachOne() { - var authority = m_view.CreateLexiconMenuAuthority(); + var authority = m_view.CreateLexiconMenuAuthority(TranslationRow()); var window = Window; var owned = ContextMenuIds().Where(authority.Owns).ToList(); @@ -147,7 +198,7 @@ public void NeverOwns_AMenuWithoutLeaves_OrWithAListSubmenu_OrWithAPropertyToggl [Test] public void Build_RejectsALeafWhoseMessageHasNoAnswerer_AndEveryList() { - var authority = m_view.CreateLexiconMenuAuthority(); + var authority = m_view.CreateLexiconMenuAuthority(TranslationRow()); var help = XCoreMenuBridge.ResolveMenu(Window, RecordEditView.ObjectMenuId).Leaves .First(l => l.CommandId == ObjectMenuAuthority.HelpCommandId); @@ -157,8 +208,111 @@ public void Build_RejectsALeafWhoseMessageHasNoAnswerer_AndEveryList() MultiStringMenuAuthority.WritingSystemListId), Throws.InvalidOperationException); } + // Contract: on every Lexicon Edit row binding an owned id, the owned menu renders what + // the adapter path rendered; the row's other groups are proved by their own authorities. + [Test] + public void OwnedMenu_RendersWhatTheAdapterPathRendered_OnEveryLexiconRowThatBindsOne() + { + var rows = DetailComposer.Compose(m_entry, Cache).Model.Fields + .Where(f => LexiconRowIds.Contains(f.MenuId)).ToList(); + Assert.That(rows.Select(f => f.Label), Is.EquivalentTo(new[] { "Translations 1", "Translation", "Complex Form Type" }), + "precondition: the entry composes the rows the owned ids are bound to"); + + var mismatches = new List(); + foreach (var row in rows) + { + EnsureAdapter(row.ObjectHvo, row.Field); + Assert.That(AdapterTree.CurrentSlice?.Object?.Hvo, Is.EqualTo(row.ObjectHvo), + "precondition: the adapter targets the row's object for " + row.Label); + var ids = new[] { row.MenuId }; + var before = Describe(XCoreMenuBridge.CreateMenuItems(Window, ids, null, null, null)); + var after = Describe(XCoreMenuBridge.CreateMenuItems(Window, ids, null, null, + m_view.CreateMenuAuthority(LabelMenuRequest(row)))); + Assert.That(after, Does.Contain("enabled=True checked=False executes=True"), + row.Label + ": Delete is offered and runs"); + if (!string.Equals(before, after, StringComparison.Ordinal)) + { + mismatches.Add(string.Format("{0} ({1}):{2}--- adapter path{2}{3}{2}--- authority{2}{4}", + row.Label, row.Field, Environment.NewLine, before, after)); + } + } + Assert.That(mismatches, Is.Empty, string.Join(Environment.NewLine, mismatches)); + } + + // Contract: an owned label menu never asks the mediator and never builds the adapter. + [Test] + public void OwnedLabelMenus_AreBuiltWithoutTheAdapterOrTheMediator() + { + Assert.That(HiddenTreeExists, Is.False, "precondition: no hidden tree yet"); + var rows = DetailComposer.Compose(m_entry, Cache).Model.Fields + .Where(f => LexiconRowIds.Contains(f.MenuId)).ToList(); + Assert.That(rows, Has.Count.EqualTo(3), "precondition"); + foreach (var row in rows) + { + var ids = RecordEditView.ComposeSliceMenuIds(row.MenuId, row.IsMultiStringRow).ToArray(); + var authority = m_view.CreateMenuAuthority(LabelMenuRequest(row)); + Assert.That(XCoreMenuBridge.OwnsAll(authority, ids), Is.True, row.Label + ": every id of the label menu is owned"); + + var items = BuildWithSpy(ids, authority, out var asked); + + Assert.That(asked, Is.False, row.Label + ": the mediator was asked"); + Assert.That(items.Any(i => i.Label.StartsWith("Delete", StringComparison.Ordinal)), Is.True); + } + Assert.That(HiddenTreeExists, Is.False, "an owned menu never builds the hidden tree"); + } + + [Test] + public void DeleteLeaf_IsAbsent_WithoutARow_OnAGhostRow_AndOnARowWithoutAnObject() + { + var leaf = DeleteTranslationLeaf(); + var translation = TranslationRow(); + var ghost = new DetailField(translation.StableId + DetailField.GhostStableIdSuffix, "Translation", "Translation", + null, DetailFieldKind.Text, EditorClassification.Known, null, null, HostRouting.Inherit, null, null, null, + menuId: translation.MenuId, objectHvo: translation.ObjectHvo, ghostPrompt: "Add a translation"); + var noObject = new DetailField("Row", "Row", "Translation", null, DetailFieldKind.Text, + EditorClassification.Known, null, null, HostRouting.Inherit, null, null, null, menuId: translation.MenuId); + + Assert.That(m_view.DeleteItem(leaf, null), Is.Null, "no row"); + Assert.That(m_view.DeleteItem(leaf, ghost), Is.Null, "a ghost row has no object to delete"); + Assert.That(m_view.DeleteItem(leaf, noObject), Is.Null, "no object"); + Assert.That(m_view.DeleteItem(leaf, translation)?.IsEnabled, Is.True, "the real row offers it"); + } + + [Test] + public void DeleteLeaf_IsDisabled_WhenTheObjectIsGone_AndWhenTheRuleRefuses() + { + var leaf = DeleteTranslationLeaf(); + var translation = TranslationRow(); + var note = StringTable.Table.GetString(DetailObjectDeletion.CannotDeleteKey); + var gone = new DetailField(translation.StableId, "Translation", "Translation", null, DetailFieldKind.Text, + EditorClassification.Known, null, null, HostRouting.Inherit, null, null, null, + menuId: translation.MenuId, objectHvo: int.MaxValue - 1); + // A sense of an entry without entry refs sits in a required field: the only one + // stays. + ILexEntry plain = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + plain = AddLexeme(m_createdObjects, "plain", GetMorphTypeOrCreateOne("stem"), "plain gloss", + GetGrammaticalCategoryOrCreateOne("noun", Cache.LangProject.PartsOfSpeechOA))); + var onlySense = new DetailField("Sense", "Sense", "Senses", null, DetailFieldKind.Header, + EditorClassification.GroupingNone, null, null, HostRouting.Inherit, null, null, null, + menuId: translation.MenuId, objectHvo: plain.SensesOS[0].Hvo); + + var goneItem = m_view.DeleteItem(leaf, gone); + var refused = m_view.DeleteItem(leaf, onlySense); + + Assert.That(goneItem.IsEnabled, Is.False); + Assert.That(goneItem.Execute, Is.Null); + Assert.That(goneItem.Label, Is.EqualTo("Delete Translation" + note)); + Assert.That(refused.IsEnabled, Is.False, "the rule refuses the last item of a required field"); + Assert.That(refused.Label, Does.EndWith(note)); + } + private XWindow Window => m_propertyTable.GetValue("window"); + private bool HiddenTreeExists => (bool)GetField(m_view, "m_dataTreeInitialized"); + + private LegacyDataTree AdapterTree => (LegacyDataTree)GetField(m_view, "m_dataEntryForm"); + private LexiconMenuAuthority NewAuthority() { var window = Window; @@ -168,6 +322,16 @@ private LexiconMenuAuthority NewAuthority() private static DetailMenuItem Echo(DetailMenuLeaf leaf) => new DetailMenuItem(XCoreMenuBridge.StripAccelerator(leaf.Label), isEnabled: true); + private static DetailMenuRequest LabelMenuRequest(DetailField field) + => DetailMenuRequest.FromAnchor(null, field, DetailMenuKind.SliceMenu); + + private DetailField TranslationRow() + => DetailComposer.Compose(m_entry, Cache).Model.Fields + .Single(f => f.Field == "Translation" && f.ObjectHvo == m_translation.Hvo); + + private DetailMenuLeaf DeleteTranslationLeaf() + => XCoreMenuBridge.ResolveMenu(Window, "mnuDataTree-Translation").Leaves.Single(); + // Every context-menu id the window configuration defines, each once. private IEnumerable ContextMenuIds() { @@ -177,6 +341,95 @@ private IEnumerable ContextMenuIds() .Select(n => n.Attributes["id"].Value).Distinct(StringComparer.Ordinal).ToList(); } + private IReadOnlyList BuildWithSpy(string[] ids, IDetailMenuAuthority authority, + out bool mediatorAsked) + { + var window = Window; + var spy = new DisplaySpyColleague(); + window.Mediator.AddColleague(spy); + try + { + var items = XCoreMenuBridge.CreateMenuItems(window, ids, null, null, authority); + mediatorAsked = spy.Asked; + return items; + } + finally + { + window.Mediator.RemoveColleague(spy); + } + } + + // Answers no display query, only records that one arrived. + private sealed class DisplaySpyColleague : IxCoreColleague + { + public bool Asked { get; private set; } + public void Init(Mediator mediator, PropertyTable propertyTable, XmlNode configurationParameters) { } + public IxCoreColleague[] GetMessageTargets() => new IxCoreColleague[] { this }; + public bool ShouldNotCall => false; + public int Priority => (int)ColleaguePriority.High; + public bool OnDisplayDataTreeDelete(object commandObject, ref UIItemDisplayProperties display) + { + Asked = true; + return false; + } + public bool OnDisplayShowFieldAlwaysVisible(object commandObject, ref UIItemDisplayProperties display) + { + Asked = true; + return false; + } + } + + private static string Describe(IReadOnlyList items, string indent = "") + { + var lines = new List(); + foreach (var item in items) + { + if (item.IsSeparator) + { + lines.Add(indent + "---"); + continue; + } + lines.Add(string.Format("{0}{1} [enabled={2} checked={3} executes={4}]", indent, item.Label, + item.IsEnabled, item.IsChecked, item.Execute != null)); + if (item.Children.Count > 0) + lines.Add(Describe(item.Children, indent + " ")); + } + return string.Join(Environment.NewLine, lines); + } + + private void EnsureAdapter(int targetHvo, string fieldName) + { + var method = typeof(RecordEditView).GetMethod("EnsureMenuCommandAdapter", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, "EnsureMenuCommandAdapter must exist (adapter targeting seam)"); + method.Invoke(m_view, new object[] { targetHvo, fieldName }); + } + + // An entry with a sense whose example has a free translation, made a complex form of + // another entry so its own LexEntryRef shows the Complex Form Type row. + private void CreateTestEntry() + { + var stemMorphType = GetMorphTypeOrCreateOne("stem"); + var noun = GetGrammaticalCategoryOrCreateOne("noun", Cache.LangProject.PartsOfSpeechOA); + m_entry = AddLexeme(m_createdObjects, "command-entry", stemMorphType, "first gloss", noun); + var example = Cache.ServiceLocator.GetInstance().Create(); + m_entry.SensesOS[0].ExamplesOS.Add(example); + example.Example.set_String(Cache.DefaultVernWs, TsStringUtils.MakeString("una casa", Cache.DefaultVernWs)); + var type = Cache.ServiceLocator.GetInstance() + .GetObject(CmPossibilityTags.kguidTranFreeTranslation); + m_translation = Cache.ServiceLocator.GetInstance().Create(example, type); + m_translation.Translation.set_String(Cache.DefaultAnalWs, TsStringUtils.MakeString("a house", Cache.DefaultAnalWs)); + + var component = AddLexeme(m_createdObjects, "part", stemMorphType, "part gloss", noun); + var complexFormRef = Cache.ServiceLocator.GetInstance().Create(); + m_entry.EntryRefsOS.Add(complexFormRef); + complexFormRef.RefType = LexEntryRefTags.krtComplexForm; + complexFormRef.ComponentLexemesRS.Add(component); + complexFormRef.PrimaryLexemesRS.Add(component); + complexFormRef.ComplexEntryTypesRS.Add(Cache.ServiceLocator.GetInstance() + .GetObject(LexEntryTypeTags.kguidLexTypCompound)); + } + private void LoadRecordEditView(string toolValue) { var windowConfiguration = m_propertyTable.GetValue("WindowConfiguration"); @@ -190,5 +443,12 @@ private void LoadRecordEditView(string toolValue) m_propertyTable.SetProperty("currentContentControl", toolValue, true); m_propertyTable.SetPropertyPersistence("currentContentControl", false); } + + private static object GetField(object target, string fieldName) + { + var field = target.GetType().GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(field, Is.Not.Null, "Missing private field: " + fieldName); + return field.GetValue(target); + } } } diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuExecutionTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuExecutionTests.cs new file mode 100644 index 0000000000..c0e27f6a6f --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/LexiconMenuExecutionTests.cs @@ -0,0 +1,225 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Xml; +using NUnit.Framework; +using SIL.FieldWorks.Common.Controls; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; +using SIL.FieldWorks.Common.FwUtils; +using SIL.LCModel; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Infrastructure; +using XCore; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// End-to-end proof that the Lexicon menu commands the native authority answers change + /// the model through the product : the item's own Execute runs + /// the host, which asks for confirmation, edits in one undoable step and tells the re-show + /// where to put the keyboard. + /// + [TestFixture] + [Apartment(System.Threading.ApartmentState.STA)] + public class LexiconMenuExecutionTests : XWorksAppTestBase + { + private PropertyTable m_propertyTable; + private List m_createdObjects; + private ILexEntry m_entry; + private ILexExampleSentence m_example; + private ICmTranslation m_translation; + private ILexEntryRef m_complexFormRef; + private RecordEditView m_view; + + protected override void Init() + { + m_application = new MockFwXApp(new MockFwManager { Cache = Cache }, null, null); + m_configFilePath = Path.Combine(FwDirectoryFinder.CodeDirectory, m_application.DefaultConfigurationPathname); + Cache.ProjectId.Path = Path.Combine(Path.GetTempPath(), Cache.ProjectId.Name, + Cache.ProjectId.Name + ".junk"); + } + + [SetUp] + public void SetUpWindow() + { + m_window = new MockFwXWindow(m_application, m_configFilePath); + ((MockFwXWindow)m_window).Init(Cache); + m_propertyTable = m_window.PropTable; + m_propertyTable.RemoveLocalAndGlobalSettings(); + m_window.LoadUI(m_configFilePath); + TestLocalizationManagerBootstrap.EnsureInitialized(); + TestLocalizationManagerBootstrap.EnsureHelpTopicProvider(m_propertyTable); + LayoutCache.InitializePartInventories(Cache.ProjectId.Name, m_application, Cache.ProjectId.Path); + m_createdObjects = new List(); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, CreateTestEntry); + m_propertyTable.SetProperty("UIMode", "New", true); + m_propertyTable.SetPropertyPersistence("UIMode", false); + LoadRecordEditView("lexiconEdit"); + DrainMediatorAndIdleQueues(); + m_view = m_propertyTable.GetValue("currentContentControlObject", null) as RecordEditView; + Assert.That(m_view, Is.Not.Null, "expected the lexicon edit RecordEditView to load"); + m_view.Clerk.JumpToRecord(m_entry.Hvo); + DrainMediatorAndIdleQueues(); + } + + [TearDown] + public void TearDownWindow() + { + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + foreach (var obj in Enumerable.Reverse(m_createdObjects)) + { + if (obj.IsValidObject) + obj.Delete(); + } + }); + m_createdObjects = null; + m_entry = null; + m_view = null; + m_propertyTable?.RemoveLocalAndGlobalSettings(); + m_propertyTable = null; + if (m_window != null && !m_window.IsDisposed) + { + m_window.Dispose(); + m_window = null; + } + } + + [Test] + public void DeleteTranslation_RemovesIt_InOneUndoableStep_WhenTheUserConfirms() + { + var confirmed = 0; + m_view.DeleteConfirmation = ui => + { + Assert.That(ui.Object.Hvo, Is.EqualTo(m_translation.Hvo), "the confirmation names the row's object"); + confirmed++; + return true; + }; + var translationHvo = m_translation.Hvo; + + InvokeDelete(TranslationRow(), "Delete Translation"); + + Assert.That(confirmed, Is.EqualTo(1)); + Assert.That(Cache.ServiceLocator.ObjectRepository.IsValidObjectId(translationHvo), Is.False, "the translation is gone"); + Assert.That(m_example.TranslationsOC, Is.Empty); + Assert.That(DetailComposer.Compose(m_entry, Cache).Model.Fields.Any(f => f.ObjectHvo == translationHvo), Is.False, + "the recomposed detail view has no row for it"); + + Assert.That(Cache.ActionHandlerAccessor.CanUndo(), Is.True); + Cache.ActionHandlerAccessor.Undo(); + Assert.That(m_example.TranslationsOC, Has.Count.EqualTo(1), "one Undo brings the translation back"); + } + + [Test] + public void DeleteTranslation_LeavesTheModelAlone_WhenTheUserDeclines() + { + m_view.DeleteConfirmation = ui => false; + + InvokeDelete(TranslationRow(), "Delete Translation"); + + Assert.That(m_translation.IsValidObject, Is.True); + Assert.That(m_example.TranslationsOC, Has.Count.EqualTo(1)); + } + + [Test] + public void DeleteComplexFormInfo_RemovesTheEntryRef() + { + m_view.DeleteConfirmation = ui => true; + var row = DetailComposer.Compose(m_entry, Cache).Model.Fields + .Single(f => f.MenuId == "mnuDataTree-ComplexFormSpec"); + Assert.That(row.ObjectHvo, Is.EqualTo(m_complexFormRef.Hvo), "precondition: the row shows the entry's own ref"); + + InvokeDelete(row, "Delete Complex Form Info"); + + Assert.That(m_entry.EntryRefsOS, Is.Empty); + Assert.That(m_entry.IsValidObject, Is.True, "only the ref goes, not the entry"); + } + + // Candidates: the rows before the object's rows (its own and its descendants'), nearest + // first, then the rows after. The header before a last translation dies with it. + [Test] + public void RowsToFocusAfterDeleting_AreTheNeighbours_BeforeThenAfter_NearestFirst() + { + var model = DetailComposer.Compose(m_entry, Cache).Model; + var fields = model.Fields.ToList(); + var ownedByTranslation = fields.Where(f => f.ObjectHvo == m_translation.Hvo).ToList(); + var first = fields.IndexOf(ownedByTranslation.First()); + var last = fields.IndexOf(ownedByTranslation.Last()); + Assert.That(first, Is.GreaterThan(1), "precondition: rows precede the translation's"); + Assert.That(fields[first - 1].Field, Is.EqualTo("Translations"), "precondition: the section header is just before"); + Assert.That(fields[first - 2].ObjectHvo, Is.EqualTo(m_example.Hvo), "precondition: the example's row is next"); + + var candidates = RecordEditView.RowsToFocusAfterDeleting(model, m_translation); + + var expected = Enumerable.Range(0, first).Reverse().Select(i => fields[i].RowAutomationId) + .Concat(Enumerable.Range(last + 1, fields.Count - last - 1).Select(i => fields[i].RowAutomationId)); + Assert.That(candidates, Is.EqualTo(expected.ToList())); + Assert.That(RecordEditView.RowsToFocusAfterDeleting(model, m_example)[0], + Is.EqualTo(fields[fields.FindIndex(f => f.ObjectHvo == m_example.Hvo) - 1].RowAutomationId), + "the translation's rows belong to the example that owns it"); + Assert.That(RecordEditView.RowsToFocusAfterDeleting(model, m_entry), Is.Empty, + "every row belongs to the entry, so nothing is left to focus"); + Assert.That(RecordEditView.RowsToFocusAfterDeleting(null, m_translation), Is.Empty); + } + + // Builds the row's label menu as the host does and runs the named item's own Execute. + private void InvokeDelete(DetailField row, string label) + { + var ids = RecordEditView.ComposeSliceMenuIds(row.MenuId, row.IsMultiStringRow).ToArray(); + var window = m_propertyTable.GetValue("window"); + var request = DetailMenuRequest.FromAnchor(null, row, DetailMenuKind.SliceMenu); + var items = XCoreMenuBridge.CreateMenuItems(window, ids, null, null, m_view.CreateMenuAuthority(request)); + var item = items.FirstOrDefault(i => i.Label == label); + Assert.That(item, Is.Not.Null, "the menu offers '{0}'", label); + Assert.That(item.Execute, Is.Not.Null, "'{0}' is enabled", label); + item.Execute(); + DrainMediatorAndIdleQueues(); + } + + private DetailField TranslationRow() + => DetailComposer.Compose(m_entry, Cache).Model.Fields + .Single(f => f.Field == "Translation" && f.ObjectHvo == m_translation.Hvo); + + private void CreateTestEntry() + { + var stemMorphType = GetMorphTypeOrCreateOne("stem"); + var noun = GetGrammaticalCategoryOrCreateOne("noun", Cache.LangProject.PartsOfSpeechOA); + m_entry = AddLexeme(m_createdObjects, "command-entry", stemMorphType, "first gloss", noun); + m_example = Cache.ServiceLocator.GetInstance().Create(); + m_entry.SensesOS[0].ExamplesOS.Add(m_example); + m_example.Example.set_String(Cache.DefaultVernWs, TsStringUtils.MakeString("una casa", Cache.DefaultVernWs)); + var type = Cache.ServiceLocator.GetInstance() + .GetObject(CmPossibilityTags.kguidTranFreeTranslation); + m_translation = Cache.ServiceLocator.GetInstance().Create(m_example, type); + m_translation.Translation.set_String(Cache.DefaultAnalWs, TsStringUtils.MakeString("a house", Cache.DefaultAnalWs)); + + var component = AddLexeme(m_createdObjects, "part", stemMorphType, "part gloss", noun); + m_complexFormRef = Cache.ServiceLocator.GetInstance().Create(); + m_entry.EntryRefsOS.Add(m_complexFormRef); + m_complexFormRef.RefType = LexEntryRefTags.krtComplexForm; + m_complexFormRef.ComponentLexemesRS.Add(component); + m_complexFormRef.PrimaryLexemesRS.Add(component); + m_complexFormRef.ComplexEntryTypesRS.Add(Cache.ServiceLocator.GetInstance() + .GetObject(LexEntryTypeTags.kguidLexTypCompound)); + } + + private void LoadRecordEditView(string toolValue) + { + var windowConfiguration = m_propertyTable.GetValue("WindowConfiguration"); + Assert.That(windowConfiguration, Is.Not.Null); + var controlNode = windowConfiguration.SelectSingleNode( + string.Format("//tool[@value='{0}']/control//control[dynamicloaderinfo/@class='SIL.FieldWorks.XWorks.RecordEditView']", toolValue)); + Assert.That(controlNode, Is.Not.Null, "Expected the RecordEditView configuration node for tool '{0}'.", toolValue); + + m_propertyTable.SetProperty("currentContentControlParameters", controlNode, true); + m_propertyTable.SetPropertyPersistence("currentContentControlParameters", false); + m_propertyTable.SetProperty("currentContentControl", toolValue, true); + m_propertyTable.SetPropertyPersistence("currentContentControl", false); + } + } +} From 09fc9c678a34892444d2ab9ea173992f128c2940 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:05:35 -0400 Subject: [PATCH 2/2] LT-22691: Minor cleanup in response to review feedback Co-Authored-By: Claude Opus 5.5 --- Src/Common/FwAvalonia/Detail/DataTree.cs | 1 + Src/FdoUi/FdoUiCore.cs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 365c5303d9..621a0afffc 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -563,6 +563,7 @@ private void RebuildItems() editor.Dispose(); _vectors.Clear(); _labelBlocks.Clear(); + _rowFocusTargets.Clear(); SelectedVector = null; var visible = DetailVisibility.ComputeVisibility(Model.Fields, GetRecordedExpansion); for (var i = 0; i < Model.Fields.Count; i++) diff --git a/Src/FdoUi/FdoUiCore.cs b/Src/FdoUi/FdoUiCore.cs index 5c256bda7c..8055dae156 100644 --- a/Src/FdoUi/FdoUiCore.cs +++ b/Src/FdoUi/FdoUiCore.cs @@ -1068,6 +1068,8 @@ public bool DeleteUnderlyingObject(Func confirm) /// public bool ConfirmDelete() { + CheckDisposed(); + var mainWindow = m_propertyTable.GetValue("window"); using (var dlg = new ConfirmDeleteObjectDlg(m_propertyTable.GetValue("HelpTopicProvider"))) {