From 561dd40394ce65c17661e794a3f6d50917c384f7 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:32:36 -0500 Subject: [PATCH 01/12] First pass at LT-22673 --- Src/Common/FwAvalonia/Detail/DetailModel.cs | 9 +- .../FwAvalonia/Detail/FwFieldControls.cs | 20 +- .../Detail/FwReversalEntriesField.cs | 365 ++++++++++++ .../Detail/IReversalEntryEditing.cs | 39 ++ Src/Common/FwAvalonia/Detail/SliceFactory.cs | 7 +- Src/Common/FwAvalonia/FwAvaloniaStrings.cs | 11 + Src/Common/FwAvalonia/FwAvaloniaStrings.resx | 7 + .../Detail/FwReversalEntriesFieldTests.cs | 344 +++++++++++ .../DetailCustomFieldRenderingTests.cs | 29 +- .../FwAvaloniaTests/SliceFactoryTests.cs | 29 +- .../Avalonia/Composer/DetailComposer.cs | 33 +- .../Plugins/ReversalIndexEntryPlugin.cs | 403 +++++++++---- Src/xWorks/Avalonia/Plugins/SlicePlugins.cs | 35 +- .../Composer/DetailEditContextEditingTests.cs | 80 --- .../Composer/ReversalEntriesComposeTests.cs | 562 ++++++++++++++++++ .../Hosting/DetailWritingSystemStateTests.cs | 5 +- .../Plugins/LexemeEditorInventoryTests.cs | 24 +- 17 files changed, 1767 insertions(+), 235 deletions(-) create mode 100644 Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs create mode 100644 Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs create mode 100644 Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs create mode 100644 Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs diff --git a/Src/Common/FwAvalonia/Detail/DetailModel.cs b/Src/Common/FwAvalonia/Detail/DetailModel.cs index f6a02160aa..e623d4bf9e 100644 --- a/Src/Common/FwAvalonia/Detail/DetailModel.cs +++ b/Src/Common/FwAvalonia/Detail/DetailModel.cs @@ -1580,7 +1580,7 @@ public DetailField( int objectHvo = 0, string ghostPrompt = null, IReadOnlyList items = null, - Func controlFactory = null, + Func controlFactory = null, Func> searchOptions = null, IReadOnlyList chooserLinks = null, IReadOnlyList paragraphs = null, @@ -1795,10 +1795,11 @@ public DetailField( /// /// For a row: the /// deferred control factory the claiming plugin supplied via the composer. The view invokes - /// it at render time and places the returned control in the value column; null (or a - /// failing factory) renders the unsupported row instead. Null for every other kind. + /// it at render time with the host's render context (edit context and callbacks, never + /// null) and places the returned control in the value column; null (or a failing + /// factory) renders the unsupported row instead. Null for every other kind. /// - public Func ControlFactory { get; } + public Func ControlFactory { get; } /// /// For a row whose targets are searched rather diff --git a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs index 92b74cbfd7..7b64574941 100644 --- a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs +++ b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs @@ -114,7 +114,7 @@ private void AddValueRow(DetailField field, string automationId, bool showWritingSystemAbbreviation, DetailWsValue value, double wsAbbrevColumnWidth) { var currentRich = value.RichText; - var abbrev = CreateWsAbbrev(value, wsAbbrevColumnWidth); + var abbrev = CreateWsAbbrev(value.WsAbbrev, wsAbbrevColumnWidth); // Values render flat with no box/fill, and go read-only with a tooltip // -- instead of corrupting on the first keystroke -- when a run carries @@ -758,11 +758,11 @@ private void AddValueRow(DetailField field, string automationId, // own fixed gutter column (see the row Grid below) so a bold vernacular value can never crowd // or overlap it. ClipToBounds keeps an unusually long abbreviation inside the gutter width // rather than bleeding into the value column. - private static TextBlock CreateWsAbbrev(DetailWsValue value, double wsAbbrevColumnWidth) + internal static TextBlock CreateWsAbbrev(string wsAbbrev, double wsAbbrevColumnWidth) { var abbrev = new TextBlock { - Text = value.WsAbbrev, + Text = wsAbbrev, MinWidth = wsAbbrevColumnWidth, VerticalAlignment = VerticalAlignment.Top, Margin = new Thickness(0, 1, FwAvaloniaDensity.WsAbbrevGutter, 0), @@ -771,7 +771,7 @@ private static TextBlock CreateWsAbbrev(DetailWsValue value, double wsAbbrevColu ClipToBounds = true }; // The gutter clips a long abbreviation, so the full text stays discoverable on hover. - ToolTip.SetTip(abbrev, value.WsAbbrev); + ToolTip.SetTip(abbrev, wsAbbrev); return abbrev; } @@ -1884,7 +1884,15 @@ public void DiscardUnstagedText() private void AddSeparatorBar() { - var bar = new Border + var bar = CreateSeparatorBar(); + Children.Add(bar); + _affordances.Add(bar); + } + + /// The thin vertical bar drawn between the items of an inline list. + internal static Border CreateSeparatorBar() + { + return new Border { Width = FwAvaloniaDensity.SeparatorBarWidth, Height = FwAvaloniaDensity.IconGlyphSize, @@ -1892,8 +1900,6 @@ private void AddSeparatorBar() Margin = FwAvaloniaDensity.SeparatorBarMargin, VerticalAlignment = VerticalAlignment.Center }; - Children.Add(bar); - _affordances.Add(bar); } } diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs new file mode 100644 index 0000000000..621fe4bc97 --- /dev/null +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -0,0 +1,365 @@ +// 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 Avalonia; +using Avalonia.Automation; +using Avalonia.Controls; +using Avalonia.Controls.Documents; +using Avalonia.Input; +using Avalonia.Interactivity; +using Avalonia.Layout; +using Avalonia.Media; + +namespace SIL.FieldWorks.Common.FwAvalonia.Detail +{ + /// + /// One of a reversal entry's forms in a writing system other than its index's own. + /// + public sealed class DetailReversalAlternative + { + /// Creates the alternative. + /// The writing system's abbreviation; empty shows the form + /// alone. + /// The form. + /// The writing system's font; null or empty keeps the + /// default. + public DetailReversalAlternative(string wsAbbrev, string text, string fontFamily) + { + WsAbbrev = wsAbbrev; + Text = text; + FontFamily = fontFamily; + } + + public string WsAbbrev { get; } + + public string Text { get; } + + public string FontFamily { get; } + } + + /// + /// One row of a : an entry linked to the sense, or the + /// group's add row. + /// + public sealed class DetailReversalRow + { + /// Creates the row. + /// The opaque identity the edit context issued for this row. + /// The entry's form, a subentry's ancestors colon-joined before + /// it; empty for an add row. + /// True for the group's add row. + /// The entry's forms in other writing systems; null means + /// none. + public DetailReversalRow(string rowKey, string text, bool isAddSlot, + IReadOnlyList otherWsForms = null) + { + RowKey = rowKey; + Text = text ?? string.Empty; + IsAddSlot = isAddSlot; + OtherWsForms = otherWsForms ?? Array.Empty(); + } + + public string RowKey { get; } + + public string Text { get; } + + public bool IsAddSlot { get; } + + /// The entry's forms in other writing systems, in display order; never + /// null. + public IReadOnlyList OtherWsForms { get; } + } + + /// + /// One reversal index's rows: the sense's entries in that index, followed by its add row. + /// + public sealed class DetailReversalGroup + { + /// Creates the group. + /// The index's writing system tag. + /// The index's writing system abbreviation, the group's + /// label. + /// The writing system's font; null or empty keeps the + /// default. + /// Whether the writing system is right-to-left. + /// The group's rows in display order; null means none. + public DetailReversalGroup(string wsTag, string wsAbbrev, string fontFamily, bool rightToLeft, + IReadOnlyList rows) + { + WsTag = wsTag; + WsAbbrev = wsAbbrev; + FontFamily = fontFamily; + RightToLeft = rightToLeft; + Rows = rows ?? Array.Empty(); + } + + public string WsTag { get; } + + public string WsAbbrev { get; } + + public string FontFamily { get; } + + public bool RightToLeft { get; } + + public IReadOnlyList Rows { get; } + } + + /// + /// FieldWorks-owned editor for a sense's reversal entries. Each reversal index is a group + /// labeled with its writing system abbreviation: one wrapping line of editable slots, one + /// per linked entry and a final empty one for adding, with a bar between neighboring + /// slots. A row commits once, when it loses focus, + /// through ; the rows are a snapshot that the host + /// rebuilds after its save. Right-clicking a row offers "Show in Reversal Index", and + /// Ctrl+click runs it directly. An edit context without + /// shows the rows read-only. + /// + public sealed class FwReversalEntriesField : StackPanel, IDisposable + { + private readonly List _teardown = new List(); + private readonly IReversalEntryEditing _editing; + private readonly Action _navigationRequested; + private bool _disposed; + + /// Builds the editor. + /// The field label, used in accessible names. + /// The field's automation id, the prefix of every row's + /// id. + /// The groups in display order; null means none. + /// The detail view's edit context; null shows the rows + /// read-only. + /// Called with a group's writing system tag when + /// one of its rows gains focus; null disables that. + /// Called with a row key to show that row's entry + /// in the Reversal Index tool; null leaves out the jump. + /// Width of the abbreviation column; null uses the + /// default. + public FwReversalEntriesField(string label, string automationId, + IReadOnlyList groups, IDetailEditContext editContext, + Action writingSystemFocused = null, Action navigationRequested = null, + double? wsAbbrevColumnWidth = null) + { + Spacing = FwAvaloniaDensity.RowSpacing; + var name = label ?? automationId; + AutomationProperties.SetAutomationId(this, automationId); + AutomationProperties.SetName(this, name); + _editing = editContext as IReversalEntryEditing; + _navigationRequested = _editing == null ? null : navigationRequested; + + var abbrevWidth = wsAbbrevColumnWidth ?? FwAvaloniaDensity.WsAbbrevWidth; + foreach (var group in groups ?? Array.Empty()) + Children.Add(CreateGroup(name, automationId, group, writingSystemFocused, abbrevWidth)); + } + + private Control CreateGroup(string label, string automationId, DetailReversalGroup group, + Action writingSystemFocused, double abbrevWidth) + { + var groupId = automationId + "." + group.WsTag; + // The group's entries run together on one wrapping line, a bar between each pair, + // the add row last. + var rows = new WrapPanel + { + Orientation = Orientation.Horizontal, + Background = FwAvaloniaDensity.TransparentBrush, + FlowDirection = group.RightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight + }; + AutomationProperties.SetAutomationId(rows, groupId); + TextBox addBox = null; + for (var i = 0; i < group.Rows.Count; i++) + { + if (i > 0) + rows.Children.Add(FwReferenceVectorField.CreateSeparatorBar()); + var row = group.Rows[i]; + rows.Children.Add(CreateRow(label, groupId, group, row, i, writingSystemFocused, out var box)); + if (row.IsAddSlot) + addBox = box; + } + + if (_editing != null && addBox != null) + { + // An empty add row is barely wider than its caret, so a click anywhere on the + // group's free space starts typing there. + EventHandler pressed = (s, e) => + { + if (!ReferenceEquals(e.Source, rows)) + return; + addBox.Focus(); + e.Handled = true; + }; + rows.PointerPressed += pressed; + _teardown.Add(() => rows.PointerPressed -= pressed); + } + + var abbrev = FwMultiWsTextField.CreateWsAbbrev(group.WsAbbrev, abbrevWidth); + var grid = new Grid { ColumnDefinitions = new ColumnDefinitions("Auto,*") }; + Grid.SetColumn(abbrev, 0); + Grid.SetColumn(rows, 1); + grid.Children.Add(abbrev); + grid.Children.Add(rows); + return grid; + } + + private Control CreateRow(string label, string groupId, DetailReversalGroup group, + DetailReversalRow row, int index, Action writingSystemFocused, out TextBox box) + { + var rowId = row.IsAddSlot ? groupId + ".Add" : groupId + "." + index; + var editor = new TextBox + { + Text = row.Text, + Padding = FwAvaloniaDensity.EditorPadding, + MinHeight = 0, + AcceptsReturn = false, + IsReadOnly = _editing == null, + FlowDirection = group.RightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight, + BorderThickness = new Thickness(0), + Background = FwAvaloniaDensity.TransparentBrush, + TextWrapping = TextWrapping.Wrap + TextWrapping = TextWrapping.NoWrap + }; + box = editor; + if (!string.IsNullOrEmpty(group.FontFamily)) + editor.FontFamily = new FontFamily(group.FontFamily); + AutomationProperties.SetAutomationId(editor, rowId); + AutomationProperties.SetName(editor, row.IsAddSlot + ? FwAvaloniaStrings.ReversalAddEntryName(label, group.WsAbbrev) + : label + " " + group.WsAbbrev); + + // Tracks what the model holds for this row, so an unchanged row never stages. + var committed = row.Text; + Action commit = () => + { + var text = editor.Text ?? string.Empty; + if (_editing != null && text != committed && _editing.TryCommitRow(row.RowKey, text)) + committed = text; + }; + + if (_editing != null) + { + // Runs before the host's own focus-loss save, which bubbles up from this box. + EventHandler lost = (s, e) => commit(); + editor.LostFocus += lost; + _teardown.Add(() => editor.LostFocus -= lost); + } + + if (writingSystemFocused != null && !string.IsNullOrEmpty(group.WsTag)) + { + EventHandler got = (s, e) => writingSystemFocused(group.WsTag); + editor.GotFocus += got; + _teardown.Add(() => editor.GotFocus -= got); + } + + if (_navigationRequested != null) + WireNavigation(editor, row, commit); + + var suffix = CreateOtherWsSuffix(row, rowId + ".OtherWs"); + if (suffix == null) + return editor; + var panel = new StackPanel + { + Orientation = Orientation.Horizontal, + Background = FwAvaloniaDensity.TransparentBrush + }; + panel.Children.Add(editor); + panel.Children.Add(suffix); + return panel; + } + + // The jump commits the row first, so a form typed into the add row exists when shown. + private void WireNavigation(TextBox box, DetailReversalRow row, Action commit) + { + Action jump = () => + { + commit(); + _navigationRequested(row.RowKey); + }; + + EventHandler pressed = (s, e) => + { + if (string.IsNullOrEmpty(box.Text) + || !e.KeyModifiers.HasFlag(KeyModifiers.Control) + || !e.GetCurrentPoint(box).Properties.IsLeftButtonPressed) + { + return; + } + jump(); + e.Handled = true; + }; + box.AddHandler(InputElement.PointerPressedEvent, pressed, RoutingStrategies.Tunnel); + + var show = new MenuItem { Header = FwAvaloniaStrings.ReversalShowInReversalIndex }; + EventHandler click = (s, e) => jump(); + show.Click += click; + var menu = new MenuFlyout { Items = { show } }; + EventHandler opening = (s, e) => show.IsEnabled = !string.IsNullOrEmpty(box.Text); + menu.Opening += opening; + var previousMenu = box.ContextFlyout; + box.ContextFlyout = menu; + var popupTeardown = PopupReporting.Wire(menu); + _teardown.Add(() => + { + box.RemoveHandler(InputElement.PointerPressedEvent, pressed); + show.Click -= click; + menu.Opening -= opening; + popupTeardown(); + box.ContextFlyout = previousMenu; + }); + } + + // Read-only, in display order: [abbrev form, abbrev form]. Null when the entry has none. + private static Control CreateOtherWsSuffix(DetailReversalRow row, string automationId) + { + if (row.OtherWsForms.Count == 0) + return null; + + var block = new TextBlock + { + TextWrapping = TextWrapping.Wrap, + VerticalAlignment = VerticalAlignment.Top, + Padding = FwAvaloniaDensity.EditorPadding, + FlowDirection = FlowDirection.LeftToRight + }; + block.Inlines.Add(new Run("[")); + for (var i = 0; i < row.OtherWsForms.Count; i++) + { + var alternative = row.OtherWsForms[i]; + if (i > 0) + block.Inlines.Add(new Run(", ")); + if (!string.IsNullOrEmpty(alternative.WsAbbrev)) + { + block.Inlines.Add(new Run(alternative.WsAbbrev) + { + FontSize = FwAvaloniaDensity.WsAbbrevFontSize, + Foreground = FwAvaloniaDensity.WsAbbrevBrush + }); + block.Inlines.Add(new Run(" ")); + } + var form = new Run(alternative.Text ?? string.Empty); + if (!string.IsNullOrEmpty(alternative.FontFamily)) + form.FontFamily = new FontFamily(alternative.FontFamily); + block.Inlines.Add(form); + } + block.Inlines.Add(new Run("]")); + AutomationProperties.SetAutomationId(block, automationId); + return block; + } + + /// + /// The count of still-attached handler teardowns; zero after . + /// + public int AttachedHandlerCount => _teardown.Count; + + /// Detaches every wired handler and drops the row menus. Idempotent. + public void Dispose() + { + if (_disposed) + return; + _disposed = true; + foreach (var detach in _teardown) + detach(); + _teardown.Clear(); + } + } +} diff --git a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs new file mode 100644 index 0000000000..ea7ee7d018 --- /dev/null +++ b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs @@ -0,0 +1,39 @@ +// 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; + +namespace SIL.FieldWorks.Common.FwAvalonia.Detail +{ + /// + /// The row-level editing capability behind , kept off + /// the core so only a context that edits a sense's + /// reversal entries carries it. A caller acquires it with + /// ctx as IReversalEntryEditing and treats a null result as read-only. Row keys are + /// the opaque values the same context issued; an + /// unknown key is rejected. + /// + public interface IReversalEntryEditing + { + /// + /// Stages the text of one row, opening the edit session only when something changes. + /// Text equal to what the row already shows changes nothing; empty text on an entry + /// row unlinks that entry (an entry left with no senses and no subentries is + /// deleted). Other text is split on colons into a chain of entry and subentry forms, + /// and the sense is linked to the deepest entry of that chain, found or created -- an + /// existing entry is never renamed. Afterwards the key names the row's new entry, or + /// an add row again after an unlink, so a second commit on the same row edits what + /// the first one produced. + /// + /// False, without opening the session, for an unknown key, an empty add + /// row, or unchanged text. + bool TryCommitRow(string rowKey, string typedText); + + /// + /// The guid of the top-level entry to show for a row: the row's own entry, or for a + /// subentry its main entry. Null for an add row or an unknown key. + /// + Guid? TryResolveMainEntryGuid(string rowKey); + } +} diff --git a/Src/Common/FwAvalonia/Detail/SliceFactory.cs b/Src/Common/FwAvalonia/Detail/SliceFactory.cs index d6289f38a3..f690ea4aa5 100644 --- a/Src/Common/FwAvalonia/Detail/SliceFactory.cs +++ b/Src/Common/FwAvalonia/Detail/SliceFactory.cs @@ -100,7 +100,7 @@ public static Control Build(DetailField field, string automationId, switch (field.Kind) { case DetailFieldKind.Custom: - return CreateCustom(field, automationId); + return CreateCustom(field, automationId, context); case DetailFieldKind.ReferenceVector: // Reference add/remove gestures commit immediately (legacy chooser-dialog behavior): the // staged session would otherwise sit open -- LCModel broadcasts PropChanged @@ -164,7 +164,8 @@ private static Control CreateLiteral(DetailField field, string automationId) // the value column. A missing, null-returning, or throwing factory // degrades to the unsupported row -- never a crash, never silently // blank. - private static Control CreateCustom(DetailField field, string automationId) + private static Control CreateCustom(DetailField field, string automationId, + SliceFactoryContext context) { if (field.ControlFactory == null) { @@ -175,7 +176,7 @@ private static Control CreateCustom(DetailField field, string automationId) try { - var control = field.ControlFactory(); + var control = field.ControlFactory(context); if (control == null) { System.Diagnostics.Debug.WriteLine( diff --git a/Src/Common/FwAvalonia/FwAvaloniaStrings.cs b/Src/Common/FwAvalonia/FwAvaloniaStrings.cs index 7cd4da6fd8..5b44a6fa95 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaStrings.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaStrings.cs @@ -302,5 +302,16 @@ public static string StructuredTextParagraphName(string fieldLabel, int paragrap /// The empty-choice label a list chooser leads with when the field allows no value. /// English must match the legacy launchers' ksNullLabel so both frameworks show the same word. public static string ChooserEmptyItemLabel => Text("FwAvalonia.Chooser.EmptyItemLabel"); + + // ----- Reversal Entries field. APPEND-ONLY. ----- + + /// The row menu command that jumps to the row's entry in the Reversal Index + /// tool. + public static string ReversalShowInReversalIndex => Text("FwAvalonia.Reversal.ShowInReversalIndex"); + + /// Screen-reader name for a writing system's empty add row. + /// {0} = the field label, {1} = the writing system abbreviation. + public static string ReversalAddEntryName(string fieldLabel, string wsAbbrev) + => string.Format(Text("FwAvalonia.Reversal.AddEntryName"), fieldLabel, wsAbbrev); } } diff --git a/Src/Common/FwAvalonia/FwAvaloniaStrings.resx b/Src/Common/FwAvalonia/FwAvaloniaStrings.resx index 4250706a24..424db73dd1 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaStrings.resx +++ b/Src/Common/FwAvalonia/FwAvaloniaStrings.resx @@ -238,4 +238,11 @@ <Empty> + + Show in Reversal Index (Ctrl-Click) + + + {0} {1} new entry + {0} = the field label, {1} = the writing system abbreviation + \ No newline at end of file diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs new file mode 100644 index 0000000000..3b7598f31c --- /dev/null +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -0,0 +1,344 @@ +// 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 System.Linq; +using Avalonia; +using Avalonia.Automation; +using Avalonia.Controls; +using Avalonia.Headless; +using Avalonia.Headless.NUnit; +using Avalonia.Input; +using Avalonia.Interactivity; +using Avalonia.Media; +using Avalonia.Threading; +using Avalonia.VisualTree; +using NUnit.Framework; +using SIL.FieldWorks.Common.FwAvalonia; +using SIL.FieldWorks.Common.FwAvalonia.Detail; + +namespace FwAvaloniaTests.Detail +{ + /// + /// The Reversal Entries control () rendered + /// headlessly: its groups and rows, when a row commits, and the row jump. A recording + /// fake stands in for the edit context, whose LCModel side is tested on its own. + /// + [TestFixture] + public class FwReversalEntriesFieldTests + { + private const string FieldId = "Reversal"; + + // Records every row commit and jump, in order, so a test can check which came first. + private sealed class RecordingReversalContext : IDetailEditContext, IReversalEntryEditing + { + public readonly List Events = new List(); + + public bool TryCommitRow(string rowKey, string typedText) + { + Events.Add("commit " + rowKey + "=" + typedText); + return true; + } + + public Guid? TryResolveMainEntryGuid(string rowKey) => Guid.Empty; + + public bool IsOpen => false; + + public bool TrySetText(DetailField field, string ws, string value) => false; + + public bool TrySetRichText(DetailField field, string ws, DetailRichTextValue value) => false; + + public bool TrySetOption(DetailField field, string optionKey) => false; + + public bool TryAddReferenceItem(DetailField field, string optionKey) => false; + + public bool TryRemoveReferenceItem(DetailField field, string optionKey) => false; + + public bool TryMoveReferenceItem(DetailField field, string optionKey, bool forward) => false; + + public IReadOnlyList Validate() => Array.Empty(); + + public void Commit() + { + } + + public void Cancel() + { + } + } + + private static DetailReversalGroup English(params string[] forms) + { + var rows = forms.Select((f, i) => new DetailReversalRow("en" + i, f, false)).ToList(); + rows.Add(new DetailReversalRow("en-add", string.Empty, true)); + return new DetailReversalGroup("en", "Eng", null, false, rows); + } + + private static (FwReversalEntriesField Field, TextBox Other, Window Window) Show( + IDetailEditContext context, List jumps, params DetailReversalGroup[] groups) + { + Action navigate = null; + if (jumps != null) + navigate = key => jumps.Add(key); + var field = new FwReversalEntriesField("Reversal Entries", FieldId, groups, context, + navigationRequested: navigate); + var other = new TextBox(); + var panel = new StackPanel(); + panel.Children.Add(field); + panel.Children.Add(other); + var window = new Window { Content = panel, Width = 420, Height = 300 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + return (field, other, window); + } + + private static T Find(Control root, string automationId) where T : Control + => root.GetVisualDescendants().OfType() + .FirstOrDefault(c => AutomationProperties.GetAutomationId(c) == automationId); + + private static void TypeAndLeave(TextBox box, string text, TextBox other) + { + box.Focus(); + box.Text = text; + other.Focus(); + Dispatcher.UIThread.RunJobs(); + } + + private static void CtrlClick(Window window, Control control) + { + var point = control.TranslatePoint(new Point(2, 2), window); + Assert.That(point, Is.Not.Null, "the click target must be attached and laid out"); + window.MouseDown(point.Value, MouseButton.Left, RawInputModifiers.Control); + window.MouseUp(point.Value, MouseButton.Left, RawInputModifiers.Control); + Dispatcher.UIThread.RunJobs(); + } + + [AvaloniaTest] + public void AGroup_ShowsItsEntries_ThenOneAddRow_UnderOneLabel() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + + Assert.That(Find(field, "Reversal.en.0").Text, Is.EqualTo("dwelling")); + Assert.That(Find(field, "Reversal.en.1").Text, Is.EqualTo("abode")); + Assert.That(Find(field, "Reversal.en.Add").Text, Is.Empty); + Assert.That(field.GetVisualDescendants().OfType().Count(t => t.Text == "Eng"), Is.EqualTo(1), + "the abbreviation labels the group once, not every entry"); + } + + [AvaloniaTest] + public void AGroupsSlots_ShareOneLine_WithABarBetweenEachPair() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var group = Find(field, "Reversal.en"); + var first = Find(field, "Reversal.en.0"); + var second = Find(field, "Reversal.en.1"); + var add = Find(field, "Reversal.en.Add"); + + Assert.That(group.Children.OfType().Count(), Is.EqualTo(2), + "three slots, so two bars"); + Assert.That(second.TranslatePoint(new Point(0, 0), window).Value.Y, + Is.EqualTo(first.TranslatePoint(new Point(0, 0), window).Value.Y), "short entries share a line"); + Assert.That(add.TranslatePoint(new Point(0, 0), window).Value.X, + Is.GreaterThan(second.TranslatePoint(new Point(0, 0), window).Value.X), "the add slot comes last"); + } + + [AvaloniaTest] + public void ALongEntry_StaysOnOneLine_InsideItsSlot() + { + var longForm = string.Join(" ", Enumerable.Repeat("dwelling", 30)); + var (field, _, _) = Show(new RecordingReversalContext(), null, English("home", longForm)); + + Assert.That(Find(field, "Reversal.en.1").Bounds.Height, + Is.EqualTo(Find(field, "Reversal.en.0").Bounds.Height), + "text wider than the line does not wrap inside its slot"); + } + + [AvaloniaTest] + public void ALoneAddSlot_HasNoBar() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English()); + + Assert.That(Find(field, "Reversal.en").Children.OfType(), Is.Empty); + } + + [AvaloniaTest] + public void ClickingAGroupsFreeSpace_StartsTypingInItsAddSlot() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, English("dwelling")); + var group = Find(field, "Reversal.en"); + var point = group.TranslatePoint(new Point(group.Bounds.Width - 2, 2), window); + + window.MouseDown(point.Value, MouseButton.Left); + window.MouseUp(point.Value, MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + + Assert.That(Find(field, "Reversal.en.Add").IsFocused, Is.True); + } + + [AvaloniaTest] + public void EachIndex_IsItsOwnGroup() + { + var french = new DetailReversalGroup("fr", "Fre", null, false, + new[] { new DetailReversalRow("fr0", "maison", false), new DetailReversalRow("fr-add", "", true) }); + + var (field, _, _) = Show(new RecordingReversalContext(), null, English("house"), french); + + Assert.That(Find(field, "Reversal.en"), Is.Not.Null); + Assert.That(Find(field, "Reversal.fr"), Is.Not.Null); + Assert.That(Find(field, "Reversal.fr.0").Text, Is.EqualTo("maison")); + } + + [AvaloniaTest] + public void OtherWritingSystemForms_ShowAsAReadOnlySuffix() + { + var rows = new[] + { + new DetailReversalRow("en0", "house", false, + new[] { new DetailReversalAlternative("EnGB", "houze", null) }), + new DetailReversalRow("en1", "home", false), + new DetailReversalRow("en-add", "", true) + }; + var (field, _, _) = Show(new RecordingReversalContext(), null, + new DetailReversalGroup("en", "Eng", null, false, rows)); + + var suffix = Find(field, "Reversal.en.0.OtherWs"); + Assert.That(suffix, Is.Not.Null); + Assert.That(string.Concat(suffix.Inlines.OfType().Select(r => r.Text)), + Is.EqualTo("[EnGB houze]")); + Assert.That(Find(field, "Reversal.en.1.OtherWs"), Is.Null, "an entry with no alternatives has none"); + Assert.That(Find(field, "Reversal.en.Add.OtherWs"), Is.Null, "an add row has none"); + Assert.That(field.GetVisualDescendants().OfType().Any(b => b.Text?.Contains("houze") == true), + Is.False, "the alternatives are display text, never an editor"); + } + + [AvaloniaTest] + public void AChangedRow_CommitsOnceWhenItLosesFocus() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English("dwelling")); + var add = Find(field, "Reversal.en.Add"); + + add.Focus(); + add.Text = "h"; + add.Text = "home"; + Assert.That(context.Events, Is.Empty, "typing alone commits nothing"); + other.Focus(); + Dispatcher.UIThread.RunJobs(); + + Assert.That(context.Events, Is.EqualTo(new[] { "commit en-add=home" })); + + add.Focus(); + other.Focus(); + Dispatcher.UIThread.RunJobs(); + Assert.That(context.Events, Has.Count.EqualTo(1), "leaving again without a change commits nothing more"); + } + + [AvaloniaTest] + public void AnUntouchedRow_NeverCommits() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English("dwelling")); + + TypeAndLeave(Find(field, "Reversal.en.0"), "dwelling", other); + TypeAndLeave(Find(field, "Reversal.en.Add"), string.Empty, other); + + Assert.That(context.Events, Is.Empty); + } + + [AvaloniaTest] + public void ClearingARow_CommitsEmptyText() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English("dwelling")); + + TypeAndLeave(Find(field, "Reversal.en.0"), string.Empty, other); + + Assert.That(context.Events, Is.EqualTo(new[] { "commit en0=" })); + } + + [AvaloniaTest] + public void CtrlClick_CommitsPendingTextThenJumps() + { + var context = new RecordingReversalContext(); + var jumps = new List(); + var (field, _, window) = Show(context, jumps, English("dwelling")); + var add = Find(field, "Reversal.en.Add"); + + add.Focus(); + add.Text = "home"; + CtrlClick(window, add); + + Assert.That(context.Events, Is.EqualTo(new[] { "commit en-add=home" }), + "the typed entry exists before it is shown"); + Assert.That(jumps, Is.EqualTo(new[] { "en-add" })); + } + + [AvaloniaTest] + public void CtrlClick_OnAnEmptyAddRow_DoesNothing() + { + var jumps = new List(); + var (field, _, window) = Show(new RecordingReversalContext(), jumps, English("dwelling")); + + CtrlClick(window, Find(field, "Reversal.en.Add")); + + Assert.That(jumps, Is.Empty); + } + + [AvaloniaTest] + public void TheRowMenu_OffersTheJump() + { + var jumps = new List(); + var (field, _, _) = Show(new RecordingReversalContext(), jumps, English("dwelling")); + var menu = Find(field, "Reversal.en.0").ContextFlyout as MenuFlyout; + + Assert.That(menu, Is.Not.Null); + var item = menu.Items.OfType().Single(); + Assert.That(item.Header, Is.EqualTo(FwAvaloniaStrings.ReversalShowInReversalIndex)); + item.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + + Assert.That(jumps, Is.EqualTo(new[] { "en0" })); + } + + [AvaloniaTest] + public void WithoutTheReversalCapability_RowsAreReadOnly_AndOfferNoJump() + { + var jumps = new List(); + var (field, _, _) = Show(new FakeDetailEditContext(), jumps, English("dwelling")); + var box = Find(field, "Reversal.en.0"); + + Assert.That(box.IsReadOnly, Is.True); + var jumpItems = (box.ContextFlyout as MenuFlyout)?.Items.OfType() + .Where(i => Equals(i.Header, FwAvaloniaStrings.ReversalShowInReversalIndex)); + Assert.That(jumpItems ?? Enumerable.Empty(), Is.Empty, + "the text box keeps its own menu, but without the jump"); + } + + [AvaloniaTest] + public void ARightToLeftGroup_FlowsRightToLeft() + { + var arabic = new DetailReversalGroup("ar", "Ara", null, true, + new[] { new DetailReversalRow("ar0", "بيت", false), new DetailReversalRow("ar-add", "", true) }); + + var (field, _, _) = Show(new RecordingReversalContext(), null, arabic); + + Assert.That(Find(field, "Reversal.ar.0").FlowDirection, Is.EqualTo(FlowDirection.RightToLeft)); + Assert.That(Find(field, "Reversal.ar.Add").FlowDirection, Is.EqualTo(FlowDirection.RightToLeft)); + } + + [AvaloniaTest] + public void Dispose_DetachesEveryHandler() + { + var (field, _, _) = Show(new RecordingReversalContext(), new List(), English("dwelling")); + var box = Find(field, "Reversal.en.0"); + var jumpMenu = box.ContextFlyout; + Assert.That(field.AttachedHandlerCount, Is.GreaterThan(0)); + + field.Dispose(); + + Assert.That(field.AttachedHandlerCount, Is.Zero); + Assert.That(box.ContextFlyout, Is.Not.SameAs(jumpMenu), "the row menu is released"); + } + } +} diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs index 6e23705486..a2faeec11a 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs @@ -27,7 +27,7 @@ namespace FwAvaloniaTests [TestFixture] public class DetailCustomFieldRenderingTests { - private static DetailModel Model(Func factory) + private static DetailModel Model(Func factory) => new DetailModel("LexEntry", "Normal", new List { @@ -38,9 +38,9 @@ private static DetailModel Model(Func factory) }, new List()); - private static DataTree Show(DetailModel model) + private static DataTree Show(DetailModel model, Action linkRequested = null) { - var view = new DataTree(model); + var view = new DataTree(model, linkRequested: linkRequested); var window = new Window { Content = view, Width = 420, Height = 200 }; window.Show(); Dispatcher.UIThread.RunJobs(); @@ -57,7 +57,7 @@ public void CustomField_RendersTheFactoryControl_InTheValueColumn() var pluginControl = new TextBlock { Text = "plugin notes bar" }; AutomationProperties.SetAutomationId(pluginControl, "PluginNotesBar"); - var view = Show(Model(() => pluginControl)); + var view = Show(Model(_ => pluginControl)); var rendered = view.GetVisualDescendants().OfType() .FirstOrDefault(t => AutomationProperties.GetAutomationId(t) == "PluginNotesBar"); @@ -79,7 +79,7 @@ public void CustomField_RendersTheFactoryControl_InTheValueColumn() [AvaloniaTest] public void CustomField_WithThrowingFactory_FallsBackToTheUnsupportedRow() { - var view = Show(Model(() => throw new InvalidOperationException("plugin exploded"))); + var view = Show(Model(_ => throw new InvalidOperationException("plugin exploded"))); Assert.That(FindUnsupportedBlock(view), Is.Not.Null, "a throwing factory degrades to the explicit unsupported row"); @@ -97,10 +97,27 @@ public void CustomField_WithoutAFactory_FallsBackToTheUnsupportedRow() [AvaloniaTest] public void CustomField_WithNullReturningFactory_FallsBackToTheUnsupportedRow() { - var view = Show(Model(() => null)); + var view = Show(Model(_ => null)); Assert.That(FindUnsupportedBlock(view), Is.Not.Null, "a null-returning factory degrades to the explicit unsupported row"); } + + [AvaloniaTest] + public void CustomField_FactoryReceivesTheViewsLinkCallback() + { + var requests = new List(); + Action received = null; + var model = Model(render => + { + received = render.LinkRequested; + return new TextBlock { Text = "plugin" }; + }); + Show(model, requests.Add); + + Assert.That(received, Is.Not.Null, "the plugin can reach the host's jump"); + received(new DetailLinkRequest(null, new DetailChooserLink("Show", "someTool"))); + Assert.That(requests, Has.Count.EqualTo(1), "the callback is the one the view was given"); + } } } diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs index 44de538ece..719f746d9e 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs @@ -26,7 +26,7 @@ namespace FwAvaloniaTests public class SliceFactoryTests { private static DetailField Field(DetailFieldKind kind, string selectedOption = null, - System.Func controlFactory = null) + System.Func controlFactory = null) => new DetailField( stableId: "f1", label: "Label", field: "Field", writingSystem: "en", kind: kind, editorClassification: EditorClassification.Known, automationId: "Auto.Id", @@ -80,10 +80,35 @@ public void CustomKind_FactoryControl_IsReturned() { var marker = new Border(); var control = SliceFactory.Build( - Field(DetailFieldKind.Custom, controlFactory: () => marker), "Auto.Id", null); + Field(DetailFieldKind.Custom, controlFactory: _ => marker), "Auto.Id", null); Assert.That(control, Is.SameAs(marker)); } + [AvaloniaTest] + public void CustomKind_FactoryReceivesTheRenderContext() + { + var context = new SliceFactoryContext(linkRequested: request => { }); + SliceFactoryContext received = null; + SliceFactory.Build(Field(DetailFieldKind.Custom, controlFactory: render => + { + received = render; + return new Border(); + }), "Auto.Id", context); + Assert.That(received, Is.SameAs(context)); + } + + [AvaloniaTest] + public void CustomKind_WithoutAContext_FactoryStillReceivesOne() + { + SliceFactoryContext received = null; + SliceFactory.Build(Field(DetailFieldKind.Custom, controlFactory: render => + { + received = render; + return new Border(); + }), "Auto.Id", null); + Assert.That(received, Is.Not.Null, "a factory can read its render context without a null check"); + } + [AvaloniaTest] public void BrowseStyleContext_TextField_SuppressesWritingSystemAbbreviation() { diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 01e2f7d6da..6d4be1547b 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -1049,6 +1049,8 @@ private void WalkField(ViewNode node, ICmObject obj, int depth) var plugin = _plugins?.Resolve(node.CustomEditorClass); if (plugin != null) { + if (HideWhenEmpty(node) && !PluginFieldHasData(node, obj)) + return; AddPluginRow(node, obj, depth, plugin); foreach (var pluginChild in node.Children) Walk(pluginChild, obj, depth + 1); @@ -3140,9 +3142,14 @@ private void AddPluginRow(ViewNode node, ICmObject obj, int depth, ISlicePlugin // ONE plugin contract -- the build context bundles everything a // plugin can need (object, node, deferred edit-context accessor, cache, focus // callback); there is no service-aware marker type test. - var context = new SlicePluginBuildContext(obj, node, _editContextAccessor, _cache, - _writingSystemFocused); - Func factory = () => plugin.BuildControl(context); + var visible = _showAllWsFields != null && _showAllWsFields.Contains(node.StableId) + ? null + : node.VisibleWritingSystems; + // Built at render time: the link callback and column width exist only in the + // render context. + Func factory = render => + plugin.BuildControl(new SlicePluginBuildContext(obj, node, _editContextAccessor, _cache, + _writingSystemFocused, render?.LinkRequested, render?.WsAbbrevColumnWidth, visible)); AddField(new DetailField(StableId(node, obj), Localize(node.Label) ?? node.Field, node.Field, node.WritingSystem, DetailFieldKind.Custom, node.EditorClassification, node.AutomationId, node.LocalizationKey, node.Routing, null, null, null, @@ -3152,6 +3159,26 @@ private void AddPluginRow(ViewNode node, ICmObject obj, int depth, ISlicePlugin controlFactory: factory)); } + // An ifdata custom row hides when its own field is an empty vector. Any other + // field type, or a field that does not resolve, counts as having data. + private bool PluginFieldHasData(ViewNode node, ICmObject obj) + { + var flid = GetFlid(obj, node.Field); + if (flid == 0) + return true; + var type = (CellarPropertyType)(_mdc.GetFieldType(flid) & (int)CellarPropertyTypeFilter.VirtualMask); + switch (type) + { + case CellarPropertyType.OwningCollection: + case CellarPropertyType.OwningSequence: + case CellarPropertyType.ReferenceCollection: + case CellarPropertyType.ReferenceSequence: + return _sda.get_VecSize(obj.Hvo, flid) > 0; + default: + return true; + } + } + private void WalkUnsupported(ViewNode node, ICmObject obj, int depth) { // The Unsupported worklist row still binds its object and slice menus so a right-click diff --git a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs index 7ab4d7a6d6..79bfe2616f 100644 --- a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs +++ b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2026 SIL International +// 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) @@ -6,42 +6,37 @@ using System.Collections.Generic; using System.Linq; using Avalonia.Controls; +using SIL.FieldWorks.Common.FwAvalonia; using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; +using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; -using SIL.LCModel.Core.KernelInterfaces; -using SIL.LCModel.Core.Text; +using SIL.LCModel.DomainServices; using SIL.Reporting; namespace SIL.FieldWorks.XWorks { /// - /// The native Avalonia Reversal Entries editor: claims the legacy - /// SIL.FieldWorks.XWorks.LexEd.ReversalIndexEntrySlice layout identity through the plugin - /// contract and renders the sense's reversal-entry forms as an editable multi-writing-system text - /// field () at the slice's real in-tree position, rather than an - /// "Unsupported" placeholder row. - /// A sense's reversal entries (ILexSense.ReferringReversalIndexEntries) are a set of - /// IReversalIndexEntry, each storing its form (ReversalForm, a multi-unicode string) - /// under its owning reversal index's writing system. The editor renders one row per EXISTING - /// reversal entry -- the entry's form in its index's writing system -- reusing the same - /// plain-text-over-preserved-runs TrySetRichText staging every other text row uses, so the - /// edit rides the detail view's SAME fenced undo step. - /// DATA-SAFE SCOPE: this editor edits the form text of EXISTING reversal entries only. - /// Creating a new reversal index entry (typing a new form on an empty row) and deleting one - /// (clearing a form) are the legacy slice's risky parses-of-semicolon-separated-lists + - /// find-or-create path (ReversalIndexEntrySlice.ReplaceReversalIndexEntries) and are not supported here: - /// a sense with no reversal entry for a given index simply shows no row for it, and clearing a - /// form to empty stores an empty form (it does not delete the entry). + /// The Avalonia Reversal Entries editor for a sense: claims the + /// SIL.FieldWorks.XWorks.LexEd.ReversalIndexEntrySlice layout identity and renders an + /// . Each reversal index whose writing system the row + /// shows is a group listing the sense's entries in it, then an add row; an index that does + /// not exist is never created just to show a group (LT-4480). Typing links, relinks, or + /// unlinks entries, with colons marking subentries (LT-4665), and a row can jump to its + /// entry in the Reversal Index tool. A build failure degrades to the Unsupported row. /// public sealed class ReversalIndexEntryPlugin : ISlicePlugin { - /// The legacy slice class this plugin claims (LexSenseParts.xml reversal entries slice). + /// The layout class this plugin claims: the sense's Reversal Entries. public const string ReversalIndexEntrySliceClassName = "SIL.FieldWorks.XWorks.LexEd.ReversalIndexEntrySlice"; /// The editor's automation id when the layout node declares none. public const string DefaultAutomationId = "ReversalEntriesEditor"; + /// The tool a row's jump opens, showing the entry. + public const string ReversalIndexTool = "reversalToolEditComplete"; + public string LegacyClassName => ReversalIndexEntrySliceClassName; public Control BuildControl(SlicePluginBuildContext context) @@ -54,145 +49,314 @@ public Control BuildControl(SlicePluginBuildContext context) try { var node = context.Node; - var rows = CreateReversalRows(sense, cache, out var entryByWsKey); - if (rows.Count == 0) - return null; // no existing reversal entry: nothing editable (creation is not supported) - - var field = new DetailField( - stableId: "reversal/" + sense.Hvo, - label: node?.Label ?? "Reversal Entries", - field: node?.Field ?? "ReferringReversalIndexEntries", - writingSystem: node?.WritingSystem, - kind: DetailFieldKind.Text, - editorClassification: node?.EditorClassification - ?? SIL.FieldWorks.Common.FwAvalonia.ViewDefinition.EditorClassification.Known, - automationId: node?.AutomationId ?? DefaultAutomationId, - localizationKey: node?.LocalizationKey, - routing: node?.Routing ?? SIL.FieldWorks.Common.FwAvalonia.ViewDefinition.HostRouting.Product, - values: rows, - options: null, - selectedOptionKey: null, - isEditable: true, - objectHvo: sense.Hvo); - - var reversalContext = new ReversalDetailEditContext(cache, context.EditContext, entryByWsKey); + var label = string.IsNullOrEmpty(node?.Label) + ? node?.Field ?? DefaultAutomationId + : StringTable.Table.LocalizeAttributeValue(node.Label); var automationId = node?.AutomationId ?? DefaultAutomationId; - return new FwMultiWsTextField(field, automationId, reversalContext, - writingSystemFocused: context.WritingSystemFocused); + var host = context.EditContext; + var editing = new ReversalDetailEditContext(cache, host, sense, label); + var groups = editing.CreateGroups(context.VisibleWritingSystems); + + Action navigate = null; + var linkRequested = context.LinkRequested; + if (linkRequested != null) + { + var field = new DetailField( + stableId: "reversal/" + sense.Hvo, + label: label, + field: node?.Field, + writingSystem: node?.WritingSystem, + kind: DetailFieldKind.Custom, + editorClassification: node?.EditorClassification ?? EditorClassification.Known, + automationId: automationId, + localizationKey: node?.LocalizationKey, + routing: node?.Routing ?? HostRouting.Product, + values: null, + options: null, + selectedOptionKey: null, + isEditable: true, + objectHvo: sense.Hvo); + navigate = rowKey => + { + var target = editing.TryResolveMainEntryGuid(rowKey); + if (target.HasValue) + RequestShowInReversalIndex(linkRequested, field, target.Value); + }; + } + + return new FwReversalEntriesField(label, automationId, groups, + host == null ? null : editing, context.WritingSystemFocused, navigate, + context.WsAbbrevColumnWidth); } catch (Exception e) { - // Graceful degradation, same policy as the other plugins: a broken reversal read/build - // degrades to the unsupported row (the view's null-factory guard), never the whole pane. Logger.WriteEvent($"ReversalIndexEntryPlugin: reversal editor unavailable for sense '{sense.Guid}': {e}"); return null; } } - // One editable row per EXISTING reversal entry: ReversalForm in its index's - // writing system. WsTag (what wsKey routes on) is the index's ws tag -- - // unique since a sense has at most one entry per index. - private static IReadOnlyList CreateReversalRows(ILexSense sense, LcmCache cache, - out IReadOnlyDictionary entryByWsKey) + /// Asks the host to show the entry with guid in the + /// Reversal Index tool. + internal static void RequestShowInReversalIndex(Action linkRequested, + DetailField field, Guid target) { - var values = new List(); - var map = new Dictionary(StringComparer.Ordinal); - var wsManager = cache.ServiceLocator.WritingSystemManager; - var factory = cache.WritingSystemFactory; - - foreach (var entry in sense.ReferringReversalIndexEntries) - { - var wsTag = entry.ReversalIndex?.WritingSystem; - if (string.IsNullOrEmpty(wsTag) || map.ContainsKey(wsTag)) - continue; - var wsHandle = wsManager.GetWsFromStr(wsTag); - if (wsHandle <= 0) - continue; - - var ws = wsManager.Get(wsHandle); - var tss = entry.ReversalForm.get_String(wsHandle); - var richText = DetailRichTextAdapter.FromTsString(tss, factory); - values.Add(new DetailWsValue(ws.Abbreviation, tss?.Text ?? string.Empty, - ws.DefaultFontName, 0, ws.RightToLeftScript, ws.Id, false, richText)); - map[ws.Id] = entry; - } - - entryByWsKey = map; - return values; + linkRequested(new DetailLinkRequest(field, new DetailChooserLink( + FwAvaloniaStrings.ReversalShowInReversalIndex, ReversalIndexTool, target.ToString()))); } } /// - /// The Reversal Entries plugin's edit context: routes / - /// to the matching reversal entry's ReversalForm (data-safe: edits existing forms only), staging - /// through the detail view's SHARED session so a reversal edit lands as - /// ONE step on the same undoable fence as every other row. Session lifecycle (IsOpen/Commit/Cancel) and - /// validation delegate to the host context, so the host view's Save/Cancel commit reversal edits too. + /// The Reversal Entries edit context: projects a sense's reversal entries into + /// rows and applies row commits to them. Every write + /// stages on the host's shared fenced session, so a commit is one undo step with the view's + /// other edits; session lifecycle and validation delegate to the host. /// - internal sealed class ReversalDetailEditContext : IDetailEditContext + internal sealed class ReversalDetailEditContext : IDetailEditContext, IReversalEntryEditing { private readonly LcmCache _cache; private readonly IDetailEditContext _host; - private readonly IReadOnlyDictionary _entryByWsKey; + private readonly ILexSense _sense; + private readonly string _fieldLabel; + private readonly Dictionary _rows = + new Dictionary(StringComparer.Ordinal); + private int _nextRowKey; - public ReversalDetailEditContext(LcmCache cache, IDetailEditContext host, - IReadOnlyDictionary entryByWsKey) + public ReversalDetailEditContext(LcmCache cache, IDetailEditContext host, ILexSense sense, + string fieldLabel) { _cache = cache ?? throw new ArgumentNullException(nameof(cache)); + _sense = sense ?? throw new ArgumentNullException(nameof(sense)); _host = host; - _entryByWsKey = entryByWsKey ?? new Dictionary(); + _fieldLabel = fieldLabel; } - public bool IsOpen => _host != null && _host.IsOpen; + // The reversal index a row belongs to and the entry it shows; null is an add row. + private sealed class RowBinding + { + public RowBinding(IReversalIndex index, IReversalIndexEntry entry) + { + Index = index; + Entry = entry; + } + + public IReversalIndex Index { get; } - public bool TrySetText(DetailField field, string ws, string value) + public IReversalIndexEntry Entry { get; set; } + } + + /// + /// The groups to show, one per analysis writing system allowed by + /// (null or empty allows all) that has a + /// reversal index. A writing system that is not a current analysis one shows only when + /// the sense has entries in it. Issues fresh row keys and forgets the previous ones. + /// + internal IReadOnlyList CreateGroups(IReadOnlyList visibleWritingSystems) { - if (string.IsNullOrEmpty(ws) || !_entryByWsKey.TryGetValue(ws, out var entry)) - return false; - var wsHandle = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(ws); - if (wsHandle <= 0) - return false; - return StageOnHost(() => + _rows.Clear(); + var linked = _sense.ReferringReversalIndexEntries.ToList(); + var current = new HashSet( + _cache.ServiceLocator.WritingSystems.CurrentAnalysisWritingSystems.Select(ws => ws.Id)); + var systems = DetailComposer.ApplyVisibleWritingSystems( + _cache.LanguageProject.AnalysisWritingSystems.ToList(), visibleWritingSystems); + + var groups = new List(); + foreach (var ws in systems) { - entry.ReversalForm.set_String(wsHandle, - TsStringUtils.MakeString(value ?? string.Empty, wsHandle)); - return true; - }); + var index = _cache.LanguageProject.LexDbOA.ReversalIndexesOC + .FirstOrDefault(ri => ri.WritingSystem == ws.Id); + if (index == null) + continue; + var entries = index.EntriesForSense(linked).ToList(); + if (entries.Count == 0 && !current.Contains(ws.Id)) + continue; + + var rows = new List(); + foreach (var entry in entries) + { + rows.Add(new DetailReversalRow(Bind(index, entry), ChainText(entry, ws.Handle), false, + OtherWsForms(entry, ws.Handle))); + } + rows.Add(new DetailReversalRow(Bind(index, null), string.Empty, true)); + groups.Add(new DetailReversalGroup(ws.Id, ws.Abbreviation, ws.DefaultFontName, + ws.RightToLeftScript, rows)); + } + return groups; + } + + private string Bind(IReversalIndex index, IReversalIndexEntry entry) + { + var key = "row" + _nextRowKey++; + _rows[key] = new RowBinding(index, entry); + return key; + } + + // A subentry shows its ancestors' forms before its own, joined by ": ". + private static string ChainText(IReversalIndexEntry entry, int ws) + { + var forms = new List(); + for (var level = entry; level != null; level = level.OwningEntry) + forms.Insert(0, level.ReversalForm.get_String(ws).Text ?? string.Empty); + return string.Join(": ", forms); + } + + private IReadOnlyList OtherWsForms(IReversalIndexEntry entry, int indexWs) + { + var result = new List(); + foreach (var ws in WritingSystemServices.GetReversalIndexWritingSystems(_cache, entry.Hvo, false)) + { + if (ws.Handle == indexWs) + continue; + var text = entry.ReversalForm.get_String(ws.Handle).Text; + if (!string.IsNullOrEmpty(text)) + result.Add(new DetailReversalAlternative(ws.Abbreviation, text, ws.DefaultFontName)); + } + return result; } - public bool TrySetRichText(DetailField field, string ws, DetailRichTextValue value) + /// + public bool TryCommitRow(string rowKey, string typedText) { - if (value == null || string.IsNullOrEmpty(ws) || !_entryByWsKey.TryGetValue(ws, out var entry)) + RowBinding binding; + if (string.IsNullOrEmpty(rowKey) || !_rows.TryGetValue(rowKey, out binding)) + return false; + var ws = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(binding.Index.WritingSystem); + if (ws <= 0) return false; - var wsHandle = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(ws); - if (wsHandle <= 0) + + var forms = SplitForms(typedText); + var current = binding.Entry; + if (current != null && !current.IsValidObject) + current = binding.Entry = null; + if (current == null && forms.Count == 0) return false; + if (current != null && ChainMatches(current, forms, ws)) + return false; + return StageOnHost(() => { - // ReversalForm is multi-unicode (plain text): re-emit the run-replay as a plain string in - // the row's writing system. The per-run rich projection still drives display/formatting, - // but the stored property carries no run structure. - var tss = DetailRichTextAdapter.ToTsString(value, _cache.WritingSystemFactory, wsHandle); - entry.ReversalForm.set_String(wsHandle, - TsStringUtils.MakeString(tss?.Text ?? string.Empty, wsHandle)); + IReversalIndexEntry target = null; + if (forms.Count > 0) + { + target = FindOrCreateEntry(binding.Index, forms, ws); + if (!target.SensesRS.Contains(_sense)) + target.SensesRS.Add(_sense); + } + if (current != null && current != target) + Unlink(current); + binding.Entry = target; return true; }); } - // Stage on the host's shared fenced session when present (the detail - // view's own context); fall back to a self-contained non-undoable write - // only when no host context exists. + /// + public Guid? TryResolveMainEntryGuid(string rowKey) + { + RowBinding binding; + if (string.IsNullOrEmpty(rowKey) || !_rows.TryGetValue(rowKey, out binding)) + return null; + var entry = binding.Entry; + if (entry == null || !entry.IsValidObject) + return null; + return entry.MainEntry.Guid; + } + + /// + /// The forms of an entry chain, top level first: the text split on colons, each part + /// trimmed, and empty parts dropped (LT-4665). + /// + internal static IList SplitForms(string text) + { + return (text ?? string.Empty) + .Split(new[] { ':' }, StringSplitOptions.RemoveEmptyEntries) + .Select(part => part.Trim()) + .Where(part => part.Length > 0) + .ToList(); + } + + // True when the entry and its ancestors, top level first, are exactly the given forms. + private static bool ChainMatches(IReversalIndexEntry entry, IList forms, int ws) + { + var level = entry; + for (var i = forms.Count - 1; i >= 0; i--) + { + if (level == null || level.ReversalForm.get_String(ws).Text != forms[i]) + return false; + level = level.OwningEntry; + } + return level == null; + } + + // Reuses the deepest existing entry matching a prefix of the chain and creates the + // levels below it. An existing entry is never renamed. + private IReversalIndexEntry FindOrCreateEntry(IReversalIndex index, IList forms, int ws) + { + IReversalIndexEntry deepest = null; + var depth = 0; + FindDeepest(index.EntriesOC, forms, 0, ws, ref deepest, ref depth); + if (depth == forms.Count) + return deepest; + + var factory = _cache.ServiceLocator.GetInstance(); + var owner = deepest; + for (var level = depth; level < forms.Count; level++) + { + var created = factory.Create(); + if (owner == null) + index.EntriesOC.Add(created); + else + owner.SubentriesOS.Add(created); + created.ReversalForm.set_String(ws, forms[level]); + owner = created; + } + return owner; + } + + // Depth-first, so a chain is found under whichever same-form homograph has it. The + // first entry to reach a new depth is kept; a full match ends the search. + private static void FindDeepest(IEnumerable candidates, IList forms, + int level, int ws, ref IReversalIndexEntry deepest, ref int depth) + { + foreach (var candidate in candidates) + { + if (candidate.ReversalForm.get_String(ws).Text != forms[level]) + continue; + if (level + 1 > depth) + { + deepest = candidate; + depth = level + 1; + } + if (depth == forms.Count) + return; + if (level + 1 < forms.Count) + { + FindDeepest(candidate.SubentriesOS, forms, level + 1, ws, ref deepest, ref depth); + if (depth == forms.Count) + return; + } + } + } + + // An entry left with no senses and no subentries is deleted. + private void Unlink(IReversalIndexEntry entry) + { + entry.SensesRS.Remove(_sense); + if (entry.SensesRS.Count == 0 && entry.SubentriesOS.Count == 0) + entry.Delete(); + } + + // A host that is not the fenced detail context (a test fake) applies the write directly. private bool StageOnHost(Func setter) { - if (_host is DetailEditContextBase fenced) - return fenced.Stage(setter); - if (_host != null) - return setter(); // a non-fenced host (a test fake): apply directly - return setter(); + var fenced = _host as DetailEditContextBase; + return fenced != null ? fenced.Stage(setter, _fieldLabel) : setter(); } - // Chooser / reference-vector / validation are not part of the reversal text editor; delegate - // the session boundary to the host so the view's Save/Cancel still drive commit/rollback. + public bool IsOpen => _host != null && _host.IsOpen; + + public bool TrySetText(DetailField field, string ws, string value) => false; + + public bool TrySetRichText(DetailField field, string ws, DetailRichTextValue value) => false; + public bool TrySetOption(DetailField field, string optionKey) => false; public bool TryAddReferenceItem(DetailField field, string optionKey) => false; @@ -203,9 +367,6 @@ private bool StageOnHost(Func setter) public bool TryResetReferenceOrder(DetailField field) => false; - // The Reversal Entries plugin edits multi-unicode reversal forms only; it implements neither - // IStructuredTextEditing (no StText rows are composed for it) nor picture editing. - public IReadOnlyList Validate() => _host?.Validate() ?? Array.Empty(); public void Commit() => _host?.Commit(); diff --git a/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs b/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs index 84048e40f1..d2dcf2c002 100644 --- a/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs +++ b/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs @@ -42,7 +42,8 @@ public interface ISlicePlugin /// the row's object and typed node, the detail view's edit context (resolved lazily through the /// composer's deferred accessor -- the context object is created during compose, BEFORE the /// edit context exists; plugin factories run at render time, after), the cache, and the - /// host's writing-system focus callback. + /// host's writing-system focus and link callbacks, the view's abbreviation-column width, + /// and the row's writing-system restriction. /// public sealed class SlicePluginBuildContext { @@ -50,13 +51,19 @@ public sealed class SlicePluginBuildContext public SlicePluginBuildContext(ICmObject target, ViewNode node, Func editContextAccessor, LcmCache cache, - Action writingSystemFocused = null) + Action writingSystemFocused = null, + Action linkRequested = null, + double? wsAbbrevColumnWidth = null, + IReadOnlyList visibleWritingSystems = null) { Target = target; Node = node; _editContextAccessor = editContextAccessor; Cache = cache; WritingSystemFocused = writingSystemFocused; + LinkRequested = linkRequested; + WsAbbrevColumnWidth = wsAbbrevColumnWidth; + VisibleWritingSystems = visibleWritingSystems; } /// The composed row's own object (the slice's object in legacy terms). @@ -75,6 +82,24 @@ public SlicePluginBuildContext(ICmObject target, ViewNode node, /// editor that gained focus. Null when the host supplies none. /// public Action WritingSystemFocused { get; } + + /// + /// The host's jump callback, the same one chooser links use: the host settles the + /// open edit session, then follows the link. Null when the host supplies none. + /// + public Action LinkRequested { get; } + + /// + /// The width the view gives its writing-system abbreviation column, so a plugin's own + /// abbreviations line up with the other rows. Null when the host supplies none. + /// + public double? WsAbbrevColumnWidth { get; } + + /// + /// The writing-system ids the row is restricted to, in order. Null or empty means no + /// restriction, which is also the case while the user has asked to see all of them. + /// + public IReadOnlyList VisibleWritingSystems { get; } } /// @@ -144,10 +169,8 @@ private static SlicePluginRegistry CreateDefault() return registry; } - // The builtin plugin list. The Reversal Entries slice - // (ReversalIndexEntrySlice) composes as a native Avalonia editable multi-WS text field through - // the plugin route. Every OTHER custom slice not absorbed by a composer route resolves to the - // labeled Unsupported worklist row. + // The builtin plugin list: the Reversal Entries slice (ReversalIndexEntrySlice). + // Every other custom slice not absorbed by a composer route renders Unsupported. internal static void RegisterBuiltins(SlicePluginRegistry registry) { registry.Register(new ReversalIndexEntryPlugin()); diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index 5d471d48e2..ca96a67e6b 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs @@ -1116,86 +1116,6 @@ public void RetagWritingSystem_RoundTripsKtptWs_OverTheSpan_AndCommits() Is.EqualTo(Cache.DefaultAnalWs), "the untouched tail keeps its original writing system"); } - // A sense with a reversal entry composes an EDITABLE reversal - // row through the ReversalIndexEntryPlugin -- not the lone Unsupported row. - [Test] - public void Compose_SenseWithReversalEntry_ComposesEditableReversalRow_NotUnsupported() - { - NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => - { - var revIndex = Cache.ServiceLocator.GetInstance() - .FindOrCreateIndexForWs(Cache.DefaultAnalWs); - var riEntry = revIndex.FindOrCreateReversalEntry("dwelling"); - riEntry.SensesRS.Add(m_entry.SensesOS[0]); - }); - - var composed = DetailComposer.Compose(m_entry, Cache); - - // The reversal slice composes a Custom (plugin) row, never an Unsupported row. - var reversalRow = composed.Model.Fields - .FirstOrDefault(f => f.Kind == DetailFieldKind.Custom - && (f.Field == "ReferringReversalIndexEntries" - || (f.Label != null && f.Label.IndexOf("Reversal", System.StringComparison.OrdinalIgnoreCase) >= 0))); - Assert.That(reversalRow, Is.Not.Null, "the reversal slice composes as a plugin (Custom) row"); - Assert.That(reversalRow.ControlFactory, Is.Not.Null, "the row carries the plugin control factory"); - - // Building the control yields an editable FwMultiWsTextField (not the unsupported rendering). - var control = reversalRow.ControlFactory(); - Assert.That(control, - Is.InstanceOf(), - "the reversal plugin builds the editable multi-WS reversal-forms field"); - } - - // Editing a reversal form stages/commits through the edit - // context -- the reversal entry's ReversalForm is updated, on the same fenced session as - // the detail view. - [Test] - public void ReversalPlugin_EditingAForm_StagesAndCommitsThroughTheEditContext() - { - IReversalIndexEntry riEntry = null; - NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => - { - var revIndex = Cache.ServiceLocator.GetInstance() - .FindOrCreateIndexForWs(Cache.DefaultAnalWs); - riEntry = revIndex.FindOrCreateReversalEntry("dwelling"); - riEntry.SensesRS.Add(m_entry.SensesOS[0]); - }); - - var composed = DetailComposer.Compose(m_entry, Cache); - var sense = m_entry.SensesOS[0]; - var plugin = new ReversalIndexEntryPlugin(); - // Reuse the composer's resolved node so the plugin gets real metadata; resolve via the plugin - // directly with a build context closing over the composed edit context. - var node = composed.Model.Fields.First(f => f.Kind == DetailFieldKind.Custom - && f.ObjectHvo == sense.Hvo); - var buildContext = new SlicePluginBuildContext(sense, null, () => composed.EditContext, Cache); - var reversalControl = (SIL.FieldWorks.Common.FwAvalonia.Detail.FwMultiWsTextField) - plugin.BuildControl(buildContext); - Assert.That(reversalControl, Is.Not.Null); - - // Stage an edit through the reversal context exposed by building the field's own context. We - // drive the edit through the composed edit context indirectly: the field staged via the - // plugin's ReversalDetailEditContext. Address it directly to assert the data path. - var field = new SIL.FieldWorks.Common.FwAvalonia.Detail.DetailField( - "reversal/" + sense.Hvo, "Reversal Entries", "ReferringReversalIndexEntries", null, - SIL.FieldWorks.Common.FwAvalonia.Detail.DetailFieldKind.Text, - SIL.FieldWorks.Common.FwAvalonia.ViewDefinition.EditorClassification.Known, - "ReversalEntriesEditor", null, - SIL.FieldWorks.Common.FwAvalonia.ViewDefinition.HostRouting.Product, null, null, null); - var analTag = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem.Id; - var entryByWsKey = new Dictionary { [analTag] = riEntry }; - var reversalContext = new ReversalDetailEditContext(Cache, composed.EditContext, entryByWsKey); - - Assert.That(reversalContext.TrySetText(field, analTag, "abode"), Is.True, - "editing an existing reversal form stages through the reversal edit context"); - reversalContext.Commit(); - - Assert.That(riEntry.ReversalForm.get_String(Cache.DefaultAnalWs).Text, Is.EqualTo("abode"), - "the committed edit updated the reversal entry's form"); - Assert.That(Cache.ActionHandlerAccessor.CanUndo(), Is.True, - "the reversal edit lands on the shared global undo stack"); - } - // DATA-SAFETY: ToTsString of an UNEDITED value reproduces the original // TsString exactly via the lossless RichXml fast-path. [Test] diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs new file mode 100644 index 0000000000..78b2badde6 --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -0,0 +1,562 @@ +// 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 System.Linq; +using NUnit.Framework; +using SIL.FieldWorks.Common.FwAvalonia; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.LCModel; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Core.WritingSystems; +using SIL.LCModel.DomainServices; +using SIL.LCModel.Infrastructure; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// The sense's Reversal Entries row (LT-22673) against real LCModel data: how the row + /// composes, what each row commit does to the reversal entries, and where a row's jump + /// lands. + /// + [TestFixture] + public class ReversalEntriesComposeTests : MemoryOnlyBackendProviderTestBase + { + private const string ReversalField = "ReferringReversalIndexEntries"; + private ILexEntry m_entry; + private ILexSense m_sense; + private IReversalIndex m_enIndex; + + public override void TestSetup() + { + base.TestSetup(); + // Real controls need the headless platform and its theme resources. + FwAvaloniaRuntime.EnsureInitialized(); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + // The project outlives each test, so start every test from empty reversal + // indexes. + var indexes = Cache.LanguageProject.LexDbOA.ReversalIndexesOC; + foreach (var index in indexes.ToList()) + indexes.Remove(index); + m_entry = Cache.ServiceLocator.GetInstance().Create(); + var morph = Cache.ServiceLocator.GetInstance().Create(); + m_entry.LexemeFormOA = morph; + morph.Form.set_String(Cache.DefaultVernWs, TsStringUtils.MakeString("casa", Cache.DefaultVernWs)); + m_sense = Cache.ServiceLocator.GetInstance().Create(); + m_entry.SensesOS.Add(m_sense); + m_enIndex = Cache.ServiceLocator.GetInstance() + .FindOrCreateIndexForWs(Cache.DefaultAnalWs); + }); + } + + private int EnWs => Cache.DefaultAnalWs; + + private string EnTag => Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem.Id; + + private int UndoCount => Cache.ActionHandlerAccessor.UndoableSequenceCount; + + private IReversalIndexEntry AddEntry(IReversalIndex index, string form, params ILexSense[] senses) + { + IReversalIndexEntry entry = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + entry = index.FindOrCreateReversalEntry(form); + foreach (var sense in senses) + entry.SensesRS.Add(sense); + }); + return entry; + } + + private IReversalIndexEntry AddSubentry(IReversalIndexEntry parent, string form, params ILexSense[] senses) + { + IReversalIndexEntry entry = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + entry = Cache.ServiceLocator.GetInstance().Create(); + parent.SubentriesOS.Add(entry); + entry.ReversalForm.set_String(EnWs, form); + foreach (var sense in senses) + entry.SensesRS.Add(sense); + }); + return entry; + } + + private ILexSense AddOtherSense() + { + ILexSense sense = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + var entry = Cache.ServiceLocator.GetInstance().Create(); + sense = Cache.ServiceLocator.GetInstance().Create(); + entry.SensesOS.Add(sense); + }); + return sense; + } + + private CoreWritingSystemDefinition AddAnalysisWs(string tag, bool rightToLeft = false) + { + CoreWritingSystemDefinition ws = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + WritingSystemServices.FindOrCreateWritingSystem(Cache, null, tag, false, false, out ws); + ws.RightToLeftScript = rightToLeft; + Cache.LangProject.AddToCurrentAnalysisWritingSystems(ws); + }); + return ws; + } + + private IReversalIndex AddIndex(CoreWritingSystemDefinition ws) + { + IReversalIndex index = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + index = Cache.ServiceLocator.GetInstance().FindOrCreateIndexForWs(ws.Handle)); + return index; + } + + // The host is the detail view's own fenced context, so commits reach the real undo stack. + private (ReversalDetailEditContext Editing, IDetailEditContext Host) NewContext(ILexSense sense = null) + { + var host = DetailComposer.Compose(m_entry, Cache).EditContext; + return (new ReversalDetailEditContext(Cache, host, sense ?? m_sense, "Reversal Entries"), host); + } + + private static DetailReversalGroup Group(IReadOnlyList groups, string wsTag) + => groups.Single(g => g.WsTag == wsTag); + + private static DetailReversalRow AddRow(DetailReversalGroup group) => group.Rows.Single(r => r.IsAddSlot); + + private static List EntryTexts(DetailReversalGroup group) + => group.Rows.Where(r => !r.IsAddSlot).Select(r => r.Text).ToList(); + + private DetailField ReversalRow(bool showHidden = false) + => DetailComposer.Compose(m_entry, Cache, showHidden).Model.Fields + .SingleOrDefault(f => f.Kind == DetailFieldKind.Custom && f.Field == ReversalField); + + // ----- Compose ----- + + [Test] + public void SenseWithEntries_ComposesTheReversalField() + { + AddEntry(m_enIndex, "dwelling", m_sense); + + var row = ReversalRow(); + + Assert.That(row, Is.Not.Null, "a sense with an entry composes the plugin row"); + Assert.That(row.ControlFactory(new SliceFactoryContext()), Is.InstanceOf(), + "the row builds the reversal editor, not the Unsupported text"); + } + + [Test] + public void EntriesInOneIndex_ShareAGroup_ThenOneAddRow() + { + AddEntry(m_enIndex, "dwelling", m_sense); + AddEntry(m_enIndex, "abode", m_sense); + + var group = Group(NewContext().Editing.CreateGroups(null), EnTag); + + Assert.That(EntryTexts(group), Is.EquivalentTo(new[] { "dwelling", "abode" })); + Assert.That(group.Rows.Count(r => r.IsAddSlot), Is.EqualTo(1)); + Assert.That(group.Rows.Last().IsAddSlot, Is.True, "the add row comes last"); + Assert.That(group.WsAbbrev, Is.EqualTo(Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem.Abbreviation)); + } + + [Test] + public void EntriesInTwoIndexes_ComposeTwoGroups_InAnalysisOrder() + { + var es = AddAnalysisWs("es"); + AddEntry(m_enIndex, "dwelling", m_sense); + AddEntry(AddIndex(es), "casa", m_sense); + + var groups = NewContext().Editing.CreateGroups(null); + + Assert.That(groups.Select(g => g.WsTag), Is.EqualTo(new[] { EnTag, es.Id })); + Assert.That(EntryTexts(Group(groups, es.Id)), Is.EqualTo(new[] { "casa" })); + } + + [Test] + public void Subentry_ShowsItsAncestorChain() + { + var top = AddEntry(m_enIndex, "top"); + var middle = AddSubentry(top, "middle"); + AddSubentry(middle, "leaf", m_sense); + + var group = Group(NewContext().Editing.CreateGroups(null), EnTag); + + Assert.That(EntryTexts(group), Is.EqualTo(new[] { "top: middle: leaf" })); + } + + [Test] + public void FormsInOtherWritingSystems_RideTheRowAsAlternatives() + { + // A variant of the index's own language, which is what an entry's alternatives use. + var enGb = AddAnalysisWs("en-GB"); + var entry = AddEntry(m_enIndex, "house", m_sense); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => entry.ReversalForm.set_String(enGb.Handle, "houze")); + + var group = Group(NewContext().Editing.CreateGroups(null), EnTag); + var row = group.Rows.Single(r => !r.IsAddSlot); + + Assert.That(row.OtherWsForms.Select(a => a.Text), Is.EqualTo(new[] { "houze" })); + Assert.That(row.OtherWsForms.Single().WsAbbrev, Is.EqualTo(enGb.Abbreviation)); + Assert.That(AddRow(group).OtherWsForms, Is.Empty, "an add row has no alternatives"); + } + + [Test] + public void AnalysisWsWithoutAnIndex_ComposesNoGroup_AndCreatesNoIndex() + { + var es = AddAnalysisWs("es"); + AddEntry(m_enIndex, "dwelling", m_sense); + var indexCount = Cache.LanguageProject.LexDbOA.ReversalIndexesOC.Count; + + var groups = NewContext().Editing.CreateGroups(null); + + Assert.That(groups.Select(g => g.WsTag), Has.None.EqualTo(es.Id)); + Assert.That(Cache.LanguageProject.LexDbOA.ReversalIndexesOC.Count, Is.EqualTo(indexCount), + "composing never creates a reversal index"); + } + + [Test] + public void NoEntries_HidesTheRow() + { + Assert.That(ReversalRow(), Is.Null, + "an ifdata row with no reversal entries composes nothing, not an Unsupported row"); + } + + [Test] + public void NoEntries_ShowHiddenFields_ComposesOnlyAddRows() + { + var row = ReversalRow(showHidden: true); + + Assert.That(row, Is.Not.Null, "Show Hidden Fields reveals the empty row"); + var groups = NewContext().Editing.CreateGroups(null); + Assert.That(groups.SelectMany(g => g.Rows).All(r => r.IsAddSlot), Is.True); + Assert.That(Group(groups, EnTag).Rows, Has.Count.EqualTo(1)); + } + + [Test] + public void EntriesOnlyInAHiddenWs_StillComposeTheRow() + { + var es = AddAnalysisWs("es"); + AddEntry(AddIndex(es), "casa", m_sense); + + Assert.That(ReversalRow(), Is.Not.Null, "an entry in any writing system is data"); + var groups = NewContext().Editing.CreateGroups(new[] { EnTag }); + Assert.That(groups.Select(g => g.WsTag), Is.EqualTo(new[] { EnTag })); + Assert.That(AddRow(Group(groups, EnTag)), Is.Not.Null, "the visible index offers its add row"); + } + + [Test] + public void EntriesInAHiddenWs_ProduceNoRow() + { + var es = AddAnalysisWs("es"); + AddEntry(m_enIndex, "dwelling", m_sense); + AddEntry(AddIndex(es), "casa", m_sense); + + var groups = NewContext().Editing.CreateGroups(new[] { EnTag }); + + Assert.That(groups.Select(g => g.WsTag), Is.EqualTo(new[] { EnTag })); + } + + // ----- Edit -> one undo step ----- + + [Test] + public void TypingInTheAddRow_CreatesAndLinksAnEntry_AsOneUndoStep() + { + AddEntry(m_enIndex, "dwelling", m_sense); + var (editing, host) = NewContext(); + var add = AddRow(Group(editing.CreateGroups(null), EnTag)); + var before = UndoCount; + + Assert.That(editing.TryCommitRow(add.RowKey, "home"), Is.True); + host.Commit(); + + Assert.That(UndoCount - before, Is.EqualTo(1)); + Assert.That(m_sense.ReferringReversalIndexEntries.Select(e => e.ReversalForm.get_String(EnWs).Text), + Is.EquivalentTo(new[] { "dwelling", "home" })); + var regrouped = Group(NewContext().Editing.CreateGroups(null), EnTag); + Assert.That(regrouped.Rows.Last().IsAddSlot, Is.True, "the re-shown group ends in a fresh add row"); + } + + [Test] + public void ColonChain_CreatesEntryAndSubentry_LinksTheDeepest() + { + var (editing, host) = NewContext(); + var add = AddRow(Group(editing.CreateGroups(null), EnTag)); + var before = UndoCount; + + editing.TryCommitRow(add.RowKey, "body: arm"); + host.Commit(); + + Assert.That(UndoCount - before, Is.EqualTo(1)); + var linked = m_sense.ReferringReversalIndexEntries.Single(); + Assert.That(linked.ReversalForm.get_String(EnWs).Text, Is.EqualTo("arm")); + Assert.That(linked.OwningEntry?.ReversalForm.get_String(EnWs).Text, Is.EqualTo("body")); + Assert.That(linked.OwningEntry.SensesRS, Does.Not.Contain(m_sense), "only the deepest entry is linked"); + Assert.That(EntryTexts(Group(NewContext().Editing.CreateGroups(null), EnTag)), + Is.EqualTo(new[] { "body: arm" })); + } + + [Test] + public void ColonChain_ReusesAnExistingParent() + { + var body = AddEntry(m_enIndex, "body"); + var (editing, host) = NewContext(); + var add = AddRow(Group(editing.CreateGroups(null), EnTag)); + + editing.TryCommitRow(add.RowKey, "body: arm"); + host.Commit(); + + Assert.That(m_enIndex.EntriesOC.Count(e => e.ReversalForm.get_String(EnWs).Text == "body"), Is.EqualTo(1), + "the existing parent is reused, not duplicated"); + Assert.That(m_sense.ReferringReversalIndexEntries.Single().OwningEntry, Is.SameAs(body)); + } + + [Test] + public void ColonChain_FindsTheChainUnderWhicheverHomographHasIt() + { + AddEntry(m_enIndex, "body"); + IReversalIndexEntry second = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + second = Cache.ServiceLocator.GetInstance().Create(); + m_enIndex.EntriesOC.Add(second); + second.ReversalForm.set_String(EnWs, "body"); + }); + var arm = AddSubentry(second, "arm"); + var (editing, host) = NewContext(); + + editing.TryCommitRow(AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey, "body: arm"); + host.Commit(); + + Assert.That(m_sense.ReferringReversalIndexEntries.Single(), Is.SameAs(arm), + "a full match wins over a partial one"); + } + + [Test] + public void ClearingARow_UnlinksAndDeletesAnOrphan_AsOneUndoStep() + { + var entry = AddEntry(m_enIndex, "dwelling", m_sense); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + var before = UndoCount; + + Assert.That(editing.TryCommitRow(row.RowKey, string.Empty), Is.True); + host.Commit(); + + Assert.That(UndoCount - before, Is.EqualTo(1)); + Assert.That(m_sense.ReferringReversalIndexEntries, Is.Empty); + Assert.That(entry.IsValidObject, Is.False, "an entry with no senses and no subentries is deleted"); + } + + [Test] + public void ClearingARow_KeepsAnEntryAnotherSenseUses() + { + var other = AddOtherSense(); + var entry = AddEntry(m_enIndex, "dwelling", m_sense, other); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + + editing.TryCommitRow(row.RowKey, string.Empty); + host.Commit(); + + Assert.That(entry.IsValidObject, Is.True); + Assert.That(entry.SensesRS, Does.Contain(other)); + Assert.That(entry.SensesRS, Does.Not.Contain(m_sense)); + } + + [Test] + public void ClearingARow_KeepsAnEntryWithASubentry() + { + var other = AddOtherSense(); + var entry = AddEntry(m_enIndex, "body", m_sense); + var arm = AddSubentry(entry, "arm", other); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + + editing.TryCommitRow(row.RowKey, string.Empty); + host.Commit(); + + Assert.That(entry.IsValidObject, Is.True, "an entry that still has subentries stays"); + Assert.That(arm.SensesRS, Does.Contain(other)); + } + + [Test] + public void EditingASharedEntrysRow_RelinksWithoutRenaming() + { + var other = AddOtherSense(); + var shared = AddEntry(m_enIndex, "dwelling", m_sense, other); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + + editing.TryCommitRow(row.RowKey, "abode"); + host.Commit(); + + Assert.That(shared.ReversalForm.get_String(EnWs).Text, Is.EqualTo("dwelling"), "the shared entry keeps its form"); + Assert.That(shared.SensesRS, Does.Contain(other)); + Assert.That(m_sense.ReferringReversalIndexEntries.Single().ReversalForm.get_String(EnWs).Text, + Is.EqualTo("abode")); + } + + [Test] + public void UnchangedText_StagesNothing() + { + AddEntry(m_enIndex, "dwelling", m_sense); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + + Assert.That(editing.TryCommitRow(row.RowKey, "dwelling"), Is.False); + Assert.That(editing.TryCommitRow(AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey, string.Empty), + Is.False, "an empty add row is not a change"); + Assert.That(host.IsOpen, Is.False, "no session opens for a no-op commit"); + } + + [Test] + public void ReeditingAJustAddedRow_EditsThatEntry() + { + var (editing, host) = NewContext(); + var add = AddRow(Group(editing.CreateGroups(null), EnTag)); + + editing.TryCommitRow(add.RowKey, "one"); + host.Commit(); + var first = m_sense.ReferringReversalIndexEntries.Single(); + editing.TryCommitRow(add.RowKey, "two"); + host.Commit(); + + Assert.That(m_sense.ReferringReversalIndexEntries.Select(e => e.ReversalForm.get_String(EnWs).Text), + Is.EqualTo(new[] { "two" }), "the row's second commit replaced its own entry"); + Assert.That(first.IsValidObject, Is.False, "the replaced entry was orphaned, so it is gone"); + } + + [Test] + public void AnAddedEntry_PersistsIntoTheNextCompose() + { + var (editing, host) = NewContext(); + editing.TryCommitRow(AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey, "home"); + host.Commit(); + + Assert.That(ReversalRow(), Is.Not.Null, "the row now has data, so it composes"); + Assert.That(EntryTexts(Group(NewContext().Editing.CreateGroups(null), EnTag)), Is.EqualTo(new[] { "home" })); + } + + // ----- Validation ----- + + [Test] + public void ACommit_TripsNoValidationRule() + { + var (editing, host) = NewContext(); + editing.TryCommitRow(AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey, "home"); + + Assert.That(host.Validate(), Is.Empty); + Assert.That(editing.Validate(), Is.Empty); + host.Commit(); + } + + // ----- Re-show ----- + + [Test] + public void AnExternalLinkChange_ShowsOnTheNextCompose() + { + var (editing, _) = NewContext(); + editing.CreateGroups(null); + AddEntry(m_enIndex, "dwelling", m_sense); + + Assert.That(EntryTexts(Group(NewContext().Editing.CreateGroups(null), EnTag)), Is.EqualTo(new[] { "dwelling" })); + } + + [Test] + public void UndoAndRedo_OfAnAdd_RoundTrip() + { + var (editing, host) = NewContext(); + editing.TryCommitRow(AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey, "home"); + host.Commit(); + + Cache.ActionHandlerAccessor.Undo(); + Assert.That(m_sense.ReferringReversalIndexEntries, Is.Empty); + Cache.ActionHandlerAccessor.Redo(); + Assert.That(m_sense.ReferringReversalIndexEntries.Single().ReversalForm.get_String(EnWs).Text, + Is.EqualTo("home")); + } + + // ----- Cluster / bidi ----- + + [Test] + public void RightToLeftIndex_RoundTripsItsForm() + { + var ar = AddAnalysisWs("ar", rightToLeft: true); + AddIndex(ar); + var (editing, host) = NewContext(); + var group = Group(editing.CreateGroups(null), ar.Id); + const string form = "بَيْت"; + + Assert.That(group.RightToLeft, Is.True); + editing.TryCommitRow(AddRow(group).RowKey, form); + host.Commit(); + + Assert.That(m_sense.ReferringReversalIndexEntries.Single().ReversalForm.get_String(ar.Handle).Text, + Is.EqualTo(form), "combining marks survive the colon split and the commit"); + } + + [TestCase(" body : arm ", new[] { "body", "arm" })] + [TestCase("::", new string[0])] + [TestCase("body::arm", new[] { "body", "arm" })] + [TestCase(":body:", new[] { "body" })] + [TestCase("", new string[0])] + public void SplitForms_TrimsAndDropsEmptyParts(string text, string[] expected) + { + Assert.That(ReversalDetailEditContext.SplitForms(text), Is.EqualTo(expected)); + } + + // ----- Navigation ----- + + [Test] + public void ATopLevelEntrysRow_JumpsToThatEntry() + { + var entry = AddEntry(m_enIndex, "dwelling", m_sense); + var (editing, _) = NewContext(); + var group = Group(editing.CreateGroups(null), EnTag); + + Assert.That(editing.TryResolveMainEntryGuid(group.Rows.First(r => !r.IsAddSlot).RowKey), + Is.EqualTo(entry.Guid)); + Assert.That(editing.TryResolveMainEntryGuid(AddRow(group).RowKey), Is.Null, "an add row has no entry"); + } + + [Test] + public void ASubentrysRow_JumpsToItsMainEntry() + { + var top = AddEntry(m_enIndex, "body"); + AddSubentry(top, "arm", m_sense); + var (editing, _) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + + Assert.That(editing.TryResolveMainEntryGuid(row.RowKey), Is.EqualTo(top.Guid)); + } + + [Test] + public void TheJumpRequest_TargetsTheReversalIndexTool() + { + var entry = AddEntry(m_enIndex, "dwelling", m_sense); + var requests = new List(); + + ReversalIndexEntryPlugin.RequestShowInReversalIndex(requests.Add, null, entry.Guid); + + Assert.That(requests.Single().Link.Tool, Is.EqualTo(ReversalIndexEntryPlugin.ReversalIndexTool)); + Assert.That(requests.Single().Link.TargetGuid, Is.EqualTo(entry.Guid.ToString())); + } + + [Test] + public void BuildControl_WithoutAHostEditContext_IsReadOnly() + { + AddEntry(m_enIndex, "dwelling", m_sense); + var context = new SlicePluginBuildContext(m_sense, null, () => null, Cache); + + var field = new ReversalIndexEntryPlugin().BuildControl(context); + + Assert.That(field, Is.InstanceOf(), + "with no edit session to stage on, the rows still show"); + } + } +} diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailWritingSystemStateTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailWritingSystemStateTests.cs index 456cc31680..07589e7a1c 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailWritingSystemStateTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailWritingSystemStateTests.cs @@ -150,10 +150,11 @@ public void PluginRowEditorFocus_UpdatesWritingSystemHvoProperty_ThroughPubSub() { window.Show(); Dispatcher.UIThread.RunJobs(); - // The plugin stamps . on each of its value boxes. + // The plugin stamps .. on each entry's box. var row = composed.Model.Fields.First(f => f.Kind == DetailFieldKind.Custom && f.ObjectHvo == sense.Hvo); - var boxId = (row.AutomationId ?? ReversalIndexEntryPlugin.DefaultAutomationId) + "." + analysis.Id; + var boxId = (row.AutomationId ?? ReversalIndexEntryPlugin.DefaultAutomationId) + "." + analysis.Id + + ".0"; var reversalBox = view.GetVisualDescendants().OfType() .FirstOrDefault(box => AutomationProperties.GetAutomationId(box) == boxId); Assert.That(reversalBox, Is.Not.Null, diff --git a/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs b/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs index d8477ccf00..4c40b02b1c 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs @@ -229,6 +229,8 @@ private sealed class FakeMessagesPlugin : ISlicePlugin public SIL.FieldWorks.Common.FwAvalonia.ViewDefinition.ViewNode LastNode; public IDetailEditContext LastEditContext; public LcmCache LastCache; + public Action LastLinkRequested; + public double? LastWsAbbrevColumnWidth; public string LegacyClassName => MessageSliceClassName; @@ -239,6 +241,8 @@ public Avalonia.Controls.Control BuildControl(SlicePluginBuildContext context) LastNode = context.Node; LastEditContext = context.EditContext; LastCache = context.Cache; + LastLinkRequested = context.LinkRequested; + LastWsAbbrevColumnWidth = context.WsAbbrevColumnWidth; return null; // never rendered in this fixture; the view's null guard covers this } } @@ -292,7 +296,7 @@ public void PluginRowFactory_ClosesOverObjectNodeCacheAndTheComposedEditContext( var composed = DetailComposer.Compose(m_entry, Cache, plugins: registry); var row = composed.Model.Fields.Single(f => f.Kind == DetailFieldKind.Custom); - row.ControlFactory(); + row.ControlFactory(null); Assert.That(plugin.BuildCalls, Is.EqualTo(1)); Assert.That(plugin.LastObject?.Hvo, Is.EqualTo(m_entry.Hvo)); @@ -301,5 +305,23 @@ public void PluginRowFactory_ClosesOverObjectNodeCacheAndTheComposedEditContext( Assert.That(plugin.LastEditContext, Is.SameAs(composed.EditContext), "the deferred accessor resolves to the detail view's own composed edit context"); } + + [Test] + public void PluginRowFactory_PassesTheRenderContextsLinkCallbackAndColumnWidth() + { + var registry = new SlicePluginRegistry(); + var plugin = new FakeMessagesPlugin(); + registry.Register(plugin); + var composed = DetailComposer.Compose(m_entry, Cache, plugins: registry); + var row = composed.Model.Fields.Single(f => f.Kind == DetailFieldKind.Custom); + Action linkRequested = request => { }; + + row.ControlFactory(new SliceFactoryContext(linkRequested: linkRequested, + wsAbbrevColumnWidth: 37)); + + Assert.That(plugin.LastLinkRequested, Is.SameAs(linkRequested), + "the plugin reaches the host's jump through the render context"); + Assert.That(plugin.LastWsAbbrevColumnWidth, Is.EqualTo(37)); + } } } From bf686227d0eabf2c332d7553c21233667896ebc3 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:37:11 -0500 Subject: [PATCH 02/12] fix misstaging --- Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 621fe4bc97..414919fddb 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -216,7 +216,6 @@ private Control CreateRow(string label, string groupId, DetailReversalGroup grou FlowDirection = group.RightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight, BorderThickness = new Thickness(0), Background = FwAvaloniaDensity.TransparentBrush, - TextWrapping = TextWrapping.Wrap TextWrapping = TextWrapping.NoWrap }; box = editor; From fb90fb24d155e700c5756fdc3933fbdb52b491d8 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:22:04 -0500 Subject: [PATCH 03/12] LT-22673: Grow, navigate and cascade-clean the reversal entry slots The reversal field now behaves like the WinForms slice it replaces: - The whole field saves once, when focus leaves it, as one undo step; moving between slots saves nothing. - Typing into a group's empty slot opens a fresh one after it, so several entries can be added in one visit. Each slot gets its own row key, so two new slots never overwrite each other. A new slot that is emptied again is removed when the user moves on. - Left and Right (alone or with Ctrl) at a slot's edge move into the neighboring slot, mirrored in right-to-left groups; Home and End go to the edges of the visual line; Enter does nothing. - Unlinking an entry also deletes each parent the deletion leaves with no senses and no subentries, so shortening "arm: hand: finger" to "arm" removes "hand" as well as "finger". - Slots keep a caret's width (the new DataTree.CaretAllowance token) past their text, so the caret shows at the end of a slot and in the empty add slot. Also adds exemplar-map rows for FwReversalEntriesField and the two plugin patterns it introduced. Co-Authored-By: Claude Opus 5.5 --- .../references/control-exemplar-map.md | 3 + .../Detail/FwReversalEntriesField.cs | 273 +++++++++++-- .../Detail/IReversalEntryEditing.cs | 21 +- Src/Common/FwAvalonia/FwAvaloniaDensity.cs | 4 + .../Detail/FwReversalEntriesFieldTests.cs | 375 ++++++++++++++++++ .../Tokens/DataTree/DataTreeTokens.axaml | 4 + .../Plugins/ReversalIndexEntryPlugin.cs | 21 +- .../Composer/ReversalEntriesComposeTests.cs | 94 +++++ 8 files changed, 753 insertions(+), 42 deletions(-) diff --git a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md index 9a70616062..24e11bccdb 100644 --- a/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md +++ b/.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md @@ -40,6 +40,7 @@ migration burden. | --- | --- | --- | | FwTextBox (41), LabeledMultiStringControl (5), MultiStringSlice (13) | `FwMultiWsTextField` — per-WS font, RTL, keyboard, abbreviation gutter, staged edits | `Src/Common/FwAvalonia/Detail/FwFieldControls.cs`; dialog usage: `InsertEntryDlgView.axaml` (lexeme form + gloss) | | FwMultiParaTextBox (1) / StTextSlice | `FwStructuredTextField` (multi-paragraph StText) | `Src/Common/FwAvalonia/Detail/FwStructuredTextField.cs` | +| ReversalIndexEntrySliceView (sense Reversal Entries; a RootSite-embedded free-text list) | `FwReversalEntriesField`: one wrapping line of free-text slots per writing-system group, separator bars, a live-growing add slot, colon-separated subentry chains, and one save (one undo step) when focus leaves the field; row edits go through the `IReversalEntryEditing` sub-capability | `Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs`; LCModel side: `Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs` | | TreeCombo (27) + PopupTree/PopupTreeManager (36) | popup tree picker | `Src/Common/FwAvalonia/FwPosChooser.cs` | | FeatureStructureTreeView (3) + `MsaInflectionFeatureListDlg` / `PhonologicalFeatureChooserDlg` | `FwFeatureStructureEditor` — LCModel-free `FsFeatStruc` tree editor: complex features expand to nested features, closed features to symbolic-value radios (one per feature) + a "None of the above" unspecified radio, plus inline create-feature / add-value affordances | `Src/Common/FwAvaloniaDialogs/FwFeatureStructureEditor.cs`; composed in `MSAGroupBox.cs` and the feature chooser (`FeatureChooserDialog*`) | | SimpleListChooser (26) / ReallySimpleListChooser (22) | `ChooserDialogView`/`ViewModel` (input/result DTOs, single- and multi-select) | `Src/Common/FwAvaloniaDialogs/ChooserDialog*` | @@ -70,6 +71,8 @@ migration burden. | Headless ViewModel/view tests | `FwAvaloniaDialogsTests/EntryGoDialogTests.cs` shapes; launcher-over-real-cache: `LcmLinkMsaDialogLauncherTests.cs` | | List-editor jump from a chooser row ("Edit the … list") | `DetailGearChrome.CreateConfigureGear` (row gear → `DetailLinkRequest` → `FollowLink`); links built by `DetailComposer.CreateChooserLinks`. **Approved divergence:** the detail view synthesizes the link for ANY possibility-list row the layout left linkless and puts it on the row, where legacy synthesizes only for `autoCustom` and puts it inside the chooser dialog. Reason and approver are in the `CreateChooserLinks` doc — do not "fix" it back to the legacy trigger. | | Adding to a reference-vector field | `FwReferenceVectorField`'s "+" opens `FwOptionChooser` in multi-select; the per-item right-click removes. **Approved divergence:** current members ride as UNAVAILABLE keys, so they list greyed and un-toggleable rather than pre-checked; legacy's `SimpleListChooser` pre-checks them and treats the dialog as setting the whole membership. "+" means add, and the row owns removal. Reason and approver are in the `FwReferenceVectorField` class doc. | +| A slice plugin that needs host services when it builds its control (for example a jump to another tool) | `DetailField.ControlFactory` receives the render-time `SliceFactoryContext`; `SlicePluginBuildContext.LinkRequested` → `RecordEditView.OnDetailLinkRequested` (saves pending edits, then posts `FollowLink`). Consumer: `ReversalIndexEntryPlugin` | +| `visibility="ifdata"` on a custom (plugin) slice | the composer checks for data before building the plugin row (`DetailComposer.PluginFieldHasData`: an empty list field hides the row; a field it can't resolve or classify still shows). Plugins need no hook of their own | ## 3. Gap register — the first implementation becomes the exemplar diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 414919fddb..5c67a257a3 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Avalonia; using Avalonia.Automation; using Avalonia.Controls; @@ -12,6 +13,7 @@ using Avalonia.Interactivity; using Avalonia.Layout; using Avalonia.Media; +using Avalonia.VisualTree; namespace SIL.FieldWorks.Common.FwAvalonia.Detail { @@ -111,15 +113,19 @@ public DetailReversalGroup(string wsTag, string wsAbbrev, string fontFamily, boo /// FieldWorks-owned editor for a sense's reversal entries. Each reversal index is a group /// labeled with its writing system abbreviation: one wrapping line of editable slots, one /// per linked entry and a final empty one for adding, with a bar between neighboring - /// slots. A row commits once, when it loses focus, - /// through ; the rows are a snapshot that the host - /// rebuilds after its save. Right-clicking a row offers "Show in Reversal Index", and - /// Ctrl+click runs it directly. An edit context without + /// slots. Typing into the empty slot opens a fresh one after it, and an add slot emptied + /// again disappears once the user moves on. Moving between slots commits nothing; when + /// focus leaves the field, every changed + /// slot commits through , as one edit. The rows are a + /// snapshot that the host rebuilds after its save. Right-clicking a row offers "Show in + /// Reversal Index", and Ctrl+click runs it directly. An edit context without /// shows the rows read-only. /// public sealed class FwReversalEntriesField : StackPanel, IDisposable { private readonly List _teardown = new List(); + private readonly List _rowCommits = new List(); + private readonly List _groups = new List(); private readonly IReversalEntryEditing _editing; private readonly Action _navigationRequested; private bool _disposed; @@ -154,39 +160,67 @@ public FwReversalEntriesField(string label, string automationId, Children.Add(CreateGroup(name, automationId, group, writingSystemFocused, abbrevWidth)); } + // A text-sized editor clips its own caret at the end, and fits none at all when empty, so + // a slot measures a little wider than its text. + private sealed class SlotTextBox : TextBox + { + protected override Type StyleKeyOverride => typeof(TextBox); + + protected override Size MeasureOverride(Size availableSize) + { + var size = base.MeasureOverride(availableSize); + return new Size(size.Width + FwAvaloniaDensity.CaretAllowance, size.Height); + } + } + + // One group's live line of slots, which grows as the user types into its last one. + private sealed class GroupState + { + public DetailReversalGroup Group; + public string GroupId; + public string Label; + public Action WritingSystemFocused; + public WrapPanel Slots; + public TextBox TrailingAdd; + public int AddedSlots; + } + private Control CreateGroup(string label, string automationId, DetailReversalGroup group, Action writingSystemFocused, double abbrevWidth) { - var groupId = automationId + "." + group.WsTag; - // The group's entries run together on one wrapping line, a bar between each pair, - // the add row last. - var rows = new WrapPanel + var state = new GroupState { - Orientation = Orientation.Horizontal, - Background = FwAvaloniaDensity.TransparentBrush, - FlowDirection = group.RightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight + Group = group, + GroupId = automationId + "." + group.WsTag, + Label = label, + WritingSystemFocused = writingSystemFocused, + // The group's slots run together on one wrapping line, a bar between each pair, + // the add slot last. + Slots = new WrapPanel + { + Orientation = Orientation.Horizontal, + Background = FwAvaloniaDensity.TransparentBrush, + FlowDirection = group.RightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight + } }; - AutomationProperties.SetAutomationId(rows, groupId); - TextBox addBox = null; + _groups.Add(state); + var rows = state.Slots; + AutomationProperties.SetAutomationId(rows, state.GroupId); for (var i = 0; i < group.Rows.Count; i++) { - if (i > 0) - rows.Children.Add(FwReferenceVectorField.CreateSeparatorBar()); var row = group.Rows[i]; - rows.Children.Add(CreateRow(label, groupId, group, row, i, writingSystemFocused, out var box)); - if (row.IsAddSlot) - addBox = box; + AppendSlot(state, row, row.IsAddSlot ? state.GroupId + ".Add" : state.GroupId + "." + i); } - if (_editing != null && addBox != null) + if (_editing != null) { - // An empty add row is barely wider than its caret, so a click anywhere on the + // An empty add slot is barely wider than its caret, so a click anywhere on the // group's free space starts typing there. EventHandler pressed = (s, e) => { - if (!ReferenceEquals(e.Source, rows)) + if (!ReferenceEquals(e.Source, rows) || state.TrailingAdd == null) return; - addBox.Focus(); + state.TrailingAdd.Focus(); e.Handled = true; }; rows.PointerPressed += pressed; @@ -202,11 +236,49 @@ private Control CreateGroup(string label, string automationId, DetailReversalGro return grid; } - private Control CreateRow(string label, string groupId, DetailReversalGroup group, - DetailReversalRow row, int index, Action writingSystemFocused, out TextBox box) + // Adds a slot to the end of the group's line, after a bar when the line is not empty. + private void AppendSlot(GroupState state, DetailReversalRow row, string rowId) + { + if (state.Slots.Children.Count > 0) + state.Slots.Children.Add(FwReferenceVectorField.CreateSeparatorBar()); + state.Slots.Children.Add(CreateRow(state, row, rowId, out var box)); + if (row.IsAddSlot) + state.TrailingAdd = box; + } + + // The first keystroke in the last add slot opens a fresh one after it, so several entries + // can be typed in one visit. Nothing is saved until focus leaves the field. + private void Grow(GroupState state, string addRowKey) + { + var key = _editing.IssueAddRowKey(addRowKey); + if (key == null) + return; + state.AddedSlots++; + AppendSlot(state, new DetailReversalRow(key, string.Empty, true), + state.GroupId + ".Add" + state.AddedSlots); + } + + // Drops an add slot the user emptied again before it was saved, with the bar that joined + // it to the line. + private static void RemoveSlot(GroupState state, Control slot) + { + var children = state.Slots.Children; + var index = children.IndexOf(slot); + if (index < 0) + return; + children.RemoveAt(index); + if (index > 0) + children.RemoveAt(index - 1); + else if (children.Count > 0) + children.RemoveAt(0); + } + + private Control CreateRow(GroupState state, DetailReversalRow row, string rowId, out TextBox box) { - var rowId = row.IsAddSlot ? groupId + ".Add" : groupId + "." + index; - var editor = new TextBox + var group = state.Group; + var label = state.Label; + var writingSystemFocused = state.WritingSystemFocused; + var editor = new SlotTextBox { Text = row.Text, Padding = FwAvaloniaDensity.EditorPadding, @@ -235,14 +307,40 @@ private Control CreateRow(string label, string groupId, DetailReversalGroup grou committed = text; }; + // The control this method returns; the slot removal below needs it. + Control slot = null; if (_editing != null) { + _rowCommits.Add(commit); // Runs before the host's own focus-loss save, which bubbles up from this box. - EventHandler lost = (s, e) => commit(); + // Moving between slots stages nothing, so the host saves only once focus leaves. + EventHandler lost = (s, e) => + { + if (row.IsAddSlot && !ReferenceEquals(editor, state.TrailingAdd) + && string.IsNullOrEmpty(editor.Text) && string.IsNullOrEmpty(committed)) + { + RemoveSlot(state, slot); + } + if (!FocusIsInside()) + CommitAll(); + }; editor.LostFocus += lost; _teardown.Add(() => editor.LostFocus -= lost); + + if (row.IsAddSlot) + { + EventHandler grow = (s, e) => + { + if (ReferenceEquals(editor, state.TrailingAdd) && !string.IsNullOrEmpty(editor.Text)) + Grow(state, row.RowKey); + }; + editor.TextChanged += grow; + _teardown.Add(() => editor.TextChanged -= grow); + } } + WireSlotNavigation(editor, group.RightToLeft); + if (writingSystemFocused != null && !string.IsNullOrEmpty(group.WsTag)) { EventHandler got = (s, e) => writingSystemFocused(group.WsTag); @@ -251,11 +349,14 @@ private Control CreateRow(string label, string groupId, DetailReversalGroup grou } if (_navigationRequested != null) - WireNavigation(editor, row, commit); + WireNavigation(editor, row); var suffix = CreateOtherWsSuffix(row, rowId + ".OtherWs"); if (suffix == null) - return editor; + { + slot = editor; + return slot; + } var panel = new StackPanel { Orientation = Orientation.Horizontal, @@ -263,15 +364,121 @@ private Control CreateRow(string label, string groupId, DetailReversalGroup grou }; panel.Children.Add(editor); panel.Children.Add(suffix); - return panel; + slot = panel; + return slot; } - // The jump commits the row first, so a form typed into the add row exists when shown. - private void WireNavigation(TextBox box, DetailReversalRow row, Action commit) + // Keys that treat the field's slots as one text. Enter does nothing. An arrow, alone or + // with Ctrl, at a slot's edge moves into the neighboring slot, across groups (in a + // right-to-left group the start is on the right). Plain Home and End go to the edges of + // the current visual line. + private void WireSlotNavigation(TextBox editor, bool rightToLeft) { - Action jump = () => + EventHandler keyDown = (s, e) => + { + if (e.Key == Key.Enter) + { + e.Handled = true; + return; + } + if ((e.Key == Key.Home || e.Key == Key.End) && e.KeyModifiers == KeyModifiers.None) + { + MoveToLineEdge(editor, e.Key == Key.Home); + e.Handled = true; + return; + } + if ((e.KeyModifiers & ~KeyModifiers.Control) != KeyModifiers.None + || (e.Key != Key.Left && e.Key != Key.Right)) + { + return; + } + if (editor.SelectionStart != editor.SelectionEnd) + return; + var toward = (e.Key == Key.Left) != rightToLeft ? -1 : 1; + var length = (editor.Text ?? string.Empty).Length; + if (toward < 0 ? editor.CaretIndex != 0 : editor.CaretIndex != length) + return; + + var slots = SlotEditors(); + var index = slots.IndexOf(editor) + toward; + if (index < 0 || index >= slots.Count) + return; + PlaceCaret(slots[index], toward < 0); + e.Handled = true; + }; + editor.AddHandler(InputElement.KeyDownEvent, keyDown, RoutingStrategies.Tunnel); + _teardown.Add(() => editor.RemoveHandler(InputElement.KeyDownEvent, keyDown)); + } + + // Home goes to the start of the first slot on the editor's visual line, End to the end of + // the last; the group's wrap panel puts every slot of one line at the same top. + private void MoveToLineEdge(TextBox editor, bool toStart) + { + foreach (var state in _groups) + { + var slots = state.Slots.Children + .Select(child => new { Slot = child, Editor = SlotEditor(child) }) + .Where(pair => pair.Editor != null) + .ToList(); + var current = slots.FirstOrDefault(pair => ReferenceEquals(pair.Editor, editor)); + if (current == null) + continue; + var line = slots.Where(pair => pair.Slot.Bounds.Y.Equals(current.Slot.Bounds.Y)).ToList(); + PlaceCaret((toStart ? line.First() : line.Last()).Editor, !toStart); + return; + } + } + + private static void PlaceCaret(TextBox target, bool atEnd) + { + target.Focus(); + var caret = atEnd ? (target.Text ?? string.Empty).Length : 0; + target.CaretIndex = caret; + target.SelectionStart = caret; + target.SelectionEnd = caret; + } + + // A slot is its editor, or a panel holding the editor and its read-only suffix. + private static TextBox SlotEditor(Control slot) + => slot as TextBox ?? (slot as Panel)?.Children.OfType().FirstOrDefault(); + + // The slot editors in reading order: group by group, each line from its first slot. + private List SlotEditors() + { + var editors = new List(); + foreach (var state in _groups) { + foreach (var child in state.Slots.Children) + { + var editor = SlotEditor(child); + if (editor != null) + editors.Add(editor); + } + } + return editors; + } + + // Every changed slot commits in order, joining the host's one open edit session. + private void CommitAll() + { + foreach (var commit in _rowCommits) commit(); + } + + // Focus has already moved when a slot's LostFocus runs, so this tells a move to another + // slot from leaving the field. + private bool FocusIsInside() + { + var focused = TopLevel.GetTopLevel(this)?.FocusManager?.GetFocusedElement() as Visual; + return focused != null && (ReferenceEquals(focused, this) || this.IsVisualAncestorOf(focused)); + } + + // The jump commits every slot first, so a form typed into the add row exists when shown. + private void WireNavigation(TextBox box, DetailReversalRow row) + { + Action jump = () => + { + CommitAll(); _navigationRequested(row.RowKey); }; diff --git a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs index ea7ee7d018..25b2648891 100644 --- a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs +++ b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs @@ -19,17 +19,26 @@ public interface IReversalEntryEditing /// /// Stages the text of one row, opening the edit session only when something changes. /// Text equal to what the row already shows changes nothing; empty text on an entry - /// row unlinks that entry (an entry left with no senses and no subentries is - /// deleted). Other text is split on colons into a chain of entry and subentry forms, - /// and the sense is linked to the deepest entry of that chain, found or created -- an - /// existing entry is never renamed. Afterwards the key names the row's new entry, or - /// an add row again after an unlink, so a second commit on the same row edits what - /// the first one produced. + /// row unlinks that entry. Other text is split on colons into a chain of entry and + /// subentry forms, and the sense is linked to the deepest entry of that chain, found or + /// created -- an existing entry is never renamed -- in place of the row's old entry. + /// An unlinked entry left with no senses and no subentries is deleted, and so is each + /// parent that deletion leaves with neither. Afterwards the key names the row's new + /// entry, or an add row again after an unlink, so a second commit on the same row edits + /// what the first one produced. /// /// False, without opening the session, for an unknown key, an empty add /// row, or unchanged text. bool TryCommitRow(string rowKey, string typedText); + /// + /// Issues the key of another add row in the same reversal index as + /// , for a slot the editor opens while the user types. Each add + /// row needs its own key, since a commit rebinds the key to the entry it produced. + /// + /// The new key, or null for an unknown key. + string IssueAddRowKey(string rowKey); + /// /// The guid of the top-level entry to show for a row: the row's own entry, or for a /// subentry its main entry. Null for an add row or an unknown key. diff --git a/Src/Common/FwAvalonia/FwAvaloniaDensity.cs b/Src/Common/FwAvalonia/FwAvaloniaDensity.cs index 4327872a7d..7ba9c13a5e 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaDensity.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaDensity.cs @@ -308,6 +308,10 @@ public static class FwAvaloniaDensity /// reference-vector items. public static double SeparatorBarWidth => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_SeparatorBarWidth); + /// Room a text-sized editor keeps past its text for the caret and a final + /// glyph's overhang. + public static double CaretAllowance => FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_CaretAllowance); + /// /// Corner radius for a compact bordered host (option/POS picker frame, MSA/feature group /// box); pairs with and diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs index 3b7598f31c..d074bf1b78 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -42,6 +42,10 @@ public bool TryCommitRow(string rowKey, string typedText) return true; } + private int _issued; + + public string IssueAddRowKey(string rowKey) => "en-add" + ++_issued; + public Guid? TryResolveMainEntryGuid(string rowKey) => Guid.Empty; public bool IsOpen => false; @@ -144,6 +148,27 @@ public void AGroupsSlots_ShareOneLine_WithABarBetweenEachPair() Is.GreaterThan(second.TranslatePoint(new Point(0, 0), window).Value.X), "the add slot comes last"); } + // A text-sized editor clips a caret at the end of its text, and an empty one has no room + // for a caret at all, so each slot's text area must be wider than its text. + [AvaloniaTest] + public void EverySlot_LeavesRoomForTheCaretAfterItsText() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("Antarctica")); + + foreach (var id in new[] { "Reversal.en.0", "Reversal.en.Add" }) + { + var box = Find(field, id); + box.Focus(); + box.CaretIndex = box.Text?.Length ?? 0; + Dispatcher.UIThread.RunJobs(); + var presenter = box.GetVisualDescendants() + .OfType().Single(); + + Assert.That(presenter.Bounds.Width, + Is.GreaterThanOrEqualTo(presenter.DesiredSize.Width + FwAvaloniaDensity.CaretAllowance), id); + } + } + [AvaloniaTest] public void ALongEntry_StaysOnOneLine_InsideItsSlot() { @@ -235,6 +260,356 @@ public void AChangedRow_CommitsOnceWhenItLosesFocus() Assert.That(context.Events, Has.Count.EqualTo(1), "leaving again without a change commits nothing more"); } + [AvaloniaTest] + public void MovingBetweenSlots_CommitsNothing_UntilFocusLeavesTheField() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English("dwelling")); + var entry = Find(field, "Reversal.en.0"); + var add = Find(field, "Reversal.en.Add"); + + entry.Focus(); + entry.Text = "abode"; + add.Focus(); + add.Text = "home"; + entry.Focus(); + Dispatcher.UIThread.RunJobs(); + Assert.That(context.Events, Is.Empty, "focus is still inside the field"); + + other.Focus(); + Dispatcher.UIThread.RunJobs(); + Assert.That(context.Events, Is.EqualTo(new[] { "commit en0=abode", "commit en-add=home" }), + "leaving the field commits every changed slot, in order"); + } + + [AvaloniaTest] + public void TypingIntoTheAddSlot_OpensAFreshOne_WithoutSaving() + { + var context = new RecordingReversalContext(); + var (field, _, _) = Show(context, null, English("dwelling")); + var add = Find(field, "Reversal.en.Add"); + + add.Focus(); + add.Text = "h"; + Dispatcher.UIThread.RunJobs(); + + var fresh = Find(field, "Reversal.en.Add1"); + Assert.That(fresh, Is.Not.Null, "the first keystroke opens another empty slot"); + Assert.That(fresh.Text, Is.Empty); + Assert.That(Find(field, "Reversal.en").Children.OfType().Count(), Is.EqualTo(2), + "the new slot is joined to the line by a bar"); + Assert.That(add.IsFocused, Is.True, "typing continues in the same slot"); + Assert.That(context.Events, Is.Empty, "nothing is saved while typing"); + } + + [AvaloniaTest] + public void FurtherKeystrokes_OpenNoMoreSlots() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English()); + var add = Find(field, "Reversal.en.Add"); + + add.Focus(); + add.Text = "h"; + add.Text = "ho"; + add.Text = "home"; + Dispatcher.UIThread.RunJobs(); + + Assert.That(field.GetVisualDescendants().OfType().Count(), Is.EqualTo(2)); + } + + [AvaloniaTest] + public void EachNewSlot_SavesWithItsOwnKey_WhenFocusLeaves() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English()); + + // TextChanged arrives through the dispatcher, so each keystroke is run before the + // next step. + var add = Find(field, "Reversal.en.Add"); + add.Focus(); + add.Text = "one"; + Dispatcher.UIThread.RunJobs(); + var second = Find(field, "Reversal.en.Add1"); + second.Focus(); + second.Text = "two"; + Dispatcher.UIThread.RunJobs(); + Assert.That(Find(field, "Reversal.en.Add2"), Is.Not.Null, "typing in the new slot opens a third"); + Assert.That(context.Events, Is.Empty); + + other.Focus(); + Dispatcher.UIThread.RunJobs(); + + Assert.That(context.Events, Is.EqualTo(new[] { "commit en-add=one", "commit en-add1=two" }), + "each typed slot saves under its own key; the empty last slot saves nothing"); + } + + [AvaloniaTest] + public void AnAddSlotEmptiedAgain_IsRemovedWhenTheUserMovesOn() + { + var context = new RecordingReversalContext(); + var (field, _, _) = Show(context, null, English("dwelling")); + var add = Find(field, "Reversal.en.Add"); + + add.Focus(); + add.Text = "h"; + Dispatcher.UIThread.RunJobs(); + add.Text = string.Empty; + Dispatcher.UIThread.RunJobs(); + Find(field, "Reversal.en.Add1").Focus(); + Dispatcher.UIThread.RunJobs(); + + Assert.That(Find(field, "Reversal.en.Add"), Is.Null); + Assert.That(Find(field, "Reversal.en").Children.OfType().Count(), Is.EqualTo(1), + "the removed slot takes its bar with it"); + Assert.That(context.Events, Is.Empty); + } + + private static void Press(TextBox box, Key key, KeyModifiers modifiers = KeyModifiers.None) + { + box.RaiseEvent(new KeyEventArgs + { + RoutedEvent = InputElement.KeyDownEvent, + Key = key, + KeyModifiers = modifiers, + Source = box + }); + Dispatcher.UIThread.RunJobs(); + } + + private static void PlaceCaret(TextBox box, int caret) + { + box.Focus(); + box.CaretIndex = caret; + box.SelectionStart = caret; + box.SelectionEnd = caret; + } + + private static DetailReversalGroup French(params string[] forms) + { + var rows = forms.Select((f, i) => new DetailReversalRow("fr" + i, f, false)).ToList(); + rows.Add(new DetailReversalRow("fr-add", string.Empty, true)); + return new DetailReversalGroup("fr", "Fre", null, false, rows); + } + + [AvaloniaTest] + public void LeftAtASlotsStart_MovesToTheEndOfThePreviousSlot() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + PlaceCaret(Find(field, "Reversal.en.1"), 0); + + Press(Find(field, "Reversal.en.1"), Key.Left); + + Assert.That(first.IsFocused, Is.True); + Assert.That(first.CaretIndex, Is.EqualTo("dwelling".Length)); + } + + [AvaloniaTest] + public void RightAtASlotsEnd_MovesToTheStartOfTheNextSlot() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var second = Find(field, "Reversal.en.1"); + var first = Find(field, "Reversal.en.0"); + PlaceCaret(first, "dwelling".Length); + + Press(first, Key.Right); + + Assert.That(second.IsFocused, Is.True); + Assert.That(second.CaretIndex, Is.Zero); + } + + [AvaloniaTest] + public void Enter_DoesNothing() + { + var context = new RecordingReversalContext(); + var (field, _, window) = Show(context, null, English("dwelling")); + var box = Find(field, "Reversal.en.0"); + var reachedHost = 0; + window.AddHandler(InputElement.KeyDownEvent, (s, e) => reachedHost++, RoutingStrategies.Bubble); + box.Focus(); + box.Text = "house"; + PlaceCaret(box, 2); + + Press(box, Key.Enter); + Press(box, Key.Enter, KeyModifiers.Control); + + Assert.That(reachedHost, Is.Zero, "Enter never reaches the view, so it saves nothing"); + Assert.That(box.Text, Is.EqualTo("house")); + Assert.That(box.IsFocused, Is.True); + Assert.That(context.Events, Is.Empty); + } + + [AvaloniaTest] + public void HomeAndEnd_GoToTheEdgesOfTheLine_AcrossSlots() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + var middle = Find(field, "Reversal.en.1"); + var add = Find(field, "Reversal.en.Add"); + + PlaceCaret(middle, 2); + Press(middle, Key.Home); + Assert.That(first.IsFocused, Is.True); + Assert.That(first.CaretIndex, Is.Zero); + + PlaceCaret(middle, 2); + Press(middle, Key.End); + Assert.That(add.IsFocused, Is.True, "the line ends with the add slot"); + Assert.That(add.CaretIndex, Is.Zero); + + PlaceCaret(first, 3); + Press(first, Key.End); + PlaceCaret(add, 0); + Press(add, Key.Home); + Assert.That(first.IsFocused, Is.True); + } + + [AvaloniaTest] + public void HomeAndEnd_StayOnTheirOwnLine_WhenTheGroupWraps() + { + var forms = Enumerable.Range(0, 12).Select(i => "dwellingplace" + i).ToArray(); + var (field, _, _) = Show(new RecordingReversalContext(), null, English(forms)); + var boxes = Enumerable.Range(0, forms.Length) + .Select(i => Find(field, "Reversal.en." + i)).ToList(); + var group = Find(field, "Reversal.en"); + double Top(TextBox box) => box.TranslatePoint(new Point(0, 0), group).Value.Y; + var firstLineTop = Top(boxes[0]); + var onSecondLine = boxes.Where(b => Top(b) > firstLineTop).ToList(); + Assert.That(onSecondLine, Is.Not.Empty, "precondition: the group wraps"); + var secondLine = onSecondLine.Where(b => Top(b).Equals(Top(onSecondLine[0]))).ToList(); + + var current = secondLine.Last(); + PlaceCaret(current, 1); + Press(current, Key.Home); + Assert.That(secondLine[0].IsFocused, Is.True, "Home goes to the start of this line, not the group"); + + PlaceCaret(boxes[0], 1); + Press(boxes[0], Key.End); + var firstLine = boxes.Where(b => Top(b).Equals(firstLineTop)).ToList(); + Assert.That(firstLine.Last().IsFocused, Is.True, "End goes to the end of this line"); + Assert.That(firstLine.Last().CaretIndex, Is.EqualTo(firstLine.Last().Text.Length)); + } + + [AvaloniaTest] + public void ModifiedHomeAndEnd_StayInTheSlot() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var middle = Find(field, "Reversal.en.1"); + + PlaceCaret(middle, 2); + Press(middle, Key.Home, KeyModifiers.Shift); + + Assert.That(middle.IsFocused, Is.True); + } + + [AvaloniaTest] + public void CtrlArrows_AtASlotsEdge_MoveBetweenSlotsToo() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + var second = Find(field, "Reversal.en.1"); + + PlaceCaret(second, 0); + Press(second, Key.Left, KeyModifiers.Control); + Assert.That(first.IsFocused, Is.True); + Assert.That(first.CaretIndex, Is.EqualTo("dwelling".Length)); + + PlaceCaret(first, "dwelling".Length); + Press(first, Key.Right, KeyModifiers.Control); + Assert.That(second.IsFocused, Is.True); + Assert.That(second.CaretIndex, Is.Zero); + } + + [AvaloniaTest] + public void ArrowsInsideASlot_StayInIt() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var second = Find(field, "Reversal.en.1"); + + PlaceCaret(second, 2); + Press(second, Key.Left); + Assert.That(second.IsFocused, Is.True, "the caret is not at the start"); + + PlaceCaret(second, 0); + Press(second, Key.Left, KeyModifiers.Shift); + Assert.That(second.IsFocused, Is.True, "a selecting arrow keeps its text behavior"); + + PlaceCaret(second, 2); + Press(second, Key.Left, KeyModifiers.Control); + Assert.That(second.IsFocused, Is.True, "Ctrl+Left inside the text moves within the slot"); + + second.Focus(); + second.SelectionStart = 0; + second.SelectionEnd = 3; + Press(second, Key.Left); + Assert.That(second.IsFocused, Is.True, "an arrow with a selection keeps its text behavior"); + } + + [AvaloniaTest] + public void ArrowsAtTheFieldsEnds_GoNowhere() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling")); + var first = Find(field, "Reversal.en.0"); + var add = Find(field, "Reversal.en.Add"); + + PlaceCaret(first, 0); + Press(first, Key.Left); + Assert.That(first.IsFocused, Is.True); + + PlaceCaret(add, 0); + Press(add, Key.Right); + Assert.That(add.IsFocused, Is.True); + } + + [AvaloniaTest] + public void RightAtAGroupsLastSlot_MovesIntoTheNextGroup() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling"), French("maison")); + var add = Find(field, "Reversal.en.Add"); + PlaceCaret(add, 0); + + Press(add, Key.Right); + + Assert.That(Find(field, "Reversal.fr.0").IsFocused, Is.True); + } + + [AvaloniaTest] + public void InARightToLeftGroup_TheArrowsMirror() + { + var arabic = new DetailReversalGroup("ar", "Ara", null, true, new[] + { + new DetailReversalRow("ar0", "بيت", false), + new DetailReversalRow("ar1", "دار", false), + new DetailReversalRow("ar-add", "", true) + }); + var (field, _, _) = Show(new RecordingReversalContext(), null, arabic); + var first = Find(field, "Reversal.ar.0"); + var second = Find(field, "Reversal.ar.1"); + + PlaceCaret(second, 0); + Press(second, Key.Right); + Assert.That(first.IsFocused, Is.True, "Right at the start moves back, since the start is on the right"); + + PlaceCaret(first, first.Text.Length); + Press(first, Key.Left); + Assert.That(second.IsFocused, Is.True, "Left at the end moves on"); + } + + [AvaloniaTest] + public void MovingBetweenSlotsByArrow_SavesNothing() + { + var context = new RecordingReversalContext(); + var (field, _, _) = Show(context, null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + first.Focus(); + first.Text = "house"; + PlaceCaret(first, first.Text.Length); + + Press(first, Key.Right); + + Assert.That(context.Events, Is.Empty); + } + [AvaloniaTest] public void AnUntouchedRow_NeverCommits() { diff --git a/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml b/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml index be5f6482ee..ee2263252b 100644 --- a/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml +++ b/Src/Common/FwAvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml @@ -162,6 +162,10 @@ (pairs with DataTree.SeparatorBarMargin). --> 2 + + 3 + diff --git a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs index 79bfe2616f..7db9434531 100644 --- a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs +++ b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs @@ -248,6 +248,15 @@ public bool TryCommitRow(string rowKey, string typedText) }); } + /// + public string IssueAddRowKey(string rowKey) + { + RowBinding binding; + if (string.IsNullOrEmpty(rowKey) || !_rows.TryGetValue(rowKey, out binding)) + return null; + return Bind(binding.Index, null); + } + /// public Guid? TryResolveMainEntryGuid(string rowKey) { @@ -336,12 +345,18 @@ private static void FindDeepest(IEnumerable candidates, ILi } } - // An entry left with no senses and no subentries is deleted. + // An entry left with no senses and no subentries is deleted, and so is each ancestor the + // deletion leaves with neither. private void Unlink(IReversalIndexEntry entry) { entry.SensesRS.Remove(_sense); - if (entry.SensesRS.Count == 0 && entry.SubentriesOS.Count == 0) - entry.Delete(); + var level = entry; + while (level != null && level.SensesRS.Count == 0 && level.SubentriesOS.Count == 0) + { + var parent = level.OwningEntry; + level.Delete(); + level = parent; + } } // A host that is not the fenced detail context (a test fake) applies the write directly. diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs index 78b2badde6..0517e02f25 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -384,6 +384,61 @@ public void ClearingARow_KeepsAnEntryWithASubentry() Assert.That(arm.SensesRS, Does.Contain(other)); } + [Test] + public void ShorteningAChain_DeletesTheAncestorsItLeavesEmpty() + { + var (editing, host) = NewContext(); + var key = AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey; + editing.TryCommitRow(key, "arm: hand: finger"); + host.Commit(); + var finger = m_sense.ReferringReversalIndexEntries.Single(); + var hand = finger.OwningEntry; + var arm = hand.OwningEntry; + + editing.TryCommitRow(key, "arm"); + host.Commit(); + + Assert.That(m_sense.ReferringReversalIndexEntries.Single(), Is.SameAs(arm)); + Assert.That(finger.IsValidObject, Is.False); + Assert.That(hand.IsValidObject, Is.False, "the parent left with no senses and no subentries goes too"); + } + + [Test] + public void ClearingAChain_DeletesEveryLevelLeftEmpty() + { + var (editing, host) = NewContext(); + var key = AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey; + editing.TryCommitRow(key, "arm: hand: finger"); + host.Commit(); + var finger = m_sense.ReferringReversalIndexEntries.Single(); + var hand = finger.OwningEntry; + var arm = hand.OwningEntry; + + editing.TryCommitRow(key, string.Empty); + host.Commit(); + + Assert.That(new[] { finger, hand, arm }.Any(e => e.IsValidObject), Is.False); + } + + [Test] + public void TheCascade_StopsAtAnAncestorStillInUse() + { + var other = AddOtherSense(); + var arm = AddEntry(m_enIndex, "arm", other); + var hand = AddSubentry(arm, "hand"); + AddSubentry(hand, "palm", other); + var finger = AddSubentry(hand, "finger", m_sense); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.First(r => !r.IsAddSlot); + + editing.TryCommitRow(row.RowKey, string.Empty); + host.Commit(); + + Assert.That(finger.IsValidObject, Is.False); + Assert.That(hand.IsValidObject, Is.True, "hand still has the subentry palm"); + Assert.That(arm.IsValidObject, Is.True, "arm still has another sense"); + } + [Test] public void EditingASharedEntrysRow_RelinksWithoutRenaming() { @@ -431,6 +486,45 @@ public void ReeditingAJustAddedRow_EditsThatEntry() Assert.That(first.IsValidObject, Is.False, "the replaced entry was orphaned, so it is gone"); } + [Test] + public void EditsToSeveralSlots_InOneFieldVisit_AreOneUndoStep() + { + AddEntry(m_enIndex, "dwelling", m_sense); + var (editing, host) = NewContext(); + var group = Group(editing.CreateGroups(null), EnTag); + var before = UndoCount; + + editing.TryCommitRow(group.Rows.First(r => !r.IsAddSlot).RowKey, "abode"); + editing.TryCommitRow(AddRow(group).RowKey, "home"); + host.Commit(); + + Assert.That(UndoCount - before, Is.EqualTo(1), "the field's edits share one undo step"); + Assert.That(m_sense.ReferringReversalIndexEntries.Select(e => e.ReversalForm.get_String(EnWs).Text), + Is.EquivalentTo(new[] { "abode", "home" })); + Cache.ActionHandlerAccessor.Undo(); + Assert.That(m_sense.ReferringReversalIndexEntries.Select(e => e.ReversalForm.get_String(EnWs).Text), + Is.EquivalentTo(new[] { "dwelling" }), "one Ctrl+Z undoes the whole visit"); + } + + [Test] + public void IssuedAddKeys_AddSeparateEntries_InOneUndoStep() + { + var (editing, host) = NewContext(); + var add = AddRow(Group(editing.CreateGroups(null), EnTag)); + var second = editing.IssueAddRowKey(add.RowKey); + var before = UndoCount; + + Assert.That(second, Is.Not.Null.And.Not.EqualTo(add.RowKey)); + editing.TryCommitRow(add.RowKey, "one"); + editing.TryCommitRow(second, "two"); + host.Commit(); + + Assert.That(UndoCount - before, Is.EqualTo(1)); + Assert.That(m_sense.ReferringReversalIndexEntries.Select(e => e.ReversalForm.get_String(EnWs).Text), + Is.EquivalentTo(new[] { "one", "two" }), "the second slot adds, it does not replace the first"); + Assert.That(editing.IssueAddRowKey("no-such-key"), Is.Null); + } + [Test] public void AnAddedEntry_PersistsIntoTheNextCompose() { From fb1524220e61ff2ae2655d97da03bcea3f63e650 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Fri, 25 Sep 2026 17:52:23 -0500 Subject: [PATCH 04/12] Claude cleaning up & fixing tests - in progress --- .../Detail/FwReversalEntriesField.cs | 147 ++++++++++++++++-- .../Detail/IReversalEntryEditing.cs | 29 ++-- .../Detail/FwReversalEntriesFieldTests.cs | 113 ++++++++++++-- .../Avalonia/Composer/DetailComposer.cs | 5 +- Src/xWorks/Avalonia/DetailEditContextBase.cs | 22 +++ .../Avalonia/DetailEditContextHolder.cs | 15 ++ .../Plugins/ReversalIndexEntryPlugin.cs | 130 ++++++++++++---- Src/xWorks/Avalonia/Plugins/SlicePlugins.cs | 25 ++- .../Composer/ReversalEntriesComposeTests.cs | 116 +++++++++++++- .../Plugins/LexemeEditorInventoryTests.cs | 18 +-- 10 files changed, 521 insertions(+), 99 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 5c67a257a3..3bba39f24d 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -124,7 +124,7 @@ public DetailReversalGroup(string wsTag, string wsAbbrev, string fontFamily, boo public sealed class FwReversalEntriesField : StackPanel, IDisposable { private readonly List _teardown = new List(); - private readonly List _rowCommits = new List(); + private readonly List _slots = new List(); private readonly List _groups = new List(); private readonly IReversalEntryEditing _editing; private readonly Action _navigationRequested; @@ -173,6 +173,72 @@ protected override Size MeasureOverride(Size availableSize) } } + // Wraps its children onto lines the way a horizontal WrapPanel does, then widens the last + // child, the group's add slot, across whatever its line has left. + private sealed class SlotLinePanel : Panel + { + protected override Size MeasureOverride(Size availableSize) + { + double width = 0, height = 0, lineWidth = 0, lineHeight = 0; + foreach (var child in Children) + { + child.Measure(availableSize); + var size = child.DesiredSize; + if (lineWidth > 0 && lineWidth + size.Width > availableSize.Width) + { + width = Math.Max(width, lineWidth); + height += lineHeight; + lineWidth = 0; + lineHeight = 0; + } + lineWidth += size.Width; + lineHeight = Math.Max(lineHeight, size.Height); + } + width = Math.Max(width, lineWidth); + height += lineHeight; + return new Size(double.IsInfinity(availableSize.Width) ? width : availableSize.Width, height); + } + + protected override Size ArrangeOverride(Size finalSize) + { + var lines = new List>(); + var line = new List(); + double lineWidth = 0; + foreach (var child in Children) + { + var childWidth = child.DesiredSize.Width; + if (line.Count > 0 && lineWidth + childWidth > finalSize.Width) + { + lines.Add(line); + line = new List(); + lineWidth = 0; + } + line.Add(child); + lineWidth += childWidth; + } + if (line.Count > 0) + lines.Add(line); + + var last = Children.Count > 0 ? Children[Children.Count - 1] : null; + double y = 0; + foreach (var current in lines) + { + var lineHeight = current.Max(child => child.DesiredSize.Height); + double x = 0; + foreach (var child in current) + { + var childWidth = child.DesiredSize.Width; + if (ReferenceEquals(child, last)) + childWidth = Math.Max(childWidth, finalSize.Width - x); + child.Arrange(new Rect(x, y, childWidth, lineHeight)); + x += childWidth; + } + y += lineHeight; + } + return finalSize; + } + } + // One group's live line of slots, which grows as the user types into its last one. private sealed class GroupState { @@ -180,7 +246,7 @@ private sealed class GroupState public string GroupId; public string Label; public Action WritingSystemFocused; - public WrapPanel Slots; + public SlotLinePanel Slots; public TextBox TrailingAdd; public int AddedSlots; } @@ -196,9 +262,8 @@ private Control CreateGroup(string label, string automationId, DetailReversalGro WritingSystemFocused = writingSystemFocused, // The group's slots run together on one wrapping line, a bar between each pair, // the add slot last. - Slots = new WrapPanel + Slots = new SlotLinePanel { - Orientation = Orientation.Horizontal, Background = FwAvaloniaDensity.TransparentBrush, FlowDirection = group.RightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight } @@ -298,26 +363,19 @@ private Control CreateRow(GroupState state, DetailReversalRow row, string rowId, ? FwAvaloniaStrings.ReversalAddEntryName(label, group.WsAbbrev) : label + " " + group.WsAbbrev); - // Tracks what the model holds for this row, so an unchanged row never stages. - var committed = row.Text; - Action commit = () => - { - var text = editor.Text ?? string.Empty; - if (_editing != null && text != committed && _editing.TryCommitRow(row.RowKey, text)) - committed = text; - }; + var slotState = new SlotState(row, editor); + _slots.Add(slotState); // The control this method returns; the slot removal below needs it. Control slot = null; if (_editing != null) { - _rowCommits.Add(commit); // Runs before the host's own focus-loss save, which bubbles up from this box. // Moving between slots stages nothing, so the host saves only once focus leaves. EventHandler lost = (s, e) => { if (row.IsAddSlot && !ReferenceEquals(editor, state.TrailingAdd) - && string.IsNullOrEmpty(editor.Text) && string.IsNullOrEmpty(committed)) + && string.IsNullOrEmpty(editor.Text) && string.IsNullOrEmpty(slotState.Committed)) { RemoveSlot(state, slot); } @@ -381,6 +439,13 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) e.Handled = true; return; } + if (e.Key == Key.Escape) + { + // Left unhandled, so the view still cancels; its re-show then finds nothing + // to save. + RevertPendingEdits(); + return; + } if ((e.Key == Key.Home || e.Key == Key.End) && e.KeyModifiers == KeyModifiers.None) { MoveToLineEdge(editor, e.Key == Key.Home); @@ -458,11 +523,59 @@ private List SlotEditors() return editors; } - // Every changed slot commits in order, joining the host's one open edit session. + // A slot's row, its editor, and the text the model holds for it, so an unchanged slot + // never stages. + private sealed class SlotState + { + public SlotState(DetailReversalRow row, TextBox editor) + { + Row = row; + Editor = editor; + Committed = row.Text; + } + + public DetailReversalRow Row { get; } + + public TextBox Editor { get; } + + public string Committed { get; set; } + + public string Text => Editor.Text ?? string.Empty; + } + + /// + /// Stages every slot changed since the last save, as one change on the host's edit + /// session. The field otherwise stages only when focus leaves it, so a host that saves + /// while focus is still inside (on navigation, a refresh, or a tool switch) calls this + /// first. Does nothing when read-only, disposed, or unchanged. + /// + public void CommitPendingEdits() => CommitAll(); + private void CommitAll() { - foreach (var commit in _rowCommits) - commit(); + if (_editing == null || _disposed) + return; + var changed = _slots.Where(slot => slot.Text != slot.Committed).ToList(); + if (changed.Count == 0) + return; + var edits = changed + .Select(slot => new KeyValuePair(slot.Row.RowKey, slot.Text)) + .ToList(); + if (_editing.TryCommitRows(edits)) + { + foreach (var slot in changed) + slot.Committed = slot.Text; + } + } + + // Puts every slot back to the text the model holds, dropping what was typed. + private void RevertPendingEdits() + { + foreach (var slot in _slots) + { + if (slot.Text != slot.Committed) + slot.Editor.Text = slot.Committed; + } } // Focus has already moved when a slot's LostFocus runs, so this tells a move to another diff --git a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs index 25b2648891..49652103d0 100644 --- a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs +++ b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; namespace SIL.FieldWorks.Common.FwAvalonia.Detail { @@ -17,18 +18,24 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail public interface IReversalEntryEditing { /// - /// Stages the text of one row, opening the edit session only when something changes. - /// Text equal to what the row already shows changes nothing; empty text on an entry - /// row unlinks that entry. Other text is split on colons into a chain of entry and - /// subentry forms, and the sense is linked to the deepest entry of that chain, found or - /// created -- an existing entry is never renamed -- in place of the row's old entry. - /// An unlinked entry left with no senses and no subentries is deleted, and so is each - /// parent that deletion leaves with neither. Afterwards the key names the row's new - /// entry, or an add row again after an unlink, so a second commit on the same row edits - /// what the first one produced. + /// Stages the text of several rows as one change, opening the edit session only when + /// something changes. Per row: text equal to what the row already shows changes nothing; + /// empty text on an entry row unlinks that entry; other text is split on colons into a + /// chain of entry and subentry forms, and the sense is linked to the deepest entry of + /// that chain, found or created -- an existing entry is never renamed -- in place of the + /// row's old entry. Every row takes its new entry before any old one is let go, and an + /// old entry another row still shows stays linked. An unlinked entry left with no senses + /// and no subentries is deleted, and so is each parent that deletion leaves with + /// neither. Afterwards each key names its row's new entry, or an add row again after an + /// unlink, so a later commit on the same row edits what this one produced. /// - /// False, without opening the session, for an unknown key, an empty add - /// row, or unchanged text. + /// Row key to typed text, one pair per changed row. + /// False, without opening the session, when no row changes or the sense no + /// longer exists; also false, after logging, when the write fails. + bool TryCommitRows(IReadOnlyList> edits); + + /// Stages one row's text: a of that single + /// row. bool TryCommitRow(string rowKey, string typedText); /// diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs index d074bf1b78..dcb5981bb5 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -36,12 +36,19 @@ private sealed class RecordingReversalContext : IDetailEditContext, IReversalEnt { public readonly List Events = new List(); - public bool TryCommitRow(string rowKey, string typedText) + public int Batches; + + public bool TryCommitRows(IReadOnlyList> edits) { - Events.Add("commit " + rowKey + "=" + typedText); + Batches++; + foreach (var edit in edits) + Events.Add("commit " + edit.Key + "=" + edit.Value); return true; } + public bool TryCommitRow(string rowKey, string typedText) + => TryCommitRows(new[] { new KeyValuePair(rowKey, typedText) }); + private int _issued; public string IssueAddRowKey(string rowKey) => "en-add" + ++_issued; @@ -62,6 +69,8 @@ public bool TryCommitRow(string rowKey, string typedText) public bool TryMoveReferenceItem(DetailField field, string optionKey, bool forward) => false; + public bool TryResetReferenceOrder(DetailField field) => false; + public IReadOnlyList Validate() => Array.Empty(); public void Commit() @@ -135,7 +144,7 @@ public void AGroup_ShowsItsEntries_ThenOneAddRow_UnderOneLabel() public void AGroupsSlots_ShareOneLine_WithABarBetweenEachPair() { var (field, _, window) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); - var group = Find(field, "Reversal.en"); + var group = Find(field, "Reversal.en"); var first = Find(field, "Reversal.en.0"); var second = Find(field, "Reversal.en.1"); var add = Find(field, "Reversal.en.Add"); @@ -180,19 +189,61 @@ public void ALongEntry_StaysOnOneLine_InsideItsSlot() "text wider than the line does not wrap inside its slot"); } + [AvaloniaTest] + public void TheAddSlot_FillsTheRestOfItsLine_AndEntrySlotsDoNot() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, English("dwelling")); + var group = Find(field, "Reversal.en"); + var entry = Find(field, "Reversal.en.0"); + var add = Find(field, "Reversal.en.Add"); + + Assert.That(add.Bounds.Right, Is.EqualTo(group.Bounds.Width).Within(0.5), + "the add slot reaches the end of its line"); + Assert.That(add.Bounds.Width, Is.GreaterThan(add.DesiredSize.Width)); + Assert.That(entry.Bounds.Width, Is.EqualTo(entry.DesiredSize.Width).Within(0.5), + "an entry slot stays as wide as its text"); + } + + [AvaloniaTest] + public void ALoneAddSlot_FillsTheWholeLine() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English()); + var group = Find(field, "Reversal.en"); + var add = Find(field, "Reversal.en.Add"); + + Assert.That(add.Bounds.X, Is.Zero); + Assert.That(add.Bounds.Width, Is.EqualTo(group.Bounds.Width).Within(0.5)); + } + + [AvaloniaTest] + public void AfterGrowth_OnlyTheNewLastSlotStretches() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English()); + var group = Find(field, "Reversal.en"); + var typed = Find(field, "Reversal.en.Add"); + + typed.Focus(); + typed.Text = "home"; + Dispatcher.UIThread.RunJobs(); + var fresh = Find(field, "Reversal.en.Add1"); + + Assert.That(typed.Bounds.Width, Is.EqualTo(typed.DesiredSize.Width).Within(0.5)); + Assert.That(fresh.Bounds.Right, Is.EqualTo(group.Bounds.Width).Within(0.5)); + } + [AvaloniaTest] public void ALoneAddSlot_HasNoBar() { var (field, _, _) = Show(new RecordingReversalContext(), null, English()); - Assert.That(Find(field, "Reversal.en").Children.OfType(), Is.Empty); + Assert.That(Find(field, "Reversal.en").Children.OfType(), Is.Empty); } [AvaloniaTest] public void ClickingAGroupsFreeSpace_StartsTypingInItsAddSlot() { var (field, _, window) = Show(new RecordingReversalContext(), null, English("dwelling")); - var group = Find(field, "Reversal.en"); + var group = Find(field, "Reversal.en"); var point = group.TranslatePoint(new Point(group.Bounds.Width - 2, 2), window); window.MouseDown(point.Value, MouseButton.Left); @@ -210,8 +261,8 @@ public void EachIndex_IsItsOwnGroup() var (field, _, _) = Show(new RecordingReversalContext(), null, English("house"), french); - Assert.That(Find(field, "Reversal.en"), Is.Not.Null); - Assert.That(Find(field, "Reversal.fr"), Is.Not.Null); + Assert.That(Find(field, "Reversal.en"), Is.Not.Null); + Assert.That(Find(field, "Reversal.fr"), Is.Not.Null); Assert.That(Find(field, "Reversal.fr.0").Text, Is.EqualTo("maison")); } @@ -280,6 +331,48 @@ public void MovingBetweenSlots_CommitsNothing_UntilFocusLeavesTheField() Dispatcher.UIThread.RunJobs(); Assert.That(context.Events, Is.EqualTo(new[] { "commit en0=abode", "commit en-add=home" }), "leaving the field commits every changed slot, in order"); + Assert.That(context.Batches, Is.EqualTo(1), "the changed slots are saved as one change"); + } + + [AvaloniaTest] + public void Escape_RestoresEverySlotsSavedText_AndSavesNothing() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + var second = Find(field, "Reversal.en.1"); + TypeAndLeave(first, "house", other); + first.Focus(); + first.Text = "home"; + second.Focus(); + second.Text = "hut"; + + Press(second, Key.Escape); + other.Focus(); + Dispatcher.UIThread.RunJobs(); + + Assert.That(first.Text, Is.EqualTo("house"), "the text saved earlier stays"); + Assert.That(second.Text, Is.EqualTo("abode")); + Assert.That(context.Events, Is.EqualTo(new[] { "commit en0=house" }), + "nothing typed since the last save is saved"); + } + + [AvaloniaTest] + public void CommitPendingEdits_SavesWhileFocusIsStillInside() + { + var context = new RecordingReversalContext(); + var (field, other, _) = Show(context, null, English("dwelling")); + var entry = Find(field, "Reversal.en.0"); + entry.Focus(); + entry.Text = "house"; + + field.CommitPendingEdits(); + Assert.That(entry.IsFocused, Is.True); + other.Focus(); + Dispatcher.UIThread.RunJobs(); + + Assert.That(context.Events, Is.EqualTo(new[] { "commit en0=house" }), + "leaving afterwards saves nothing more"); } [AvaloniaTest] @@ -296,7 +389,7 @@ public void TypingIntoTheAddSlot_OpensAFreshOne_WithoutSaving() var fresh = Find(field, "Reversal.en.Add1"); Assert.That(fresh, Is.Not.Null, "the first keystroke opens another empty slot"); Assert.That(fresh.Text, Is.Empty); - Assert.That(Find(field, "Reversal.en").Children.OfType().Count(), Is.EqualTo(2), + Assert.That(Find(field, "Reversal.en").Children.OfType().Count(), Is.EqualTo(2), "the new slot is joined to the line by a bar"); Assert.That(add.IsFocused, Is.True, "typing continues in the same slot"); Assert.That(context.Events, Is.Empty, "nothing is saved while typing"); @@ -359,7 +452,7 @@ public void AnAddSlotEmptiedAgain_IsRemovedWhenTheUserMovesOn() Dispatcher.UIThread.RunJobs(); Assert.That(Find(field, "Reversal.en.Add"), Is.Null); - Assert.That(Find(field, "Reversal.en").Children.OfType().Count(), Is.EqualTo(1), + Assert.That(Find(field, "Reversal.en").Children.OfType().Count(), Is.EqualTo(1), "the removed slot takes its bar with it"); Assert.That(context.Events, Is.Empty); } @@ -471,7 +564,7 @@ public void HomeAndEnd_StayOnTheirOwnLine_WhenTheGroupWraps() var (field, _, _) = Show(new RecordingReversalContext(), null, English(forms)); var boxes = Enumerable.Range(0, forms.Length) .Select(i => Find(field, "Reversal.en." + i)).ToList(); - var group = Find(field, "Reversal.en"); + var group = Find(field, "Reversal.en"); double Top(TextBox box) => box.TranslatePoint(new Point(0, 0), group).Value.Y; var firstLineTop = Top(boxes[0]); var onSecondLine = boxes.Where(b => Top(b) > firstLineTop).ToList(); diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 6d4be1547b..7e18b42edb 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -3145,11 +3145,10 @@ private void AddPluginRow(ViewNode node, ICmObject obj, int depth, ISlicePlugin var visible = _showAllWsFields != null && _showAllWsFields.Contains(node.StableId) ? null : node.VisibleWritingSystems; - // Built at render time: the link callback and column width exist only in the - // render context. + // Built at render time, since the view's services exist only then. Func factory = render => plugin.BuildControl(new SlicePluginBuildContext(obj, node, _editContextAccessor, _cache, - _writingSystemFocused, render?.LinkRequested, render?.WsAbbrevColumnWidth, visible)); + _writingSystemFocused, render, visible)); AddField(new DetailField(StableId(node, obj), Localize(node.Label) ?? node.Field, node.Field, node.WritingSystem, DetailFieldKind.Custom, node.EditorClassification, node.AutomationId, node.LocalizationKey, node.Routing, null, null, null, diff --git a/Src/xWorks/Avalonia/DetailEditContextBase.cs b/Src/xWorks/Avalonia/DetailEditContextBase.cs index dba15f67a1..ea93515c9a 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 readonly List _pendingEditFlushes = new List(); protected DetailEditContextBase(LcmCache cache, ICmObject root) { @@ -100,6 +101,27 @@ public virtual IReadOnlyList Validate() return errors; } + /// + /// Registers , which stages the edits an editor holds back until + /// focus leaves it. runs every registered flush, so a + /// save made while focus is still inside such an editor includes what it holds. + /// + public void AddPendingEditFlush(Action flush) + { + if (flush != null) + _pendingEditFlushes.Add(flush); + } + + /// + /// Stages whatever the registered editors are holding back, possibly opening the session. + /// A save calls this first, before it checks whether a session is open. + /// + public void FlushPendingEdits() + { + foreach (var flush in _pendingEditFlushes.ToArray()) + flush(); + } + /// public void Commit() { diff --git a/Src/xWorks/Avalonia/DetailEditContextHolder.cs b/Src/xWorks/Avalonia/DetailEditContextHolder.cs index a7db67df09..18b66336a4 100644 --- a/Src/xWorks/Avalonia/DetailEditContextHolder.cs +++ b/Src/xWorks/Avalonia/DetailEditContextHolder.cs @@ -86,6 +86,7 @@ public void Clear() public IReadOnlyList Settle() { var current = Current; + FlushPendingEdits(current); if (current == null || !current.IsOpen) return System.Array.Empty(); try @@ -117,6 +118,20 @@ public IReadOnlyList Settle() } } + // An editor that stages only on focus loss still holds its edits when a save runs with + // focus inside it; staging them first lets the settle below commit them. + private static void FlushPendingEdits(IDetailEditContext current) + { + try + { + (current as DetailEditContextBase)?.FlushPendingEdits(); + } + catch (System.Exception e) + { + SIL.Reporting.Logger.WriteError(e); + } + } + /// /// Intercepts global Undo/Redo for the given action handler while a session is open (see /// class remarks). Detaches any previously attached handler first. diff --git a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs index 7db9434531..930cb24ea3 100644 --- a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs +++ b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs @@ -11,6 +11,8 @@ using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Core.Text; using SIL.LCModel.DomainServices; using SIL.Reporting; @@ -58,7 +60,7 @@ public Control BuildControl(SlicePluginBuildContext context) var groups = editing.CreateGroups(context.VisibleWritingSystems); Action navigate = null; - var linkRequested = context.LinkRequested; + var linkRequested = context.Render?.LinkRequested; if (linkRequested != null) { var field = new DetailField( @@ -84,9 +86,13 @@ public Control BuildControl(SlicePluginBuildContext context) }; } - return new FwReversalEntriesField(label, automationId, groups, + var control = new FwReversalEntriesField(label, automationId, groups, host == null ? null : editing, context.WritingSystemFocused, navigate, - context.WsAbbrevColumnWidth); + context.Render?.WsAbbrevColumnWidth); + // The field stages only when focus leaves it, so the host's save must ask for its + // edits when it runs with focus still inside. + (host as DetailEditContextBase)?.AddPendingEditFlush(control.CommitPendingEdits); + return control; } catch (Exception e) { @@ -213,39 +219,92 @@ private IReadOnlyList OtherWsForms(IReversalIndexEntr return result; } + // One row's staged change: its binding, its new chain of forms, and its index's ws. + private sealed class RowChange + { + public RowChange(RowBinding binding, IList forms, int ws) + { + Binding = binding; + Forms = forms; + Ws = ws; + } + + public RowBinding Binding { get; } + + public IList Forms { get; } + + public int Ws { get; } + } + /// public bool TryCommitRow(string rowKey, string typedText) + => TryCommitRows(new[] { new KeyValuePair(rowKey, typedText) }); + + /// + public bool TryCommitRows(IReadOnlyList> edits) { - RowBinding binding; - if (string.IsNullOrEmpty(rowKey) || !_rows.TryGetValue(rowKey, out binding)) - return false; - var ws = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(binding.Index.WritingSystem); - if (ws <= 0) + if (edits == null || !_sense.IsValidObject) return false; - var forms = SplitForms(typedText); - var current = binding.Entry; - if (current != null && !current.IsValidObject) - current = binding.Entry = null; - if (current == null && forms.Count == 0) - return false; - if (current != null && ChainMatches(current, forms, ws)) + var changes = new List(); + foreach (var edit in edits) + { + RowBinding binding; + if (string.IsNullOrEmpty(edit.Key) || !_rows.TryGetValue(edit.Key, out binding)) + continue; + var ws = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(binding.Index.WritingSystem); + if (ws <= 0) + continue; + if (binding.Entry != null && !binding.Entry.IsValidObject) + binding.Entry = null; + var forms = SplitForms(edit.Value); + if (binding.Entry == null ? forms.Count == 0 : ChainMatches(binding.Entry, forms, ws)) + continue; + changes.Add(new RowChange(binding, forms, ws)); + } + if (changes.Count == 0) return false; - return StageOnHost(() => + var previous = changes.Select(change => change.Binding.Entry).ToList(); + try { - IReversalIndexEntry target = null; - if (forms.Count > 0) + return StageOnHost(() => { - target = FindOrCreateEntry(binding.Index, forms, ws); - if (!target.SensesRS.Contains(_sense)) - target.SensesRS.Add(_sense); - } - if (current != null && current != target) - Unlink(current); - binding.Entry = target; - return true; - }); + // Every row takes its new entry before any entry is let go, so an entry + // one row gives up and another takes over is never deleted in between. + var released = new List(); + foreach (var change in changes) + { + IReversalIndexEntry target = null; + if (change.Forms.Count > 0) + { + target = FindOrCreateEntry(change.Binding.Index, change.Forms, change.Ws); + if (!target.SensesRS.Contains(_sense)) + target.SensesRS.Add(_sense); + } + if (change.Binding.Entry != null && change.Binding.Entry != target) + released.Add(change.Binding.Entry); + change.Binding.Entry = target; + } + + var stillWanted = new HashSet( + _rows.Values.Select(binding => binding.Entry).Where(entry => entry != null)); + foreach (var entry in released.Distinct()) + { + if (entry.IsValidObject && !stillWanted.Contains(entry)) + Unlink(entry); + } + return true; + }); + } + catch (Exception e) + { + // The write rolled back or never ran, so the rows keep the entries they showed. + for (var i = 0; i < changes.Count; i++) + changes[i].Binding.Entry = previous[i]; + Logger.WriteError(e); + return false; + } } /// @@ -261,7 +320,7 @@ public string IssueAddRowKey(string rowKey) public Guid? TryResolveMainEntryGuid(string rowKey) { RowBinding binding; - if (string.IsNullOrEmpty(rowKey) || !_rows.TryGetValue(rowKey, out binding)) + if (!_sense.IsValidObject || string.IsNullOrEmpty(rowKey) || !_rows.TryGetValue(rowKey, out binding)) return null; var entry = binding.Entry; if (entry == null || !entry.IsValidObject) @@ -271,7 +330,7 @@ public string IssueAddRowKey(string rowKey) /// /// The forms of an entry chain, top level first: the text split on colons, each part - /// trimmed, and empty parts dropped (LT-4665). + /// trimmed and decomposed (NFD, as stored forms are), and empty parts dropped (LT-4665). /// internal static IList SplitForms(string text) { @@ -279,16 +338,25 @@ internal static IList SplitForms(string text) .Split(new[] { ':' }, StringSplitOptions.RemoveEmptyEntries) .Select(part => part.Trim()) .Where(part => part.Length > 0) + .Select(Decomposed) .ToList(); } + // Typed text arrives precomposed while stored forms are decomposed, so both sides of a + // comparison go through NFD first. + private static string Decomposed(string text) + => CustomIcu.GetIcuNormalizer(FwNormalizationMode.knmNFD).Normalize(text ?? string.Empty); + + private static string StoredForm(IReversalIndexEntry entry, int ws) + => Decomposed(entry.ReversalForm.get_String(ws).Text); + // True when the entry and its ancestors, top level first, are exactly the given forms. private static bool ChainMatches(IReversalIndexEntry entry, IList forms, int ws) { var level = entry; for (var i = forms.Count - 1; i >= 0; i--) { - if (level == null || level.ReversalForm.get_String(ws).Text != forms[i]) + if (level == null || StoredForm(level, ws) != forms[i]) return false; level = level.OwningEntry; } @@ -327,7 +395,7 @@ private static void FindDeepest(IEnumerable candidates, ILi { foreach (var candidate in candidates) { - if (candidate.ReversalForm.get_String(ws).Text != forms[level]) + if (StoredForm(candidate, ws) != forms[level]) continue; if (level + 1 > depth) { diff --git a/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs b/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs index d2dcf2c002..bb96de216a 100644 --- a/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs +++ b/Src/xWorks/Avalonia/Plugins/SlicePlugins.cs @@ -41,9 +41,9 @@ public interface ISlicePlugin /// Everything the composer hands a plugin factory, bundled into one contract: /// the row's object and typed node, the detail view's edit context (resolved lazily through the /// composer's deferred accessor -- the context object is created during compose, BEFORE the - /// edit context exists; plugin factories run at render time, after), the cache, and the - /// host's writing-system focus and link callbacks, the view's abbreviation-column width, - /// and the row's writing-system restriction. + /// edit context exists; plugin factories run at render time, after), the cache, the + /// host's writing-system focus callback, the view's render-time services, and the row's + /// writing-system restriction. /// public sealed class SlicePluginBuildContext { @@ -52,8 +52,7 @@ public sealed class SlicePluginBuildContext public SlicePluginBuildContext(ICmObject target, ViewNode node, Func editContextAccessor, LcmCache cache, Action writingSystemFocused = null, - Action linkRequested = null, - double? wsAbbrevColumnWidth = null, + SliceFactoryContext render = null, IReadOnlyList visibleWritingSystems = null) { Target = target; @@ -61,8 +60,7 @@ public SlicePluginBuildContext(ICmObject target, ViewNode node, _editContextAccessor = editContextAccessor; Cache = cache; WritingSystemFocused = writingSystemFocused; - LinkRequested = linkRequested; - WsAbbrevColumnWidth = wsAbbrevColumnWidth; + Render = render; VisibleWritingSystems = visibleWritingSystems; } @@ -84,16 +82,11 @@ public SlicePluginBuildContext(ICmObject target, ViewNode node, public Action WritingSystemFocused { get; } /// - /// The host's jump callback, the same one chooser links use: the host settles the - /// open edit session, then follows the link. Null when the host supplies none. + /// The services the view hands every row it renders, such as the jump callback chooser + /// links use and the width of the writing-system abbreviation column. Null when the + /// control is built outside a rendering view. /// - public Action LinkRequested { get; } - - /// - /// The width the view gives its writing-system abbreviation column, so a plugin's own - /// abbreviations line up with the other rows. Null when the host supplies none. - /// - public double? WsAbbrevColumnWidth { get; } + public SliceFactoryContext Render { get; } /// /// The writing-system ids the row is restricted to, in order. Null or empty means no diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs index 0517e02f25..3eab6bc390 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Avalonia.LogicalTree; using NUnit.Framework; using SIL.FieldWorks.Common.FwAvalonia; using SIL.FieldWorks.Common.FwAvalonia.Detail; @@ -103,7 +104,9 @@ private CoreWritingSystemDefinition AddAnalysisWs(string tag, bool rightToLeft = { WritingSystemServices.FindOrCreateWritingSystem(Cache, null, tag, false, false, out ws); ws.RightToLeftScript = rightToLeft; - Cache.LangProject.AddToCurrentAnalysisWritingSystems(ws); + // The project outlives each test, so a writing system may already be current. + if (!Cache.LangProject.CurrentAnalysisWritingSystems.Contains(ws)) + Cache.LangProject.AddToCurrentAnalysisWritingSystems(ws); }); return ws; } @@ -525,6 +528,117 @@ public void IssuedAddKeys_AddSeparateEntries_InOneUndoStep() Assert.That(editing.IssueAddRowKey("no-such-key"), Is.Null); } + [Test] + public void SwappingTwoRows_KeepsBothEntriesLinked() + { + var dwelling = AddEntry(m_enIndex, "dwelling", m_sense); + var abode = AddEntry(m_enIndex, "abode", m_sense); + var (editing, host) = NewContext(); + var rows = Group(editing.CreateGroups(null), EnTag).Rows.Where(r => !r.IsAddSlot).ToList(); + var first = rows.Single(r => r.Text == "dwelling"); + var second = rows.Single(r => r.Text == "abode"); + + Assert.That(editing.TryCommitRows(new[] + { + new KeyValuePair(first.RowKey, "abode"), + new KeyValuePair(second.RowKey, "dwelling") + }), Is.True); + host.Commit(); + + Assert.That(dwelling.IsValidObject && abode.IsValidObject, Is.True, + "an entry one row lets go and another takes is never deleted"); + Assert.That(m_sense.ReferringReversalIndexEntries, Is.EquivalentTo(new[] { dwelling, abode })); + } + + [Test] + public void ShiftingTextUpARow_DeletesOnlyTheEntryNoRowKeeps() + { + var one = AddEntry(m_enIndex, "one", m_sense); + var two = AddEntry(m_enIndex, "two", m_sense); + var (editing, host) = NewContext(); + var rows = Group(editing.CreateGroups(null), EnTag).Rows.Where(r => !r.IsAddSlot).ToList(); + + editing.TryCommitRows(new[] + { + new KeyValuePair(rows.Single(r => r.Text == "one").RowKey, "two"), + new KeyValuePair(rows.Single(r => r.Text == "two").RowKey, string.Empty) + }); + host.Commit(); + + Assert.That(one.IsValidObject, Is.False, "no row shows the first entry any more"); + Assert.That(m_sense.ReferringReversalIndexEntries, Is.EqualTo(new[] { two })); + } + + [Test] + public void AnUnchangedRow_KeepsItsEntrysOtherWritingSystemForms() + { + var enGb = AddAnalysisWs("en-GB"); + var entry = AddEntry(m_enIndex, "house", m_sense); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => entry.ReversalForm.set_String(enGb.Handle, "houze")); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.Single(r => !r.IsAddSlot); + + Assert.That(editing.TryCommitRow(row.RowKey, "house"), Is.False, + "the alternative is not part of the row's text, so the row is unchanged"); + Assert.That(host.IsOpen, Is.False); + Assert.That(entry.ReversalForm.get_String(enGb.Handle).Text, Is.EqualTo("houze")); + } + + [Test] + public void PrecomposedTyping_MatchesADecomposedStoredForm() + { + const string decomposed = "café"; + const string precomposed = "café"; + var entry = AddEntry(m_enIndex, decomposed, m_sense); + var (editing, host) = NewContext(); + var group = Group(editing.CreateGroups(null), EnTag); + + Assert.That(editing.TryCommitRow(group.Rows.Single(r => !r.IsAddSlot).RowKey, precomposed), + Is.False, "the same text in another normalization is no change"); + var other = AddOtherSense(); + var (otherEditing, otherHost) = NewContext(other); + otherEditing.TryCommitRow(AddRow(Group(otherEditing.CreateGroups(null), EnTag)).RowKey, precomposed); + otherHost.Commit(); + + Assert.That(m_enIndex.EntriesOC, Is.EqualTo(new[] { entry }), "the existing entry is reused"); + Assert.That(entry.SensesRS, Does.Contain(other)); + } + + [Test] + public void ACommitForADeletedSense_ChangesNothing() + { + var entry = AddEntry(m_enIndex, "dwelling", m_sense); + var (editing, host) = NewContext(); + var row = Group(editing.CreateGroups(null), EnTag).Rows.Single(r => !r.IsAddSlot); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => m_entry.SensesOS.Remove(m_sense)); + + Assert.That(editing.TryCommitRow(row.RowKey, "abode"), Is.False); + Assert.That(editing.TryResolveMainEntryGuid(row.RowKey), Is.Null); + Assert.That(host.IsOpen, Is.False); + Assert.That(entry.ReversalForm.get_String(EnWs).Text, Is.EqualTo("dwelling")); + } + + [Test] + public void Settling_SavesWhatTheFieldHolds_WhileFocusIsStillInIt() + { + AddEntry(m_enIndex, "dwelling", m_sense); + var host = DetailComposer.Compose(m_entry, Cache).EditContext; + var holder = new DetailEditContextHolder(); + holder.Replace(host); + var field = (FwReversalEntriesField)new ReversalIndexEntryPlugin().BuildControl( + new SlicePluginBuildContext(m_sense, null, () => host, Cache)); + var slot = field.GetLogicalDescendants().OfType() + .Single(box => box.Text == "dwelling"); + slot.Text = "abode"; + + holder.Settle(); + + Assert.That(host.IsOpen, Is.False, "the settle committed the edit it flushed"); + Assert.That(m_sense.ReferringReversalIndexEntries.Single().ReversalForm.get_String(EnWs).Text, + Is.EqualTo("abode")); + } + [Test] public void AnAddedEntry_PersistsIntoTheNextCompose() { diff --git a/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs b/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs index 4c40b02b1c..f786600c07 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Plugins/LexemeEditorInventoryTests.cs @@ -229,8 +229,7 @@ private sealed class FakeMessagesPlugin : ISlicePlugin public SIL.FieldWorks.Common.FwAvalonia.ViewDefinition.ViewNode LastNode; public IDetailEditContext LastEditContext; public LcmCache LastCache; - public Action LastLinkRequested; - public double? LastWsAbbrevColumnWidth; + public SliceFactoryContext LastRender; public string LegacyClassName => MessageSliceClassName; @@ -241,8 +240,7 @@ public Avalonia.Controls.Control BuildControl(SlicePluginBuildContext context) LastNode = context.Node; LastEditContext = context.EditContext; LastCache = context.Cache; - LastLinkRequested = context.LinkRequested; - LastWsAbbrevColumnWidth = context.WsAbbrevColumnWidth; + LastRender = context.Render; return null; // never rendered in this fixture; the view's null guard covers this } } @@ -307,7 +305,7 @@ public void PluginRowFactory_ClosesOverObjectNodeCacheAndTheComposedEditContext( } [Test] - public void PluginRowFactory_PassesTheRenderContextsLinkCallbackAndColumnWidth() + public void PluginRowFactory_PassesTheRenderContextThrough() { var registry = new SlicePluginRegistry(); var plugin = new FakeMessagesPlugin(); @@ -315,13 +313,13 @@ public void PluginRowFactory_PassesTheRenderContextsLinkCallbackAndColumnWidth() var composed = DetailComposer.Compose(m_entry, Cache, plugins: registry); var row = composed.Model.Fields.Single(f => f.Kind == DetailFieldKind.Custom); Action linkRequested = request => { }; + var render = new SliceFactoryContext(linkRequested: linkRequested, + wsAbbrevColumnWidth: 37); - row.ControlFactory(new SliceFactoryContext(linkRequested: linkRequested, - wsAbbrevColumnWidth: 37)); + row.ControlFactory(render); - Assert.That(plugin.LastLinkRequested, Is.SameAs(linkRequested), - "the plugin reaches the host's jump through the render context"); - Assert.That(plugin.LastWsAbbrevColumnWidth, Is.EqualTo(37)); + Assert.That(plugin.LastRender, Is.SameAs(render), + "the plugin reaches the host's jump and column width through the render context"); } } } From b2d6b928f7abd8261293ebd39602b63ecbbb1cda Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:07:30 -0500 Subject: [PATCH 05/12] LT-22673: Tab through every reversal entry slot Tab and Shift+Tab now visit each slot in reading order, including add slots opened while typing, and leave the field only from its last or first slot, where the detail view's tab order moves on to the next or previous slice. A slot opened after the view built the row takes the row's tab index from the slot before it, so Tab can leave from it. Co-Authored-By: Claude Opus 5.5 --- .../Detail/FwReversalEntriesField.cs | 25 +++- .../Detail/FwReversalEntriesFieldTests.cs | 134 ++++++++++++++++++ 2 files changed, 155 insertions(+), 4 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 3bba39f24d..7034312f75 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -319,8 +319,12 @@ private void Grow(GroupState state, string addRowKey) if (key == null) return; state.AddedSlots++; + // The view gives a row's controls its tab index when it builds the row, so a slot + // opened later takes the index from the slot before it, or Tab could not leave it. + var tabIndex = KeyboardNavigation.GetTabIndex(state.TrailingAdd); AppendSlot(state, new DetailReversalRow(key, string.Empty, true), state.GroupId + ".Add" + state.AddedSlots); + KeyboardNavigation.SetTabIndex(state.TrailingAdd, tabIndex); } // Drops an add slot the user emptied again before it was saved, with the bar that joined @@ -426,10 +430,11 @@ private Control CreateRow(GroupState state, DetailReversalRow row, string rowId, return slot; } - // Keys that treat the field's slots as one text. Enter does nothing. An arrow, alone or - // with Ctrl, at a slot's edge moves into the neighboring slot, across groups (in a - // right-to-left group the start is on the right). Plain Home and End go to the edges of - // the current visual line. + // Keys that treat the field's slots as one text. Enter does nothing. Tab and Shift+Tab + // visit every slot in reading order, add slots included, and leave the field only from + // its last or first slot. An arrow, alone or with Ctrl, at a slot's edge moves into the + // neighboring slot, across groups (in a right-to-left group the start is on the right). + // Plain Home and End go to the edges of the current visual line. private void WireSlotNavigation(TextBox editor, bool rightToLeft) { EventHandler keyDown = (s, e) => @@ -439,6 +444,18 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) e.Handled = true; return; } + if (e.Key == Key.Tab && (e.KeyModifiers & ~KeyModifiers.Shift) == KeyModifiers.None) + { + var editors = SlotEditors(); + var next = editors.IndexOf(editor) + + (e.KeyModifiers == KeyModifiers.Shift ? -1 : 1); + // Past either end the view's own tab order moves on to the neighboring row. + if (next < 0 || next >= editors.Count) + return; + editors[next].Focus(NavigationMethod.Tab, e.KeyModifiers); + e.Handled = true; + return; + } if (e.Key == Key.Escape) { // Left unhandled, so the view still cancels; its re-show then finds nothing diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs index dcb5981bb5..ecd9f0c2ca 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -18,6 +18,7 @@ using NUnit.Framework; using SIL.FieldWorks.Common.FwAvalonia; using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; namespace FwAvaloniaTests.Detail { @@ -511,6 +512,139 @@ public void RightAtASlotsEnd_MovesToTheStartOfTheNextSlot() Assert.That(second.CaretIndex, Is.Zero); } + private static void Tab(Window window, bool shift = false) + { + window.KeyPressQwerty(PhysicalKey.Tab, shift ? RawInputModifiers.Shift : RawInputModifiers.None); + Dispatcher.UIThread.RunJobs(); + } + + private static string FocusedId(Window window) + => window.FocusManager?.GetFocusedElement() is Control focused + ? AutomationProperties.GetAutomationId(focused) + : null; + + [AvaloniaTest] + public void Tab_VisitsEverySlotInOrder_IncludingAddSlotsOpenedWhileTyping() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, + English("dwelling"), French("maison")); + var add = Find(field, "Reversal.en.Add"); + add.Focus(); + add.Text = "home"; + Dispatcher.UIThread.RunJobs(); + Find(field, "Reversal.en.0").Focus(); + + var visited = new List(); + for (var i = 0; i < 4; i++) + { + Tab(window); + visited.Add(FocusedId(window)); + } + + Assert.That(visited, Is.EqualTo(new[] + { + "Reversal.en.Add", "Reversal.en.Add1", "Reversal.fr.0", "Reversal.fr.Add" + })); + } + + [AvaloniaTest] + public void ShiftTab_VisitsTheSlotsInReverse() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, + English("dwelling"), French("maison")); + Find(field, "Reversal.fr.Add").Focus(); + + var visited = new List(); + for (var i = 0; i < 3; i++) + { + Tab(window, shift: true); + visited.Add(FocusedId(window)); + } + + Assert.That(visited, Is.EqualTo(new[] { "Reversal.fr.0", "Reversal.en.Add", "Reversal.en.0" })); + } + + [AvaloniaTest] + public void Tab_MovesMidText_WithoutEditingTheSlot() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + PlaceCaret(first, 3); + + Tab(window); + + Assert.That(FocusedId(window), Is.EqualTo("Reversal.en.1")); + Assert.That(first.Text, Is.EqualTo("dwelling"), "Tab is navigation, never typed text"); + } + + [AvaloniaTest] + public void TabFromTheLastSlot_LeavesTheField_AndSavesIt() + { + var context = new RecordingReversalContext(); + var (field, other, window) = Show(context, null, English("dwelling")); + var entry = Find(field, "Reversal.en.0"); + entry.Focus(); + entry.Text = "house"; + Tab(window); + Assert.That(context.Events, Is.Empty, "moving to the add slot stays inside the field"); + + Tab(window); + + Assert.That(other.IsFocused, Is.True, "Tab from the final add slot goes to the next control"); + Assert.That(context.Events, Is.EqualTo(new[] { "commit en0=house" })); + } + + [AvaloniaTest] + public void ShiftTabFromTheFirstSlot_LeavesTheField() + { + var field = new FwReversalEntriesField("Reversal Entries", FieldId, + new[] { English("dwelling") }, new RecordingReversalContext()); + var before = new TextBox(); + var panel = new StackPanel(); + panel.Children.Add(before); + panel.Children.Add(field); + var window = new Window { Content = panel, Width = 420, Height = 300 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + Find(field, "Reversal.en.0").Focus(); + + Tab(window, shift: true); + + Assert.That(before.IsFocused, Is.True); + } + + // The detail view orders Tab by row, so a slot opened after the view built the row must + // still sort with its row. + [AvaloniaTest] + public void InTheDetailView_TabFromAnAddSlotOpenedWhileTyping_MovesToTheNextRow() + { + var context = new RecordingReversalContext(); + var reversal = new DetailField("Reversal", "Reversal Entries", "ReferringReversalIndexEntries", + null, DetailFieldKind.Custom, EditorClassification.Known, FieldId, null, HostRouting.Inherit, + null, null, null, objectHvo: 1, + controlFactory: render => new FwReversalEntriesField("Reversal Entries", FieldId, + new[] { English("dwelling") }, context)); + var next = new DetailField("Next", "Next", "Next", null, DetailFieldKind.Text, + EditorClassification.Known, "Next", null, HostRouting.Inherit, + new List { new DetailWsValue("vern", "value") }, null, null, objectHvo: 1); + var model = new DetailModel("LexSense", "Normal", new List { reversal, next }, + new List()); + var view = new DataTree(model, editContext: context); + var window = new Window { Content = view, Width = 480, Height = 300 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + var add = Find(view, "Reversal.en.Add"); + add.Focus(); + add.Text = "home"; + Dispatcher.UIThread.RunJobs(); + + Tab(window); + Assert.That(FocusedId(window), Is.EqualTo("Reversal.en.Add1")); + Tab(window); + + Assert.That(FocusedId(window), Is.EqualTo("Next.vern")); + } + [AvaloniaTest] public void Enter_DoesNothing() { From db4e96f798f474e43397914a2c747f161283a2c4 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:15:44 -0500 Subject: [PATCH 06/12] Connect up and down arrows and Ctrl+Home and Ctrl+End --- .../Detail/FwReversalEntriesField.cs | 150 +++++++++++++++++- .../Detail/FwReversalEntriesFieldTests.cs | 131 +++++++++++++++ 2 files changed, 275 insertions(+), 6 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 7034312f75..787dc4b20c 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -9,6 +9,7 @@ using Avalonia.Automation; using Avalonia.Controls; using Avalonia.Controls.Documents; +using Avalonia.Controls.Presenters; using Avalonia.Input; using Avalonia.Interactivity; using Avalonia.Layout; @@ -129,6 +130,9 @@ public sealed class FwReversalEntriesField : StackPanel, IDisposable private readonly IReversalEntryEditing _editing; private readonly Action _navigationRequested; private bool _disposed; + // The horizontal position a run of Up/Down navigates by, so ragged lines do not walk + // the caret sideways; null until one starts, and again as soon as anything else moves it. + private double? _lineNavigationX; /// Builds the editor. /// The field label, used in accessible names. @@ -158,6 +162,13 @@ public FwReversalEntriesField(string label, string automationId, var abbrevWidth = wsAbbrevColumnWidth ?? FwAvaloniaDensity.WsAbbrevWidth; foreach (var group in groups ?? Array.Empty()) Children.Add(CreateGroup(name, automationId, group, writingSystemFocused, abbrevWidth)); + + // A click puts the caret somewhere of its own, so the next Up or Down starts from + // there rather than from wherever the last one was heading. + EventHandler pressed = (s, e) => _lineNavigationX = null; + AddHandler(InputElement.PointerPressedEvent, pressed, RoutingStrategies.Tunnel, + handledEventsToo: true); + _teardown.Add(() => RemoveHandler(InputElement.PointerPressedEvent, pressed)); } // A text-sized editor clips its own caret at the end, and fits none at all when empty, so @@ -432,13 +443,18 @@ private Control CreateRow(GroupState state, DetailReversalRow row, string rowId, // Keys that treat the field's slots as one text. Enter does nothing. Tab and Shift+Tab // visit every slot in reading order, add slots included, and leave the field only from - // its last or first slot. An arrow, alone or with Ctrl, at a slot's edge moves into the - // neighboring slot, across groups (in a right-to-left group the start is on the right). - // Plain Home and End go to the edges of the current visual line. + // its last or first slot. Left and Right, alone or with Ctrl, at a slot's edge move into + // the neighboring slot, across groups (in a right-to-left group the start is on the + // right). Up and Down move between visual lines at the same horizontal position. Home + // and End go to the edges of the current visual line, and with Ctrl to the ends of the + // whole field. private void WireSlotNavigation(TextBox editor, bool rightToLeft) { EventHandler keyDown = (s, e) => { + // Only an unbroken run of Up/Down keeps the position it navigates by. + if (e.Key != Key.Up && e.Key != Key.Down) + _lineNavigationX = null; if (e.Key == Key.Enter) { e.Handled = true; @@ -463,12 +479,24 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) RevertPendingEdits(); return; } - if ((e.Key == Key.Home || e.Key == Key.End) && e.KeyModifiers == KeyModifiers.None) + if ((e.Key == Key.Home || e.Key == Key.End) + && (e.KeyModifiers == KeyModifiers.None || e.KeyModifiers == KeyModifiers.Control)) { - MoveToLineEdge(editor, e.Key == Key.Home); + var toStart = e.Key == Key.Home; + if (e.KeyModifiers == KeyModifiers.Control) + MoveToFieldEdge(toStart); + else + MoveToLineEdge(editor, toStart); e.Handled = true; return; } + if ((e.Key == Key.Up || e.Key == Key.Down) && e.KeyModifiers == KeyModifiers.None) + { + // Past the first or last line the key is left alone, for the view to answer. + if (MoveToNeighboringLine(editor, e.Key == Key.Up)) + e.Handled = true; + return; + } if ((e.KeyModifiers & ~KeyModifiers.Control) != KeyModifiers.None || (e.Key != Key.Left && e.Key != Key.Right)) { @@ -492,6 +520,112 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) _teardown.Add(() => editor.RemoveHandler(InputElement.KeyDownEvent, keyDown)); } + // Ctrl+Home and Ctrl+End treat the whole field as one text: its very first slot and its + // very last, whichever group they are in. + private void MoveToFieldEdge(bool toStart) + { + var slots = SlotEditors(); + if (slots.Count > 0) + PlaceCaret(toStart ? slots[0] : slots[slots.Count - 1], !toStart); + } + + // Up and Down move to the neighboring visual line, keeping the caret's horizontal + // position: the slot under it takes the caret. False at the field's first or last line. + private bool MoveToNeighboringLine(TextBox editor, bool up) + { + var lines = SlotLines(); + var index = lines.FindIndex(line => + line.Any(slot => ReferenceEquals(SlotEditor(slot), editor))); + var target = index + (up ? -1 : 1); + if (index < 0 || target < 0 || target >= lines.Count) + return false; + _lineNavigationX = _lineNavigationX ?? CaretX(editor); + var slot = NearestSlot(lines[target], _lineNavigationX); + if (slot == null) + return false; + PlaceCaretAtX(SlotEditor(slot), _lineNavigationX); + return true; + } + + // The field's slots grouped into visual lines, top line first and each line in + // left-to-right order: a group's panel puts one line's slots at the same top, and the + // groups stack in order. + private List> SlotLines() + { + var lines = new List>(); + foreach (var state in _groups) + { + var slots = state.Slots.Children.Where(child => SlotEditor(child) != null); + foreach (var line in slots.GroupBy(child => child.Bounds.Y).OrderBy(line => line.Key)) + lines.Add(line.OrderBy(child => child.Bounds.X).ToList()); + } + return lines; + } + + // The line's slot at horizontal position x, or the nearest one when x falls on a bar + // between slots or past the line's end. The first slot when there is no position to + // match, null for a line without slots. + private Control NearestSlot(IReadOnlyList line, double? x) + { + if (!x.HasValue) + return line.FirstOrDefault(); + Control nearest = null; + var shortest = double.MaxValue; + foreach (var slot in line) + { + var left = slot.TranslatePoint(new Point(0, 0), this)?.X; + if (!left.HasValue) + continue; + var right = left.Value + slot.Bounds.Width; + var distance = x.Value < left.Value + ? left.Value - x.Value + : x.Value > right ? x.Value - right : 0; + if (distance < shortest) + { + nearest = slot; + shortest = distance; + } + } + return nearest ?? line.FirstOrDefault(); + } + + // The caret's horizontal position in the field's own coordinates, or null while the + // slot has no laid-out text to measure it against. + private double? CaretX(TextBox editor) + { + var presenter = SlotPresenter(editor); + var layout = presenter?.TextLayout; + if (layout == null) + return null; + var length = (editor.Text ?? string.Empty).Length; + var caret = layout.HitTestTextPosition(Math.Min(Math.Max(editor.CaretIndex, 0), length)); + return presenter.TranslatePoint(new Point(caret.X, 0), this)?.X; + } + + // Puts the caret on the character nearest horizontal position x, or at the slot's end + // when there is no position to match or no laid-out text to match it against. + private void PlaceCaretAtX(TextBox target, double? x) + { + target.Focus(); + var end = (target.Text ?? string.Empty).Length; + var caret = end; + var presenter = SlotPresenter(target); + var layout = presenter?.TextLayout; + if (x.HasValue && layout != null) + { + var local = this.TranslatePoint(new Point(x.Value, 0), presenter); + if (local.HasValue) + { + var hit = layout.HitTestPoint(new Point(local.Value.X, 0)); + caret = Math.Min(hit.TextPosition + (hit.IsTrailing ? 1 : 0), end); + } + } + SetCaret(target, caret); + } + + private static TextPresenter SlotPresenter(TextBox editor) + => editor?.GetVisualDescendants().OfType().FirstOrDefault(); + // Home goes to the start of the first slot on the editor's visual line, End to the end of // the last; the group's wrap panel puts every slot of one line at the same top. private void MoveToLineEdge(TextBox editor, bool toStart) @@ -514,7 +648,11 @@ private void MoveToLineEdge(TextBox editor, bool toStart) private static void PlaceCaret(TextBox target, bool atEnd) { target.Focus(); - var caret = atEnd ? (target.Text ?? string.Empty).Length : 0; + SetCaret(target, atEnd ? (target.Text ?? string.Empty).Length : 0); + } + + private static void SetCaret(TextBox target, int caret) + { target.CaretIndex = caret; target.SelectionStart = caret; target.SelectionEnd = caret; diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs index ecd9f0c2ca..a082390412 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -729,6 +729,137 @@ public void ModifiedHomeAndEnd_StayInTheSlot() Assert.That(middle.IsFocused, Is.True); } + // Every slot of the field, in build order; a row's read-only suffix is a TextBlock, so + // the text boxes are exactly the slots. + private static List Slots(Control field) + => field.GetVisualDescendants().OfType().ToList(); + + private static TextBox FocusedSlot(Window window) + => window.FocusManager?.GetFocusedElement() as TextBox; + + private static void Click(Window window, Control control) + { + var point = control.TranslatePoint(new Point(2, 2), window); + Assert.That(point, Is.Not.Null, "the click target must be attached and laid out"); + window.MouseDown(point.Value, MouseButton.Left); + window.MouseUp(point.Value, MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + } + + // A group wide enough to wrap, so the field has more than one visual line. + private static (FwReversalEntriesField Field, Window Window, List FirstLine, + List SecondLine) ShowWrappingGroup() + { + var forms = Enumerable.Range(0, 12).Select(i => "dwellingplace" + i).ToArray(); + var (field, _, window) = Show(new RecordingReversalContext(), null, English(forms)); + var group = Find(field, "Reversal.en"); + var slots = Slots(field); + double Top(TextBox box) => box.TranslatePoint(new Point(0, 0), group).Value.Y; + var firstTop = Top(slots[0]); + var below = slots.Where(box => Top(box) > firstTop).ToList(); + Assert.That(below, Is.Not.Empty, "precondition: the group wraps"); + var secondTop = Top(below[0]); + return (field, window, + slots.Where(box => Top(box).Equals(firstTop)).ToList(), + slots.Where(box => Top(box).Equals(secondTop)).ToList()); + } + + [AvaloniaTest] + public void UpAndDown_MoveBetweenTheLinesOfAWrappingGroup() + { + var (_, window, firstLine, secondLine) = ShowWrappingGroup(); + var start = secondLine[0]; + + PlaceCaret(start, 1); + Press(start, Key.Up); + Assert.That(firstLine, Does.Contain(FocusedSlot(window)), "Up moves to the line above"); + + Press(FocusedSlot(window), Key.Down); + Assert.That(secondLine, Does.Contain(FocusedSlot(window)), "Down moves back down"); + } + + [AvaloniaTest] + public void Down_MovesIntoTheNextGroup_AndUpComesBack() + { + var (field, _, window) = Show(new RecordingReversalContext(), null, + English("dwelling"), French("maison")); + var start = Find(field, "Reversal.en.0"); + + PlaceCaret(start, 2); + Press(start, Key.Down); + Assert.That(FocusedId(window), Does.StartWith("Reversal.fr."), + "the next line is the next group's"); + + Press(FocusedSlot(window), Key.Up); + Assert.That(FocusedId(window), Does.StartWith("Reversal.en.")); + } + + [AvaloniaTest] + public void ARunOfUpAndDown_KeepsTheHorizontalPositionItStartedFrom() + { + var (_, window, firstLine, _) = ShowWrappingGroup(); + var start = firstLine.Last(); + PlaceCaret(start, start.Text.Length); + + Press(start, Key.Down); + Press(FocusedSlot(window), Key.Up); + + Assert.That(FocusedSlot(window), Is.SameAs(start), + "the caret returns to where the run started, not to the slot under the line below"); + Assert.That(start.CaretIndex, Is.EqualTo(start.Text.Length)); + } + + [AvaloniaTest] + public void AClick_RestartsThePositionUpAndDownNavigateBy() + { + var (_, window, firstLine, secondLine) = ShowWrappingGroup(); + var start = firstLine.Last(); + PlaceCaret(start, start.Text.Length); + Press(start, Key.Down); + + Click(window, secondLine[0]); + Press(secondLine[0], Key.Up); + + Assert.That(FocusedSlot(window), Is.SameAs(firstLine[0]), + "Up follows the click's own position, not the run it interrupted"); + } + + [AvaloniaTest] + public void UpAtTheTopLine_AndDownAtTheBottom_StayPut() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling")); + var first = Find(field, "Reversal.en.0"); + var add = Find(field, "Reversal.en.Add"); + + PlaceCaret(first, 2); + Press(first, Key.Up); + Assert.That(first.IsFocused, Is.True); + + PlaceCaret(add, 0); + Press(add, Key.Down); + Assert.That(add.IsFocused, Is.True); + } + + [AvaloniaTest] + public void CtrlHomeAndCtrlEnd_GoToTheEndsOfTheWholeField() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, + English("dwelling", "abode"), French("maison")); + var first = Find(field, "Reversal.en.0"); + var last = Find(field, "Reversal.fr.Add"); + var middle = Find(field, "Reversal.fr.0"); + + PlaceCaret(middle, 1); + Press(middle, Key.Home, KeyModifiers.Control); + Assert.That(first.IsFocused, Is.True, "Ctrl+Home goes to the field's own first slot"); + Assert.That(first.CaretIndex, Is.Zero); + + PlaceCaret(first, 2); + Press(first, Key.End, KeyModifiers.Control); + Assert.That(last.IsFocused, Is.True, "Ctrl+End goes to the field's own last slot"); + Assert.That(last.CaretIndex, Is.EqualTo(last.Text.Length)); + } + [AvaloniaTest] public void CtrlArrows_AtASlotsEdge_MoveBetweenSlotsToo() { From a09aae97f43b424284942ab3ae8ba07c3dd7bf83 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Wed, 30 Sep 2026 17:04:53 -0500 Subject: [PATCH 07/12] LT-22673: Close a failed reversal batch and steady arrow navigation A batch that throws now closes the host's edit session, so whatever it wrote before the failure cannot reach a later save. The row scan moved inside the same guard: it reads a reversal index that may be deleted, and that throw used to escape to the caller. Arrow keys no longer navigate from a stale position. A modified arrow ends a run of Up and Down, and an arrow pressed with a selection collapses it, at the end the arrow points at, and goes no further. The next press moves. Co-Authored-By: Claude Opus 5 --- .../Detail/FwReversalEntriesField.cs | 32 +++++- .../Detail/IReversalEntryEditing.cs | 4 +- .../Detail/FwReversalEntriesFieldTests.cs | 103 +++++++++++++++++- .../Plugins/ReversalIndexEntryPlugin.cs | 50 +++++---- .../Composer/ReversalEntriesComposeTests.cs | 35 ++++++ 5 files changed, 191 insertions(+), 33 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 787dc4b20c..3b5ad3d325 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -452,8 +452,9 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) { EventHandler keyDown = (s, e) => { - // Only an unbroken run of Up/Down keeps the position it navigates by. - if (e.Key != Key.Up && e.Key != Key.Down) + // Modified arrows can move the caret without joining a plain Up/Down run. + if ((e.Key != Key.Up && e.Key != Key.Down) + || e.KeyModifiers != KeyModifiers.None) _lineNavigationX = null; if (e.Key == Key.Enter) { @@ -490,6 +491,16 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) e.Handled = true; return; } + // An arrow that is not extending a selection collapses it and goes no further, + // the way a text box does; the position a run navigated by goes with it. + if (IsArrow(e.Key) && (e.KeyModifiers & KeyModifiers.Shift) == KeyModifiers.None + && editor.SelectionStart != editor.SelectionEnd) + { + CollapseSelection(editor, e.Key, rightToLeft); + _lineNavigationX = null; + e.Handled = true; + return; + } if ((e.Key == Key.Up || e.Key == Key.Down) && e.KeyModifiers == KeyModifiers.None) { // Past the first or last line the key is left alone, for the view to answer. @@ -502,8 +513,6 @@ private void WireSlotNavigation(TextBox editor, bool rightToLeft) { return; } - if (editor.SelectionStart != editor.SelectionEnd) - return; var toward = (e.Key == Key.Left) != rightToLeft ? -1 : 1; var length = (editor.Text ?? string.Empty).Length; if (toward < 0 ? editor.CaretIndex != 0 : editor.CaretIndex != length) @@ -651,6 +660,21 @@ private static void PlaceCaret(TextBox target, bool atEnd) SetCaret(target, atEnd ? (target.Text ?? string.Empty).Length : 0); } + private static bool IsArrow(Key key) + => key == Key.Left || key == Key.Right || key == Key.Up || key == Key.Down; + + // The caret lands at the end of the selection the arrow points at: its start for Up + // and Left, its end for Down and Right, mirrored in a right-to-left group. + private static void CollapseSelection(TextBox editor, Key key, bool rightToLeft) + { + var toStart = key == Key.Up || key == Key.Down + ? key == Key.Up + : (key == Key.Left) != rightToLeft; + SetCaret(editor, toStart + ? Math.Min(editor.SelectionStart, editor.SelectionEnd) + : Math.Max(editor.SelectionStart, editor.SelectionEnd)); + } + private static void SetCaret(TextBox target, int caret) { target.CaretIndex = caret; diff --git a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs index 49652103d0..61c3ba70e7 100644 --- a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs +++ b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs @@ -31,7 +31,9 @@ public interface IReversalEntryEditing /// /// Row key to typed text, one pair per changed row. /// False, without opening the session, when no row changes or the sense no - /// longer exists; also false, after logging, when the write fails. + /// longer exists; also false when the write fails, which is logged and closes the + /// session, edits and all, rather than leaving a half-written batch to be + /// saved. bool TryCommitRows(IReadOnlyList> edits); /// Stages one row's text: a of that single diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs index a082390412..7966ec5173 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -824,6 +824,103 @@ public void AClick_RestartsThePositionUpAndDownNavigateBy() "Up follows the click's own position, not the run it interrupted"); } + // A mouse-drag selection cannot be driven headlessly, and a key-made one would clear the + // run by itself. The caret goes at the selection's start, where it does not collapse it. + private static void PlaceSelection(TextBox box, int caret, int start, int end) + { + box.Focus(); + box.SelectionStart = start; + box.SelectionEnd = end; + box.CaretIndex = caret; + Assert.That(box.SelectionStart, Is.Not.EqualTo(box.SelectionEnd), + "precondition: the slot holds a selection"); + Assert.That(box.CaretIndex, Is.EqualTo(caret), "precondition: the caret is where it was put"); + } + + [AvaloniaTest] + public void AnArrowWithASelection_OnlyCollapsesIt_AndTheNextPressMoves() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var first = Find(field, "Reversal.en.0"); + var second = Find(field, "Reversal.en.1"); + PlaceSelection(second, 0, 0, "abode".Length); + + Press(second, Key.Left); + + Assert.That(second.SelectionStart, Is.EqualTo(second.SelectionEnd), "the selection is gone"); + Assert.That(second.IsFocused, Is.True, "the first press collapses and goes no further"); + Assert.That(second.CaretIndex, Is.Zero, "Left collapses to the selection's start"); + + Press(second, Key.Left); + + Assert.That(first.IsFocused, Is.True, "the next press moves as usual"); + Assert.That(first.CaretIndex, Is.EqualTo("dwelling".Length)); + } + + [AvaloniaTest] + public void RightWithASelection_CollapsesToItsEnd() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var second = Find(field, "Reversal.en.1"); + PlaceSelection(second, 0, 0, "abode".Length); + + Press(second, Key.Right); + + Assert.That(second.IsFocused, Is.True); + Assert.That(second.CaretIndex, Is.EqualTo("abode".Length)); + Assert.That(second.SelectionStart, Is.EqualTo(second.SelectionEnd)); + } + + [AvaloniaTest] + public void ASelection_RestartsThePositionUpAndDownNavigateBy() + { + var (_, window, firstLine, secondLine) = ShowWrappingGroup(); + var start = firstLine.Last(); + PlaceCaret(start, start.Text.Length); + Press(start, Key.Down); + var target = secondLine[0]; + PlaceSelection(target, 0, 0, target.Text.Length); + + Press(target, Key.Up); + Assert.That(target.SelectionStart, Is.EqualTo(target.SelectionEnd), "the selection is gone"); + Assert.That(FocusedSlot(window), Is.SameAs(target), "the first press only collapses"); + + Press(target, Key.Up); + + Assert.That(FocusedSlot(window), Is.SameAs(firstLine[0]), + "the selection ended the run, so Up follows the caret's own position"); + } + + [AvaloniaTest] + public void ShiftArrows_StillExtendTheSelection() + { + var (field, _, _) = Show(new RecordingReversalContext(), null, English("dwelling", "abode")); + var box = Find(field, "Reversal.en.0"); + PlaceSelection(box, 0, 0, 3); + + Press(box, Key.Right, KeyModifiers.Shift); + + Assert.That(box.SelectionStart, Is.Not.EqualTo(box.SelectionEnd), + "a selection gesture is not a navigation one, so it is left alone"); + Assert.That(box.IsFocused, Is.True); + } + + [AvaloniaTest] + public void AModifiedArrow_RestartsThePositionUpAndDownNavigateBy() + { + var (_, window, firstLine, secondLine) = ShowWrappingGroup(); + var start = firstLine.Last(); + PlaceCaret(start, start.Text.Length); + Press(start, Key.Down); + PlaceCaret(secondLine[0], 0); + + Press(secondLine[0], Key.Up, KeyModifiers.Shift); + Press(secondLine[0], Key.Up); + + Assert.That(FocusedSlot(window), Is.SameAs(firstLine[0]), + "the modified arrow ended the run, so Up follows the caret's own position"); + } + [AvaloniaTest] public void UpAtTheTopLine_AndDownAtTheBottom_StayPut() { @@ -895,12 +992,6 @@ public void ArrowsInsideASlot_StayInIt() PlaceCaret(second, 2); Press(second, Key.Left, KeyModifiers.Control); Assert.That(second.IsFocused, Is.True, "Ctrl+Left inside the text moves within the slot"); - - second.Focus(); - second.SelectionStart = 0; - second.SelectionEnd = 3; - Press(second, Key.Left); - Assert.That(second.IsFocused, Is.True, "an arrow with a selection keeps its text behavior"); } [AvaloniaTest] diff --git a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs index 930cb24ea3..4bb16eb2be 100644 --- a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs +++ b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs @@ -246,28 +246,30 @@ public bool TryCommitRows(IReadOnlyList> edits) if (edits == null || !_sense.IsValidObject) return false; - var changes = new List(); - foreach (var edit in edits) - { - RowBinding binding; - if (string.IsNullOrEmpty(edit.Key) || !_rows.TryGetValue(edit.Key, out binding)) - continue; - var ws = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(binding.Index.WritingSystem); - if (ws <= 0) - continue; - if (binding.Entry != null && !binding.Entry.IsValidObject) - binding.Entry = null; - var forms = SplitForms(edit.Value); - if (binding.Entry == null ? forms.Count == 0 : ChainMatches(binding.Entry, forms, ws)) - continue; - changes.Add(new RowChange(binding, forms, ws)); - } - if (changes.Count == 0) - return false; - - var previous = changes.Select(change => change.Binding.Entry).ToList(); + List changes = null; + List previous = null; try { + changes = new List(); + foreach (var edit in edits) + { + RowBinding binding; + if (string.IsNullOrEmpty(edit.Key) || !_rows.TryGetValue(edit.Key, out binding)) + continue; + var ws = _cache.ServiceLocator.WritingSystemManager.GetWsFromStr(binding.Index.WritingSystem); + if (ws <= 0) + continue; + if (binding.Entry != null && !binding.Entry.IsValidObject) + binding.Entry = null; + var forms = SplitForms(edit.Value); + if (binding.Entry == null ? forms.Count == 0 : ChainMatches(binding.Entry, forms, ws)) + continue; + changes.Add(new RowChange(binding, forms, ws)); + } + if (changes.Count == 0) + return false; + + previous = changes.Select(change => change.Binding.Entry).ToList(); return StageOnHost(() => { // Every row takes its new entry before any entry is let go, so an entry @@ -299,8 +301,12 @@ public bool TryCommitRows(IReadOnlyList> edits) } catch (Exception e) { - // The write rolled back or never ran, so the rows keep the entries they showed. - for (var i = 0; i < changes.Count; i++) + // A session this call did not open keeps whatever the batch wrote before it + // threw, so the whole step closes rather than reaching the next save + // half-linked. The rows go back to the entries they showed. + if (_host != null && _host.IsOpen) + _host.Cancel(); + for (var i = 0; previous != null && i < changes.Count; i++) changes[i].Binding.Entry = previous[i]; Logger.WriteError(e); return false; diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs index 3eab6bc390..bcd61c47ce 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -639,6 +639,41 @@ public void Settling_SavesWhatTheFieldHolds_WhileFocusIsStillInIt() Is.EqualTo("abode")); } + // A failed batch closes the session it wrote into, at the cost of the edit that opened + // it, rather than carrying half a batch to the next save. + [Test] + public void AFailedBatch_ClosesTheSession_LeavingNothingToSave() + { + var es = AddAnalysisWs("es"); + var esIndex = AddIndex(es); + var (editing, host) = NewContext(); + var groups = editing.CreateGroups(null); + var enAdd = AddRow(Group(groups, EnTag)); + var esAdd = AddRow(Group(groups, es.Id)); + // The second row still points at this index, which is gone by the time it writes. + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => Cache.LanguageProject.LexDbOA.ReversalIndexesOC.Remove(esIndex)); + ((DetailEditContextBase)host).Stage(() => + { + m_sense.Gloss.set_String(EnWs, "seed"); + return true; + }, "Gloss"); + Assert.That(host.IsOpen, Is.True, "precondition: another field's edit opened the session"); + + var staged = editing.TryCommitRows(new[] + { + new KeyValuePair(enAdd.RowKey, "home"), + new KeyValuePair(esAdd.RowKey, "casa") + }); + + Assert.That(staged, Is.False); + Assert.That(host.IsOpen, Is.False, "the failed batch closed the session"); + Assert.That(m_sense.ReferringReversalIndexEntries, Is.Empty, + "neither row was written, the one before the failure included"); + Assert.That(m_sense.Gloss.get_String(EnWs).Text, Is.Null, + "the edit that opened the session went with it"); + } + [Test] public void AnAddedEntry_PersistsIntoTheNextCompose() { From 78c87ed19b2b23ef6a4594788c88b45f36dda63e Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:40:20 -0500 Subject: [PATCH 08/12] LT-22673: Drop four reversal tests that prove nothing of their own Each one's assertions are already made by another test, or cannot fail for a bug in this code: - EntriesInAHiddenWs_ProduceNoRow asserts exactly what EntriesOnlyInAHiddenWs_StillComposeTheRow asserts, and less. - AnExternalLinkChange_ShowsOnTheNextCompose shows only that a fresh compose reads current state, which AnAddedEntry_PersistsIntoTheNextCompose shows along the path a user takes. - ACommit_TripsNoValidationRule passes whether or not the context delegates validation: the entry always has a lexeme form. - ALoneAddSlot_FillsTheWholeLine is the degenerate case of the stretch already covered by TheAddSlot_FillsTheRestOfItsLine_AndEntrySlotsDoNot. Co-Authored-By: Claude Opus 5 --- .../Detail/FwReversalEntriesFieldTests.cs | 11 ------ .../Composer/ReversalEntriesComposeTests.cs | 35 ------------------- 2 files changed, 46 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs index 7966ec5173..5bf2c89b87 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/FwReversalEntriesFieldTests.cs @@ -205,17 +205,6 @@ public void TheAddSlot_FillsTheRestOfItsLine_AndEntrySlotsDoNot() "an entry slot stays as wide as its text"); } - [AvaloniaTest] - public void ALoneAddSlot_FillsTheWholeLine() - { - var (field, _, _) = Show(new RecordingReversalContext(), null, English()); - var group = Find(field, "Reversal.en"); - var add = Find(field, "Reversal.en.Add"); - - Assert.That(add.Bounds.X, Is.Zero); - Assert.That(add.Bounds.Width, Is.EqualTo(group.Bounds.Width).Within(0.5)); - } - [AvaloniaTest] public void AfterGrowth_OnlyTheNewLastSlotStretches() { diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs index bcd61c47ce..499bc2a5f0 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -252,18 +252,6 @@ public void EntriesOnlyInAHiddenWs_StillComposeTheRow() Assert.That(AddRow(Group(groups, EnTag)), Is.Not.Null, "the visible index offers its add row"); } - [Test] - public void EntriesInAHiddenWs_ProduceNoRow() - { - var es = AddAnalysisWs("es"); - AddEntry(m_enIndex, "dwelling", m_sense); - AddEntry(AddIndex(es), "casa", m_sense); - - var groups = NewContext().Editing.CreateGroups(new[] { EnTag }); - - Assert.That(groups.Select(g => g.WsTag), Is.EqualTo(new[] { EnTag })); - } - // ----- Edit -> one undo step ----- [Test] @@ -685,31 +673,8 @@ public void AnAddedEntry_PersistsIntoTheNextCompose() Assert.That(EntryTexts(Group(NewContext().Editing.CreateGroups(null), EnTag)), Is.EqualTo(new[] { "home" })); } - // ----- Validation ----- - - [Test] - public void ACommit_TripsNoValidationRule() - { - var (editing, host) = NewContext(); - editing.TryCommitRow(AddRow(Group(editing.CreateGroups(null), EnTag)).RowKey, "home"); - - Assert.That(host.Validate(), Is.Empty); - Assert.That(editing.Validate(), Is.Empty); - host.Commit(); - } - // ----- Re-show ----- - [Test] - public void AnExternalLinkChange_ShowsOnTheNextCompose() - { - var (editing, _) = NewContext(); - editing.CreateGroups(null); - AddEntry(m_enIndex, "dwelling", m_sense); - - Assert.That(EntryTexts(Group(NewContext().Editing.CreateGroups(null), EnTag)), Is.EqualTo(new[] { "dwelling" })); - } - [Test] public void UndoAndRedo_OfAnAdd_RoundTrip() { From 6b2962b0b5ca85222ea98a0c58c144e486ee3263 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:06:12 -0500 Subject: [PATCH 09/12] LT-22673: Keep only the live control's save hook for a row A section toggle rebuilds a row's controls without building a new edit context, so every rebuild registered another pending-edit flush. The context kept each control it had replaced and asked all of them for their text on later saves. AddPendingEditFlush now takes the row's own id as a key, and a later registration under that key replaces the one before it. Co-Authored-By: Claude Opus 5 --- Src/xWorks/Avalonia/DetailEditContextBase.cs | 17 +++++++---- .../Plugins/ReversalIndexEntryPlugin.cs | 10 ++++--- .../Composer/ReversalEntriesComposeTests.cs | 28 +++++++++++++++++++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/Src/xWorks/Avalonia/DetailEditContextBase.cs b/Src/xWorks/Avalonia/DetailEditContextBase.cs index ea93515c9a..1d40782eec 100644 --- a/Src/xWorks/Avalonia/DetailEditContextBase.cs +++ b/Src/xWorks/Avalonia/DetailEditContextBase.cs @@ -21,7 +21,8 @@ namespace SIL.FieldWorks.XWorks public abstract class DetailEditContextBase : IDetailEditContext { private LcmDetailEditSession _session; - private readonly List _pendingEditFlushes = new List(); + private readonly Dictionary _pendingEditFlushes = + new Dictionary(StringComparer.Ordinal); protected DetailEditContextBase(LcmCache cache, ICmObject root) { @@ -106,10 +107,16 @@ public virtual IReadOnlyList Validate() /// focus leaves it. runs every registered flush, so a /// save made while focus is still inside such an editor includes what it holds. /// - public void AddPendingEditFlush(Action flush) + /// Identifies the row the editor belongs to, the same across + /// rebuilds of it. The view rebuilds a row's controls on a section toggle without + /// building a new context, so a later registration under the same key replaces the + /// earlier one and the context holds only the live editor. + /// Stages what that editor holds; null registers nothing. + public void AddPendingEditFlush(string ownerKey, Action flush) { - if (flush != null) - _pendingEditFlushes.Add(flush); + if (string.IsNullOrEmpty(ownerKey) || flush == null) + return; + _pendingEditFlushes[ownerKey] = flush; } /// @@ -118,7 +125,7 @@ public void AddPendingEditFlush(Action flush) /// public void FlushPendingEdits() { - foreach (var flush in _pendingEditFlushes.ToArray()) + foreach (var flush in new List(_pendingEditFlushes.Values)) flush(); } diff --git a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs index 4bb16eb2be..59167ad3b1 100644 --- a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs +++ b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs @@ -58,13 +58,15 @@ public Control BuildControl(SlicePluginBuildContext context) var host = context.EditContext; var editing = new ReversalDetailEditContext(cache, host, sense, label); var groups = editing.CreateGroups(context.VisibleWritingSystems); + // The row's identity, which every rebuild of it shares. + var fieldId = "reversal/" + sense.Hvo; Action navigate = null; var linkRequested = context.Render?.LinkRequested; if (linkRequested != null) { var field = new DetailField( - stableId: "reversal/" + sense.Hvo, + stableId: fieldId, label: label, field: node?.Field, writingSystem: node?.WritingSystem, @@ -89,9 +91,9 @@ public Control BuildControl(SlicePluginBuildContext context) var control = new FwReversalEntriesField(label, automationId, groups, host == null ? null : editing, context.WritingSystemFocused, navigate, context.Render?.WsAbbrevColumnWidth); - // The field stages only when focus leaves it, so the host's save must ask for its - // edits when it runs with focus still inside. - (host as DetailEditContextBase)?.AddPendingEditFlush(control.CommitPendingEdits); + // The field stages only when focus leaves it, so the host's save asks it for what + // it holds. The row's own id as the key drops the control a rebuild replaced. + (host as DetailEditContextBase)?.AddPendingEditFlush(fieldId, control.CommitPendingEdits); return control; } catch (Exception e) diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs index 499bc2a5f0..b0e67d1100 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -627,6 +627,34 @@ public void Settling_SavesWhatTheFieldHolds_WhileFocusIsStillInIt() Is.EqualTo("abode")); } + // A section toggle rebuilds a row's controls without building a new edit context, so the + // control a rebuild replaced must not still be asked for the text it was holding. + [Test] + public void ARebuiltRow_LeavesOnlyTheLiveControlHoldingEdits() + { + AddEntry(m_enIndex, "dwelling", m_sense); + var host = DetailComposer.Compose(m_entry, Cache).EditContext; + var holder = new DetailEditContextHolder(); + holder.Replace(host); + var replaced = BuildReversalField(host); + var live = BuildReversalField(host); + Slot(replaced, "dwelling").Text = "replaced"; + Slot(live, "dwelling").Text = "abode"; + + holder.Settle(); + + Assert.That(m_sense.ReferringReversalIndexEntries.Select(e => e.ReversalForm.get_String(EnWs).Text), + Is.EqualTo(new[] { "abode" }), "only the control the row shows now writes its text"); + } + + private FwReversalEntriesField BuildReversalField(IDetailEditContext host) + => (FwReversalEntriesField)new ReversalIndexEntryPlugin().BuildControl( + new SlicePluginBuildContext(m_sense, null, () => host, Cache)); + + private static Avalonia.Controls.TextBox Slot(FwReversalEntriesField field, string text) + => field.GetLogicalDescendants().OfType() + .Single(box => box.Text == text); + // A failed batch closes the session it wrote into, at the cost of the edit that opened // it, rather than carrying half a batch to the next save. [Test] From 86b81574942dc62a7c1ebb2f935e68a8d6d3003b Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:45:51 -0500 Subject: [PATCH 10/12] LT-22673: Cancel a failed reversal batch through the view A failed batch cancelled the host's edit session directly, behind the detail view. The view never heard of the cancel and never re-showed, so every field kept showing edits the cancel had rolled back, and the reversal field kept treating its own rolled-back edits as saved, so it never retried them. The view now hands its controls its own cancel through SliceFactoryContext, and a failed batch uses it when another edit's session is still open: the session rolls back and the view re-shows from the model, as it does for Escape. With no view, the session is cancelled directly as before. Co-Authored-By: Claude Opus 5.5 --- Src/Common/FwAvalonia/Detail/DataTree.cs | 3 +- .../Detail/IReversalEntryEditing.cs | 6 +-- Src/Common/FwAvalonia/Detail/SliceFactory.cs | 12 ++++- .../DetailCustomFieldRenderingTests.cs | 26 ++++++++++ .../Plugins/ReversalIndexEntryPlugin.cs | 22 +++++--- .../Composer/ReversalEntriesComposeTests.cs | 51 +++++++++++++++++++ 6 files changed, 109 insertions(+), 11 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 4b4f6c1171..412fa9c1e8 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -851,6 +851,7 @@ private Control CreateEditor(DetailField field, string automationId) // Legacy labels each alternative of a MultiStringSlice and leaves a StringSlice's // single value unlabelled, so the gutter follows the row's own kind. showWritingSystemAbbreviation: field.IsMultiStringRow, - wsAbbrevColumnWidth: _wsAbbrevColumnWidth)); + wsAbbrevColumnWidth: _wsAbbrevColumnWidth, + cancel: _editContext == null ? (Action)null : OnCancel)); } } diff --git a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs index 61c3ba70e7..0a562cfba6 100644 --- a/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs +++ b/Src/Common/FwAvalonia/Detail/IReversalEntryEditing.cs @@ -31,9 +31,9 @@ public interface IReversalEntryEditing /// /// Row key to typed text, one pair per changed row. /// False, without opening the session, when no row changes or the sense no - /// longer exists; also false when the write fails, which is logged and closes the - /// session, edits and all, rather than leaving a half-written batch to be - /// saved. + /// longer exists; also false when the write fails, which is logged and cancels the + /// session, edits and all, the way the view's own cancel does, rather than leaving a + /// half-written batch to be saved. bool TryCommitRows(IReadOnlyList> edits); /// Stages one row's text: a of that single diff --git a/Src/Common/FwAvalonia/Detail/SliceFactory.cs b/Src/Common/FwAvalonia/Detail/SliceFactory.cs index f690ea4aa5..bc3f2a72a4 100644 --- a/Src/Common/FwAvalonia/Detail/SliceFactory.cs +++ b/Src/Common/FwAvalonia/Detail/SliceFactory.cs @@ -31,7 +31,8 @@ public SliceFactoryContext( IFwClipboard clipboard = null, Action save = null, bool showWritingSystemAbbreviation = true, - double? wsAbbrevColumnWidth = null) + double? wsAbbrevColumnWidth = null, + Action cancel = null) { EditContext = editContext; WritingSystemFocused = writingSystemFocused; @@ -39,6 +40,7 @@ public SliceFactoryContext( LinkRequested = linkRequested; Clipboard = clipboard; Save = save; + Cancel = cancel; ShowWritingSystemAbbreviation = showWritingSystemAbbreviation; WsAbbrevColumnWidth = wsAbbrevColumnWidth ?? FwAvaloniaDensity.WsAbbrevWidth; } @@ -68,6 +70,14 @@ public SliceFactoryContext( /// public Action Save { get; } + /// + /// Cancels the view's open edit session and has the host re-show the view from the + /// domain, as Escape does. An editor that cannot finish a write calls this rather than + /// cancelling the session itself, so no field is left showing text the cancel rolled + /// back. Null on hosts that drive their own sessions. + /// + public Action Cancel { get; } + /// /// Whether a multi-WS text field shows its per-WS abbreviation gutter. The detail pane shows it; /// a dense in-cell editor suppresses it (matching the legacy in-cell editor). diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs index a2faeec11a..12c67fa695 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs @@ -119,5 +119,31 @@ public void CustomField_FactoryReceivesTheViewsLinkCallback() received(new DetailLinkRequest(null, new DetailChooserLink("Show", "someTool"))); Assert.That(requests, Has.Count.EqualTo(1), "the callback is the one the view was given"); } + + // A plugin that cannot finish a write cancels through the view, not the session itself, + // so the host re-shows every field from the domain. + [AvaloniaTest] + public void CustomField_FactoryReceivesTheViewsCancel_WhichAlsoCompletesTheEdit() + { + Action cancel = null; + var model = Model(render => + { + cancel = render.Cancel; + return new TextBlock { Text = "plugin" }; + }); + var context = new FakeDetailEditContext(); + var view = new DataTree(model, editContext: context); + var completed = 0; + view.EditCompleted += (s, e) => completed++; + var window = new Window { Content = view, Width = 420, Height = 200 }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + + Assert.That(cancel, Is.Not.Null, "an editable view hands its controls a cancel"); + cancel(); + + Assert.That(context.CancelCount, Is.EqualTo(1), "the view's session was cancelled"); + Assert.That(completed, Is.EqualTo(1), "and the host is told to re-show"); + } } } diff --git a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs index 59167ad3b1..f7331cd633 100644 --- a/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs +++ b/Src/xWorks/Avalonia/Plugins/ReversalIndexEntryPlugin.cs @@ -56,7 +56,8 @@ public Control BuildControl(SlicePluginBuildContext context) : StringTable.Table.LocalizeAttributeValue(node.Label); var automationId = node?.AutomationId ?? DefaultAutomationId; var host = context.EditContext; - var editing = new ReversalDetailEditContext(cache, host, sense, label); + var editing = new ReversalDetailEditContext(cache, host, sense, label, + context.Render?.Cancel); var groups = editing.CreateGroups(context.VisibleWritingSystems); // The row's identity, which every rebuild of it shares. var fieldId = "reversal/" + sense.Hvo; @@ -125,17 +126,22 @@ internal sealed class ReversalDetailEditContext : IDetailEditContext, IReversalE private readonly IDetailEditContext _host; private readonly ILexSense _sense; private readonly string _fieldLabel; + private readonly Action _cancelView; private readonly Dictionary _rows = new Dictionary(StringComparer.Ordinal); private int _nextRowKey; + /// The view's own cancel, which also re-shows it from the + /// model; null when there is no view, and a failed write then cancels the host's session + /// directly. public ReversalDetailEditContext(LcmCache cache, IDetailEditContext host, ILexSense sense, - string fieldLabel) + string fieldLabel, Action cancelView = null) { _cache = cache ?? throw new ArgumentNullException(nameof(cache)); _sense = sense ?? throw new ArgumentNullException(nameof(sense)); _host = host; _fieldLabel = fieldLabel; + _cancelView = cancelView; } // The reversal index a row belongs to and the entry it shows; null is an add row. @@ -303,11 +309,15 @@ public bool TryCommitRows(IReadOnlyList> edits) } catch (Exception e) { - // A session this call did not open keeps whatever the batch wrote before it - // threw, so the whole step closes rather than reaching the next save - // half-linked. The rows go back to the entries they showed. + // A session still open holds what the batch wrote before it threw. Cancelling it + // through the view also re-shows every field, none left showing rolled-back text. if (_host != null && _host.IsOpen) - _host.Cancel(); + { + if (_cancelView != null) + _cancelView(); + else + _host.Cancel(); + } for (var i = 0; previous != null && i < changes.Count; i++) changes[i].Binding.Entry = previous[i]; Logger.WriteError(e); diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs index b0e67d1100..9799c75221 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/ReversalEntriesComposeTests.cs @@ -655,6 +655,57 @@ private static Avalonia.Controls.TextBox Slot(FwReversalEntriesField field, stri => field.GetLogicalDescendants().OfType() .Single(box => box.Text == text); + // A context on the composed host whose view cancel is counted, and an add row in an index + // that is deleted before the row writes, so any batch through it fails. + private (ReversalDetailEditContext Editing, IDetailEditContext Host, string DoomedRow, + Func ViewCancels) ContextWithADoomedRow() + { + var es = AddAnalysisWs("es"); + var esIndex = AddIndex(es); + var host = DetailComposer.Compose(m_entry, Cache).EditContext; + var cancels = 0; + var editing = new ReversalDetailEditContext(Cache, host, m_sense, "Reversal Entries", () => + { + cancels++; + host.Cancel(); + }); + var doomed = AddRow(Group(editing.CreateGroups(null), es.Id)).RowKey; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => Cache.LanguageProject.LexDbOA.ReversalIndexesOC.Remove(esIndex)); + return (editing, host, doomed, () => cancels); + } + + // Cancelling through the view re-shows it, so no other field keeps showing text the + // cancel rolled back. + [Test] + public void AFailedBatch_InAnotherFieldsSession_CancelsThroughTheView() + { + var (editing, host, doomed, viewCancels) = ContextWithADoomedRow(); + ((DetailEditContextBase)host).Stage(() => + { + m_sense.Gloss.set_String(EnWs, "seed"); + return true; + }, "Gloss"); + + Assert.That(editing.TryCommitRow(doomed, "casa"), Is.False); + + Assert.That(viewCancels(), Is.EqualTo(1), "the view cancelled, so it re-shows"); + Assert.That(host.IsOpen, Is.False); + } + + // With nothing else staged there is nothing stale to re-show, and a view cancel would + // only throw away whatever the user is still typing. + [Test] + public void AFailedBatch_WithNothingElseStaged_LeavesTheViewAlone() + { + var (editing, host, doomed, viewCancels) = ContextWithADoomedRow(); + + Assert.That(editing.TryCommitRow(doomed, "casa"), Is.False); + + Assert.That(viewCancels(), Is.Zero); + Assert.That(host.IsOpen, Is.False, "no session is left open either way"); + } + // A failed batch closes the session it wrote into, at the cost of the edit that opened // it, rather than carrying half a batch to the next save. [Test] From 622cf1554dd8cda33099b98ba6de734fe0c5a848 Mon Sep 17 00:00:00 2001 From: Hasso <4933670+papeh@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:27:09 -0500 Subject: [PATCH 11/12] LT-22673: Dispose the detail editors a view replaces Collapsing or expanding a section rebuilt the rows' editors, and every re-show and "no entry" message replaced the whole view, but nothing ever disposed the editors being replaced, so the handlers they had attached stayed attached. The view now disposes the editors a rebuild replaces, after the form has let go of them so a focus loss their removal raises still reaches their handlers, and implements IDisposable for the rest. Its WinForms host disposes the content a swap replaces, the same way, and its current content when the host itself is disposed. The Reversal Entries field also raises Disposed, so the plugin drops its pending-edit flush from the host; a collapsed row no longer leaves its control registered there until the next re-show. Co-Authored-By: Claude Opus 5.5 --- .../FwAvalonia/AvaloniaHostControlBase.cs | 18 ++++++- Src/Common/FwAvalonia/Detail/DataTree.cs | 30 +++++++++++- .../Detail/FwReversalEntriesField.cs | 7 +++ .../DetailCustomFieldRenderingTests.cs | 48 +++++++++++++++++++ .../FwAvaloniaTests/DetailHostControlTests.cs | 48 +++++++++++++++++++ Src/xWorks/Avalonia/DetailEditContextBase.cs | 18 +++++++ .../Plugins/ReversalIndexEntryPlugin.cs | 9 +++- .../Composer/ReversalEntriesComposeTests.cs | 17 +++++++ 8 files changed, 191 insertions(+), 4 deletions(-) diff --git a/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs b/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs index 3ed7a73fb1..f4189adbb7 100644 --- a/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs +++ b/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs @@ -79,10 +79,20 @@ protected void RaiseDetailInteractionCompleted() /// Swaps the hosted Avalonia content and shows the control. protected void SetHostContent(Avalonia.Controls.Control content) { - Host.Content = content; + ReplaceContent(content); Show(); } + // Every swap builds new content, so the outgoing content is never shown again. It is + // disposed only once out of the host, so a focus loss its removal raises still lands. + private void ReplaceContent(Avalonia.Controls.Control content) + { + var outgoing = Host.Content; + Host.Content = content; + if (!ReferenceEquals(outgoing, content)) + (outgoing as IDisposable)?.Dispose(); + } + /// The current Avalonia content, or null. protected Avalonia.Controls.Control CurrentContent => Host.Content as Avalonia.Controls.Control; @@ -126,6 +136,10 @@ protected override void Dispose(bool disposing) _companionStrip.Controls.RemoveAt(i); } } + // An owner settles its edit session before disposing this control, so disposing the + // content only releases its editors. + if (disposing) + (Host?.Content as IDisposable)?.Dispose(); base.Dispose(disposing); } @@ -180,7 +194,7 @@ public void ShowContextMenu(IReadOnlyList items, public void ShowMessage(string message) { - Host.Content = new Avalonia.Controls.TextBlock { Text = message ?? string.Empty }; + ReplaceContent(new Avalonia.Controls.TextBlock { Text = message ?? string.Empty }); Show(); } diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 412fa9c1e8..924073d651 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -33,7 +33,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// undo step per field, no Save/Cancel buttons. Validation failures show inline and block the /// commit; Escape rolls the session back. Without a context the view is read-only display. /// - public sealed class DataTree : UserControl, IDetailPopupSink + public sealed class DataTree : UserControl, IDetailPopupSink, IDisposable { private readonly IDetailEditContext _editContext; private readonly Action _writingSystemFocused; @@ -58,6 +58,10 @@ public sealed class DataTree : UserControl, IDetailPopupSink // The vector rows of the shown fields, and the one whose item is current (at most one // per view), so re-show continuity can read and restore that selection cheaply. private readonly List _vectors = new List(); + // The editors the shown rows hold that attach handlers of their own, disposed when a + // rebuild replaces them or the view itself is disposed. + private readonly List _editors = new List(); + private bool _disposed; /// The reference-vector row that has a current item; null when none /// does. @@ -334,6 +338,22 @@ private static void ApplyRowTabIndex(Control root, int row) /// The detail model this view renders. public DetailModel Model { get; } + /// + /// Disposes the editors the view holds, detaching every handler they attached. The host + /// calls this once the view is out of its window and never shown again, since a + /// disposed editor ignores what is typed into it. + /// + public void Dispose() + { + if (_disposed) + return; + _disposed = true; + var editors = _editors.ToArray(); + _editors.Clear(); + foreach (var editor in editors) + editor.Dispose(); + } + /// /// Raised after a commit or cancel completed, so the host can re-resolve and re-show the /// detail view from current domain state. @@ -479,7 +499,13 @@ private void DeliverWhenIdle() // expanded. private void RebuildItems() { + var replaced = _editors.ToArray(); + _editors.Clear(); _form.Items.Clear(); + // Only once the form has let go of them, so a focus loss their removal raises still + // reaches their own handlers and stages what they hold. + foreach (var editor in replaced) + editor.Dispose(); _vectors.Clear(); _labelBlocks.Clear(); SelectedVector = null; @@ -677,6 +703,8 @@ private FieldContent AddField(int row, DetailField field) AutomationProperties.SetName(labelBlock, field.Label ?? field.Field ?? string.Empty); ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // the label text is its own tip var editor = CreateEditor(field, automationId); + if (editor is IDisposable disposable) + _editors.Add(disposable); editor.Margin = new Thickness(0, 0, 0, FwAvaloniaDensity.FieldSpacing); // Every reachable control in a row shares its TabIndex, so Avalonia visits them in diff --git a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs index 3b5ad3d325..4e60c8ddea 100644 --- a/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs +++ b/Src/Common/FwAvalonia/Detail/FwReversalEntriesField.cs @@ -858,6 +858,13 @@ public void Dispose() foreach (var detach in _teardown) detach(); _teardown.Clear(); + Disposed?.Invoke(this, EventArgs.Empty); } + + /// + /// Raised once, when the field is disposed, so anything that registered it elsewhere, + /// such as a pending-edit flush on the host, can let it go. + /// + public event EventHandler Disposed; } } diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs index 12c67fa695..33e8c5aff5 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs @@ -8,6 +8,7 @@ using Avalonia.Automation; using Avalonia.Controls; using Avalonia.Headless.NUnit; +using Avalonia.Interactivity; using Avalonia.Threading; using Avalonia.VisualTree; using NUnit.Framework; @@ -120,6 +121,53 @@ public void CustomField_FactoryReceivesTheViewsLinkCallback() Assert.That(requests, Has.Count.EqualTo(1), "the callback is the one the view was given"); } + // A collapse rebuilds the rows with new editors, so the ones it removes are disposed, and + // expanding again builds fresh ones rather than reviving them. + [AvaloniaTest] + public void CollapsingASection_DisposesTheEditorsItRemoves() + { + var built = new List(); + var model = new DetailModel("LexEntry", "Normal", new List + { + new DetailField("h", "Sense 1", null, null, DetailFieldKind.Header, + EditorClassification.GroupingNone, null, null, HostRouting.Inherit, null, null, null, + isEditable: false, indent: 0, isCollapsible: true, isInitiallyExpanded: true), + new DetailField("c", "Messages", "Self", null, DetailFieldKind.Custom, + EditorClassification.Dynamic, null, null, HostRouting.Product, null, null, null, + isEditable: true, indent: 1, controlFactory: _ => + { + var spy = new DisposalSpy(); + built.Add(spy); + return spy; + }) + }, + new List()); + var view = Show(model); + var header = view.GetVisualDescendants().OfType