From b79b8fce584ddf33bc16d6b30e8a90c78208c1aa Mon Sep 17 00:00:00 2001 From: Ariel Rorabaugh Date: Tue, 15 Sep 2026 09:43:37 -0400 Subject: [PATCH 1/8] LT-22688: Implement DataTree Tab/Shift+Tab row navigation Adds keyboard row-to-row Tab/Shift+Tab navigation to the Avalonia DataTree (Src/Common/FwAvalonia/Detail/DataTree.cs), replacing legacy WinForms Slice.TabIndex/ContainerControl navigation with Avalonia's native KeyboardNavigation primitives, plus two fixes found by live testing in FieldWorks. DataTree.cs: - KeyboardNavigation.SetTabNavigation(this, Contained) so Tab/Shift+Tab stay inside the detail view instead of leaving it for the hosting WinForms Form. - KeyboardNavigation.SetTabIndex(editor, row) per row, plus the same index on every row's IHoverAffordanceProvider.HoverAffordances (the chooser's configure gear, the reference vector's add button and gear), so Tab reaches a row's affordances right after its value, matching legacy's per-slice multi-stop shape. - KeyboardNavigation.SetIsTabStop(button, false) on the field-menu kebab and the collapsible-header toggle: chrome, not a field, never its own Tab stop, matching legacy. - New CurrentRow/CurrentRowChanged, updated from a bubbling GotFocus handler that also calls BringIntoView() on the focused control -- the Avalonia analog of legacy's CurrentSlice/MakeSliceVisible. InputKeyClaimingAvaloniaHost.cs: - InputKeyClaimPolicy.ShouldClaimKey now also claims Tab (excluding Ctrl+Tab, matching legacy SimpleRootSite.IsInputKey). Without this, WinForms consumed Tab at the host boundary before Avalonia's own navigation ever saw it, so the DataTree.cs wiring above was inert on the real Win32 platform -- found by live testing after the first implementation pass. AvaloniaHostControlBase.cs: - Wraps the embedded WinFormsAvaloniaControlHost's content in one VisualLayerManager, giving the hosted island the AdornerLayer a Window's own template would otherwise supply. Live testing found Tab-navigation focus is invisible on several fields (Grammatical Info, Morph Type, Components, Semantic Domains); this did not turn out to close that gap, but is kept as correct hosting hygiene -- see the DataTree-TabNavigation working docs for the open follow-up. CONTEXT.md: - Adds a CurrentRow glossary entry alongside the existing Detail entry. Proving-it-works (headless navigation tests, integration test plan) has not started yet; the focus-visibility gap above is still open. Co-Authored-By: Claude Sonnet 5 --- CONTEXT.md | 1 + .../FwAvalonia/AvaloniaHostControlBase.cs | 13 ++- Src/Common/FwAvalonia/Detail/DataTree.cs | 89 +++++++++++++++++++ .../InputKeyClaimingAvaloniaHost.cs | 18 ++-- 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index c55b89971c..f707b751fd 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -67,6 +67,7 @@ It is intentionally not a full architecture manual. It should stay biased toward - **Utility**: A user-invoked data maintenance or migration tool (e.g. resetting homographs, removing parser annotations, fixing duplicate analyses). Utilities implement `IUtility` (`FwCoreDlgs`), are registered in `UtilityCatalogInclude.xml` via reflection, and run through `UtilityDlg` (Tools > Utilities menu). - **DistFiles**: Runtime assets copied into outputs or installers. - **Detail**: In the Avalonia migration, the framework-neutral model of an editing view: a flattened, ordered list of typed fields (`DetailModel` / `DetailField`) composed from the view definition and rendered by the Avalonia `DataTree`. A detail model is data; the rendered view is its visible form. It supersedes **Region**, the migration-invented word this vocabulary replaced — do not reintroduce `Region*` names for these concepts. +- **CurrentRow**: The `DetailField` that currently has keyboard focus inside a rendered Avalonia `DataTree` (`Src/Common/FwAvalonia/Detail/DataTree.cs`); analogous to legacy `Slice`'s `CurrentSlice`/`ContainingDataTree.CurrentSlice`, but named for the rendered unit that has focus (a **row**) rather than the model vocabulary (`DetailField`) it is built from. - **View definition**: The typed IR (`ViewDefinitionModel`, a `ViewNode` tree) compiled from the legacy XML Parts/Layout files by `ViewDefinitionCompiler` and cached by `(className, layoutName, layoutType, fingerprint)`. It is the input the composer projects into a `DetailModel`. - **Surface**: RETIRED as project vocabulary — it accumulated conflicting scopes (record UIs only, record UIs plus dialogs, any migratable UI). Say the concrete thing: a **view** (the rendered UI a tool shows for a record — the legacy WinForms `DataTree`/`BrowseViewer` or the Avalonia `DetailHostControl`), a **dialog**, or the **UI framework** (`UIFramework`, selected per tool from the `UIMode` setting via `UIFrameworkResolver` / `UIFrameworkRegistry`). "Surface" survives only in styling vocabulary (the drawn background: `FwSurfaceStyles`, the surface font) and in the linguistic term "surface form". - **Cross-namespace twin**: A deliberate reuse of a legacy type name for its Avalonia counterpart (`FwAvalonia.Detail.DataTree` beside `DetailControls.DataTree`; `SliceFactory`, `MSAGroupBox`). Disambiguate with a `using` alias at the call site; never rename the type to dodge the collision. The twins cover only the half of the legacy responsibility that still fits — the Avalonia `DataTree` renders a prebuilt `DetailModel` and does not compose it. diff --git a/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs b/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs index e6c729fa49..c3303cd2df 100644 --- a/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs +++ b/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Windows.Forms; +using Avalonia.Controls.Primitives; using Avalonia.Win32.Interoperability; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.Common.FwAvalonia.Seams; @@ -22,6 +23,9 @@ public abstract class AvaloniaHostControlBase : System.Windows.Forms.UserControl { /// The Avalonia content host. Protected so derived detail hosts can set content directly. protected readonly WinFormsAvaloniaControlHost Host; + // The one VisualLayerManager for this embedded root, which has no Window chrome to supply + // one. Without it, Control.FocusAdorner has no AdornerLayer to paint into (LT-22688). + private readonly VisualLayerManager _layerManager; private readonly Panel _companionStrip; /// Raised after a hosted detail view reports an edit completed (wired by the derived host). @@ -46,6 +50,8 @@ protected AvaloniaHostControlBase() // deliberate no-op. The Avalonia content still constructs and lays out off-screen. No-op (and // thus identical) on the real Win32 platform. FwAvaloniaPlatform.GuardHeadlessEmbed(Host); + _layerManager = new VisualLayerManager(); + Host.Content = _layerManager; _companionStrip = new Panel { @@ -67,12 +73,13 @@ protected AvaloniaHostControlBase() /// Swaps the hosted Avalonia content and shows the control. protected void SetHostContent(Avalonia.Controls.Control content) { - Host.Content = content; + _layerManager.Child = content; Show(); } /// The current Avalonia content, or null. - protected Avalonia.Controls.Control CurrentContent => Host.Content as Avalonia.Controls.Control; + protected Avalonia.Controls.Control CurrentContent => + _layerManager.Child as Avalonia.Controls.Control; public void SetCompanionControls(IReadOnlyList controls) { @@ -167,7 +174,7 @@ public void ShowContextMenu(IReadOnlyList items, public void ShowMessage(string message) { - Host.Content = new Avalonia.Controls.TextBlock { Text = message ?? string.Empty }; + _layerManager.Child = 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 1efeac6a68..848ef53aa5 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -10,6 +10,7 @@ using Avalonia.Controls; using Avalonia.Layout; using Avalonia.Media; +using Avalonia.VisualTree; using Avalonia.Styling; using SIL.FieldWorks.Common.FwAvalonia; using SIL.FieldWorks.Common.FwAvalonia.Seams; @@ -36,6 +37,13 @@ public sealed class DataTree : UserControl { private readonly IDetailEditContext _editContext; private readonly Action _writingSystemFocused; + private readonly List> _rowControls = new List>(); + // Row index by control, for OnRowGotFocus's focus-to-row lookup (LT-22688). + private readonly Dictionary _controlToRow = new Dictionary(); + // Collapsible section toggles, keyed by field stable id, captured at build + // time: WireCollapsibleHeaders finds them since the header now wraps in + // the field-menu gutter, where the kebab is also a Button. + private readonly Dictionary _collapsibleToggles = new Dictionary(); private readonly Action _labelColumnWidthChanged; private TextBlock _validationBlock; @@ -176,6 +184,26 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, RebuildItems(); + if (i < model.Fields.Count - 1) + { + var rule = new Border { Background = FwAvaloniaDensity.SliceRuleBrush, Height = 1 }; + AutomationProperties.SetAutomationId(rule, $"SliceRule.{i}"); + Grid.SetRow(rule, i * 2 + 1); + // 14.3: the rule underlines the VALUE side only; the label panel stays clean. + Grid.SetColumn(rule, 2); + grid.Children.Add(rule); + _rowControls[i].Add(rule); // collapses with its row + } + } + + for (var r = 0; r < _rowControls.Count; r++) + { + foreach (var control in _rowControls[r]) + _controlToRow[control] = r; + } + + WireCollapsibleHeaders(model.Fields); + // Viewing parity (11.x): the whole detail view scrolls, like legacy DataTree's AutoScroll panel. // Equal row height read-only vs editable (layout parity): the field container is // ALWAYS wrapped in @@ -202,6 +230,11 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, AutomationProperties.SetAutomationId(scroller, "DataTree.Scroll"); Content = scroller; + // Contains Tab/Shift+Tab to this view instead of continuing into the hosting + // WinForms Form (LT-22688). + Avalonia.Input.KeyboardNavigation.SetTabNavigation(this, + Avalonia.Input.KeyboardNavigationMode.Contained); + // Screen-local command shortcuts: // Enter commits (validation-gated), Escape cancels -- handled at the view so they // work @@ -217,6 +250,10 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, if (_editContext != null && _editContext.IsOpen) OnSave(); }, Avalonia.Interactivity.RoutingStrategies.Bubble); + + // Tracks CurrentRow and scrolls newly focused rows into view (LT-22688). + AddHandler(Avalonia.Input.InputElement.GotFocusEvent, OnRowGotFocus, + Avalonia.Interactivity.RoutingStrategies.Bubble); } /// @@ -261,6 +298,29 @@ private void OnViewKeyDown(object sender, Avalonia.Input.KeyEventArgs e) } } + // Keeps CurrentRow in sync with whichever row actually has focus and scrolls it + // into view (LT-22688). + private void OnRowGotFocus(object sender, Avalonia.Input.GotFocusEventArgs e) + { + if (!(e.Source is Control focused)) + return; + + for (var control = focused; control != null; control = control.GetVisualParent() as Control) + { + if (!_controlToRow.TryGetValue(control, out var row)) + continue; + var field = Model.Fields[row]; + if (!ReferenceEquals(CurrentRow, field)) + { + CurrentRow = field; + CurrentRowChanged?.Invoke(this, EventArgs.Empty); + } + break; + } + + focused.BringIntoView(); + } + /// The detail model this view renders. public DetailModel Model { get; } @@ -270,6 +330,15 @@ private void OnViewKeyDown(object sender, Avalonia.Input.KeyEventArgs e) /// public event EventHandler EditCompleted; + /// + /// The row that currently has keyboard focus, or null when focus is elsewhere. Matches + /// legacy Slice's ContainingDataTree.CurrentSlice role in this view (LT-22688). + /// + public DetailField CurrentRow { get; private set; } + + /// Raised when CurrentRow changes. + public event EventHandler CurrentRowChanged; + // 14.4: no Save/Cancel buttons -- the legacy view saves as you go. The footer carries // only the // inline validation messages (a failed autosave is never silent). @@ -416,6 +485,9 @@ private FieldContent AddField(int index, DetailField field) RebuildItems(); }; header = button; + _collapsibleToggles[field.StableId] = button; + // Chrome, not a field, same reasoning as the field-menu kebab (LT-22688). + Avalonia.Input.KeyboardNavigation.SetIsTabStop(button, false); } else { @@ -512,6 +584,14 @@ private FieldContent AddField(int index, DetailField field) ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // 11.17: legacy label tooltips var editor = CreateEditor(field, automationId); editor.Margin = new Thickness(0, 0, 0, FwAvaloniaDensity.FieldSpacing); + Grid.SetRow(editor, row * 2); + Grid.SetColumn(editor, 2); + grid.Children.Add(editor); + _rowControls[row].Add(editor); + // Every reachable control in a row shares its TabIndex, so Avalonia visits them in + // visual order (editor first, then any affordance below) before moving to the next + // row (LT-22688). + Avalonia.Input.KeyboardNavigation.SetTabIndex(editor, row); if (editor is FwReferenceVectorField vector) { _vectors.Add(vector); @@ -530,7 +610,13 @@ private FieldContent AddField(int index, DetailField field) if (labelKebab != null) HoverReveal.Attach(hoverSources, new[] { labelKebab }); if (editor is IHoverAffordanceProvider provider && provider.HoverAffordances.Count > 0) + { HoverReveal.Attach(hoverSources, provider.HoverAffordances); + // Matches legacy's per-slice multi-stop shape: Tab reaches the configure gear + // (and any other affordance) right after the field's own value (LT-22688). + foreach (var affordance in provider.HoverAffordances) + Avalonia.Input.KeyboardNavigation.SetTabIndex(affordance, row); + } return new FieldContent { Content = editor, Label = labelCell }; } @@ -582,6 +668,9 @@ private Control WrapWithFieldMenu(Control inner, DetailField field, string autom if (hasMenu || hasHotlinks) { var button = DetailChrome.CreateKebabButton(); + // Mouse/right-click reachable only, matching legacy's slice menu, which was + // never its own Tab stop (LT-22688). + Avalonia.Input.KeyboardNavigation.SetIsTabStop(button, false); AutomationProperties.SetAutomationId(button, automationId + ".FieldMenu"); AutomationProperties.SetName(button, FwAvaloniaStrings.FieldOptionsMenu); ToolTip.SetTip(button, FwAvaloniaStrings.FieldOptionsMenu); diff --git a/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs b/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs index 9f6d976fdd..64c18a66ae 100644 --- a/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs +++ b/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs @@ -17,8 +17,9 @@ namespace SIL.FieldWorks.Common.FwAvalonia public static class InputKeyClaimPolicy { /// - /// Whether the host claims as an input key: the arrow keys always, Enter - /// only when is set, and never unless the host holds focus. + /// Whether the host claims as an input key: the arrow keys + /// and Tab (excluding Ctrl+Tab) always, Enter only when + /// is set, and never unless the host holds focus. /// public static bool ShouldClaimKey(Keys keyData, bool hostContainsFocus, bool claimEnterKey) { @@ -31,6 +32,9 @@ public static bool ShouldClaimKey(Keys keyData, bool hostContainsFocus, bool cla case 0x25: // Left case 0x27: // Right return true; + // Matches legacy SimpleRootSite.IsInputKey's Ctrl+Tab exclusion (LT-22688). + case 0x09: // Tab + return (keyData & Keys.Control) == Keys.None; case 0x0D: // Enter return claimEnterKey; default: @@ -43,11 +47,11 @@ public static bool ShouldClaimKey(Keys keyData, bool hostContainsFocus, bool cla /// A that claims the keyboard-navigation keys the hosted /// Avalonia control needs, so the WinForms parent (a detail pane, or a modal dialog form) /// does not - /// consume Up/Down/Left/Right -- and, when asked, Enter -- as its own control-navigation / - /// default-button - /// handling before the Avalonia content sees them. Without this, WinForms eats the presses and hosted - /// list/keyboard navigation does nothing. Keys are claimed only while this host holds focus, so they - /// route normally when focus is elsewhere in the parent. + /// consume Up/Down/Left/Right/Tab -- and, when asked, Enter -- as its own + /// control-navigation / default-button + /// handling before the Avalonia content sees them. Without this, WinForms eats the presses + /// and hosted list/keyboard/Tab navigation does nothing. Keys are claimed only while this + /// host holds focus, so they route normally when focus is elsewhere in the parent. /// public class InputKeyClaimingAvaloniaHost : WinFormsAvaloniaControlHost { From c4b6c9dcf33cd119a981ac3bf5445c4db634d2c2 Mon Sep 17 00:00:00 2001 From: Ariel Rorabaugh Date: Wed, 16 Sep 2026 09:28:29 -0400 Subject: [PATCH 2/8] LT-22688: Fix DataTree Tab reachability + CurrentRow tracking Three related but distinct bugs found by live-testing DataTree's Tab/Shift+Tab row navigation, in code that landed as part of b79b8fce5. DataTree.cs: CurrentRow/CurrentRowChanged silently stopped updating after the Form/FormItem rebuild replaced the old upfront Grid constructor's control-to-row population loop with RebuildItems(), without replacing that population step. _rowControls was left declared but never populated; _controlToRow was declared and read by OnRowGotFocus but likewise never populated. Removed the dead _rowControls field and added RegisterRowControl(row, control), called from AddField for a header's container, a field's editor and label cell, and each hover affordance; _controlToRow is cleared at the top of every RebuildItems() call so a rebuild never leaves stale entries. Tab was skipping entire rows for every Button-rooted editor (FwChooserField, FwReferenceVectorField's buttons) while every TextBox-rooted row worked. TabIndex is not an inherited property in Avalonia: AddField set it only on the editor reference it holds, which for a composite control is the outer container -- the real focusable descendant (e.g. FwMultiWsTextField's per-WS TextBox) never received an explicit TabIndex and defaulted to int.MaxValue, sorting after every row that DID get an explicit small TabIndex regardless of visual position. The same gap existed for a section header's hotlink Field Options button, which never received a TabIndex at all. Added ApplyRowTabIndex(root, row), which sets TabIndex on root and every visual descendant, and applied it to both the field editor and the header container. The column-resize GridSplitter had no row of its own and so also defaulted to int.MaxValue, putting it in the Tab sequence out of position. Excluded it from the tab order entirely with KeyboardNavigation.SetIsTabStop(false) -- chrome, not a field, the same treatment already given the field-menu kebab and the collapsible-header toggle. FwFieldControls.cs: A writing-system value needing true per-run fonts (mixed styling within one alternative) renders through CreateValueContentWithFontSwap as a TextBlock display swapped for the real editable TextBox on a pointer press; the TextBox starts invisible and the TextBlock is never Focusable, so Tab had nothing reachable there at all. A plain value never takes this code path, so most rows were unaffected. Added a GotFocus handler on the display TextBlock performing the same swap-and-focus the pointer handler does, and made it Focusable, for the editable case. Proving-it-works (headless navigation tests, integration test plan) has not started yet. Co-Authored-By: Claude Sonnet 5 Change-Id: If5f1f48d3b3a4871b5cc554ff863abcec5c4ed8e --- Src/Common/FwAvalonia/Detail/DataTree.cs | 94 +++++++++++++------ .../FwAvalonia/Detail/FwFieldControls.cs | 17 ++++ 2 files changed, 81 insertions(+), 30 deletions(-) diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 848ef53aa5..8d30f9018c 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.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) @@ -37,8 +37,11 @@ public sealed class DataTree : UserControl { private readonly IDetailEditContext _editContext; private readonly Action _writingSystemFocused; - private readonly List> _rowControls = new List>(); - // Row index by control, for OnRowGotFocus's focus-to-row lookup (LT-22688). + // Row index by control, for OnRowGotFocus's focus-to-row lookup. Repopulated by + // AddField on every RebuildItems() call, which clears it first -- a rebuild + // replaces the Form's Items outright rather than toggling IsVisible on cached + // controls, so a stale entry would otherwise point at a control no longer in + // the visual tree (LT-22688). private readonly Dictionary _controlToRow = new Dictionary(); // Collapsible section toggles, keyed by field stable id, captured at build // time: WireCollapsibleHeaders finds them since the header now wraps in @@ -173,6 +176,12 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, Width = FwAvaloniaDensity.SplitterWidth }; AutomationProperties.SetAutomationId(splitter, "DataTree.Splitter"); + // Chrome, not a field, same reasoning as the field-menu kebab and the + // collapsible-header toggle: a column-resize handle has no row of its own to + // take a TabIndex from, so it was left at Avalonia's default (int.MaxValue) + // and visited out of order; legacy's splitter was never keyboard-focusable + // either (LT-22688). + Avalonia.Input.KeyboardNavigation.SetIsTabStop(splitter, false); Grid.SetColumn(splitter, 1); outerGrid.Children.Add(splitter); // added after the Form so its drag handle stays hit-testable outerGrid.LayoutUpdated += (s, e) => @@ -184,26 +193,6 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, RebuildItems(); - if (i < model.Fields.Count - 1) - { - var rule = new Border { Background = FwAvaloniaDensity.SliceRuleBrush, Height = 1 }; - AutomationProperties.SetAutomationId(rule, $"SliceRule.{i}"); - Grid.SetRow(rule, i * 2 + 1); - // 14.3: the rule underlines the VALUE side only; the label panel stays clean. - Grid.SetColumn(rule, 2); - grid.Children.Add(rule); - _rowControls[i].Add(rule); // collapses with its row - } - } - - for (var r = 0; r < _rowControls.Count; r++) - { - foreach (var control in _rowControls[r]) - _controlToRow[control] = r; - } - - WireCollapsibleHeaders(model.Fields); - // Viewing parity (11.x): the whole detail view scrolls, like legacy DataTree's AutoScroll panel. // Equal row height read-only vs editable (layout parity): the field container is // ALWAYS wrapped in @@ -321,6 +310,30 @@ private void OnRowGotFocus(object sender, Avalonia.Input.GotFocusEventArgs e) focused.BringIntoView(); } + // Records that `control` belongs to `row`, so OnRowGotFocus's ancestor walk can + // resolve focus anywhere in the row back to its DetailField (LT-22688). + private void RegisterRowControl(int row, Control control) + { + if (control != null) + _controlToRow[control] = row; + } + + // Applies `row`'s TabIndex to `root` AND every visual descendant, not just + // `root` itself -- TabIndex does not inherit in Avalonia, so a composite + // control's real focusable parts (a header's hotlink Button, a multi-WS + // field's per-WS TextBox, ...) would otherwise keep Avalonia's default + // TabIndex (int.MaxValue) and sort after every explicitly row-indexed + // control regardless of visual position (LT-22688). + private static void ApplyRowTabIndex(Control root, int row) + { + Avalonia.Input.KeyboardNavigation.SetTabIndex(root, row); + foreach (var descendant in root.GetVisualDescendants()) + { + if (descendant is Control descendantControl) + Avalonia.Input.KeyboardNavigation.SetTabIndex(descendantControl, row); + } + } + /// The detail model this view renders. public DetailModel Model { get; } @@ -389,6 +402,7 @@ private void RebuildItems() { _form.Items.Clear(); _vectors.Clear(); + _controlToRow.Clear(); SelectedVector = null; var visible = DetailVisibility.ComputeVisibility(Model.Fields, GetRecordedExpansion); for (var i = 0; i < Model.Fields.Count; i++) @@ -445,7 +459,7 @@ private struct FieldContent public Control Label; } - private FieldContent AddField(int index, DetailField field) + private FieldContent AddField(int row, DetailField field) { var automationId = string.IsNullOrEmpty(field.AutomationId) ? field.StableId : field.AutomationId; var indent = new Thickness(field.Indent * 12, 0, 0, 0); @@ -521,7 +535,7 @@ private FieldContent AddField(int index, DetailField field) // The header cell and its inline hotlink strip always travel together (the strip is part of // the header row, hidden/shown with it by the collapse logic). Control headerControl; - if (field.Indent == 0 && index > 0) + if (field.Indent == 0 && row > 0) { var withRule = new StackPanel(); withRule.Children.Add(new Border @@ -549,6 +563,11 @@ private FieldContent AddField(int index, DetailField field) if (headerKebab != null) HoverReveal.Attach(new[] { headerCell }, new[] { headerKebab }); + // The hotlink strip's "Field Options" Button never gets a TabIndex any + // other way, so without this it defaults to int.MaxValue and is visited + // dead last, well after every explicitly row-indexed field (LT-22688). + ApplyRowTabIndex(headerControl, row); + RegisterRowControl(row, headerControl); return new FieldContent { Content = headerControl, Label = null }; } @@ -584,14 +603,24 @@ private FieldContent AddField(int index, DetailField field) ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // 11.17: legacy label tooltips var editor = CreateEditor(field, automationId); editor.Margin = new Thickness(0, 0, 0, FwAvaloniaDensity.FieldSpacing); - Grid.SetRow(editor, row * 2); - Grid.SetColumn(editor, 2); - grid.Children.Add(editor); - _rowControls[row].Add(editor); + // Every reachable control in a row shares its TabIndex, so Avalonia visits them in // visual order (editor first, then any affordance below) before moving to the next // row (LT-22688). - Avalonia.Input.KeyboardNavigation.SetTabIndex(editor, row); + // TabIndex is not an inherited property: setting it on `editor` alone does + // nothing for a composite editor's REAL focusable descendants (e.g. + // FwMultiWsTextField's own per-WS TextBox, built directly in its + // constructor) when the composite container itself is Focusable=false. + // Left unset, those descendants sort at Avalonia's default TabIndex + // (int.MaxValue) -- after every row whose editor DID get an explicit, + // small row-number TabIndex -- so Tab could never reach a Button-rooted + // row (FwChooserField, FwReferenceVectorField's buttons) from within a + // TextBox-rooted one: nothing in that tier ever sorts after a lower + // explicit index. Propagating the row's TabIndex to every visual + // descendant closes that gap for any composite editor uniformly + // (confirmed via live-test Debug logging showing TabIndex=2147483647 on + // the TextBox that actually receives focus) (LT-22688). + ApplyRowTabIndex(editor, row); if (editor is FwReferenceVectorField vector) { _vectors.Add(vector); @@ -615,9 +644,14 @@ private FieldContent AddField(int index, DetailField field) // Matches legacy's per-slice multi-stop shape: Tab reaches the configure gear // (and any other affordance) right after the field's own value (LT-22688). foreach (var affordance in provider.HoverAffordances) + { Avalonia.Input.KeyboardNavigation.SetTabIndex(affordance, row); + RegisterRowControl(row, affordance); + } } + RegisterRowControl(row, editor); + RegisterRowControl(row, labelCell); return new FieldContent { Content = editor, Label = labelCell }; } diff --git a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs index ef2502ad37..b41a4553d3 100644 --- a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs +++ b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs @@ -944,6 +944,22 @@ private Control CreateValueContentWithFontSwap(DetailField field, string automat }; display.AddHandler(InputElement.PointerPressedEvent, displayPressed, Avalonia.Interactivity.RoutingStrategies.Tunnel); + // Keyboard equivalent of the pointer-press swap above (LT-22688): `box` + // starts invisible and `display` starts non-focusable (TextBlock's + // default), so Tab correctly treats this row as having nothing reachable + // at all -- a value needing true per-run fonts (mixed styling within one + // alternative) was keyboard-unreachable, while a plain value (which never + // takes this display/box swap at all, see the early return above) worked + // fine. Making `display` a normal tab stop and swapping on GotFocus (not + // just PointerPressed) gives Tab the same entry point a mouse click has. + display.Focusable = true; + EventHandler displayGotFocus = (s, e) => + { + box.IsVisible = true; + display.IsVisible = false; + box.Focus(); + }; + display.GotFocus += displayGotFocus; EventHandler lost = (s, e) => { box.IsVisible = false; @@ -953,6 +969,7 @@ private Control CreateValueContentWithFontSwap(DetailField field, string automat _teardown.Add(() => { display.RemoveHandler(InputElement.PointerPressedEvent, displayPressed); + display.GotFocus -= displayGotFocus; box.LostFocus -= lost; }); } From 7b80df1e1a2d0d6a6a28c01215982ebf8fdebf9c Mon Sep 17 00:00:00 2001 From: Ariel Rorabaugh Date: Wed, 16 Sep 2026 11:00:44 -0400 Subject: [PATCH 3/8] LT-22688: Remove unneeded VisualLayerManager and CurrentRow Live-testing confirmed two pieces added earlier in this ticket were never actually needed, once the real Tab-reachability bug (TabIndex not propagating to composite controls' descendants) was fixed. AvaloniaHostControlBase.cs: The VisualLayerManager wrapping the hosted Avalonia content was added to give Control.FocusAdorner an AdornerLayer to paint into, since the WinFormsAvaloniaControlHost's embedded root supplies none of its own. A developer A/B test (toggling a temporary flag and rebuilding) showed Tab navigation and focus-visual rendering both work correctly with the VisualLayerManager bypassed entirely, so the wrapper, its field, and its wiring in SetHostContent, CurrentContent, and ShowMessage are removed. Host.Content now holds the hosted view directly, the same as before this fix was ever added. DataTree.cs: CurrentRow and CurrentRowChanged were added as the Avalonia replacement for the WinForms DataTree's CurrentSlice and CurrentSliceChanged, which around a dozen slice classes read or set. No Avalonia-side consumer of CurrentRow exists yet, so the property, the event, RegisterRowControl, and the _controlToRow bookkeeping that fed them are removed rather than carried forward unused. OnRowGotFocus keeps its other, unrelated job -- scrolling a newly focused row into view -- now a one-line body. CONTEXT.md's CurrentRow entry is kept, reworded to reserve the name for whenever a real consumer needs it rebuilt, so the naming decision itself is not lost. Comment cleanup: Reworded four comments this ticket's earlier commits added that used the banned word "legacy" (fieldworks-code-commenting), and trimmed three that exceeded the 200-character inline-comment budget -- one of them by converting ApplyRowTabIndex's explanation to an XML doc comment, which is exempt from that cap. A FwFieldControls.cs comment also dropped past-tense bug narrative for a statement of current behavior. Co-Authored-By: Claude Sonnet 5 Change-Id: Icf006618c9eaec2ece2cbfe2fc3919569818dd38 --- CONTEXT.md | 2 +- .../FwAvalonia/AvaloniaHostControlBase.cs | 12 +-- Src/Common/FwAvalonia/Detail/DataTree.cs | 100 ++++-------------- .../FwAvalonia/Detail/FwFieldControls.cs | 11 +- .../InputKeyClaimingAvaloniaHost.cs | 3 +- 5 files changed, 28 insertions(+), 100 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index f707b751fd..d9e31f6c5f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -67,7 +67,7 @@ It is intentionally not a full architecture manual. It should stay biased toward - **Utility**: A user-invoked data maintenance or migration tool (e.g. resetting homographs, removing parser annotations, fixing duplicate analyses). Utilities implement `IUtility` (`FwCoreDlgs`), are registered in `UtilityCatalogInclude.xml` via reflection, and run through `UtilityDlg` (Tools > Utilities menu). - **DistFiles**: Runtime assets copied into outputs or installers. - **Detail**: In the Avalonia migration, the framework-neutral model of an editing view: a flattened, ordered list of typed fields (`DetailModel` / `DetailField`) composed from the view definition and rendered by the Avalonia `DataTree`. A detail model is data; the rendered view is its visible form. It supersedes **Region**, the migration-invented word this vocabulary replaced — do not reintroduce `Region*` names for these concepts. -- **CurrentRow**: The `DetailField` that currently has keyboard focus inside a rendered Avalonia `DataTree` (`Src/Common/FwAvalonia/Detail/DataTree.cs`); analogous to legacy `Slice`'s `CurrentSlice`/`ContainingDataTree.CurrentSlice`, but named for the rendered unit that has focus (a **row**) rather than the model vocabulary (`DetailField`) it is built from. +- **CurrentRow**: Reserved name for a future focus-tracking property/event on the Avalonia `DataTree` (`Src/Common/FwAvalonia/Detail/DataTree.cs`), analogous to the Winforms `Slice`'s `CurrentSlice`/`CurrentSliceChanged`; named for the rendered unit that would hold focus (a **row**) rather than the model vocabulary (`DetailField`) it is built from. Not currently implemented. Use this name, not `CurrentField` or another alternative, whenever a first real consumer needs it built. - **View definition**: The typed IR (`ViewDefinitionModel`, a `ViewNode` tree) compiled from the legacy XML Parts/Layout files by `ViewDefinitionCompiler` and cached by `(className, layoutName, layoutType, fingerprint)`. It is the input the composer projects into a `DetailModel`. - **Surface**: RETIRED as project vocabulary — it accumulated conflicting scopes (record UIs only, record UIs plus dialogs, any migratable UI). Say the concrete thing: a **view** (the rendered UI a tool shows for a record — the legacy WinForms `DataTree`/`BrowseViewer` or the Avalonia `DetailHostControl`), a **dialog**, or the **UI framework** (`UIFramework`, selected per tool from the `UIMode` setting via `UIFrameworkResolver` / `UIFrameworkRegistry`). "Surface" survives only in styling vocabulary (the drawn background: `FwSurfaceStyles`, the surface font) and in the linguistic term "surface form". - **Cross-namespace twin**: A deliberate reuse of a legacy type name for its Avalonia counterpart (`FwAvalonia.Detail.DataTree` beside `DetailControls.DataTree`; `SliceFactory`, `MSAGroupBox`). Disambiguate with a `using` alias at the call site; never rename the type to dodge the collision. The twins cover only the half of the legacy responsibility that still fits — the Avalonia `DataTree` renders a prebuilt `DetailModel` and does not compose it. diff --git a/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs b/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs index c3303cd2df..fcc8b67de6 100644 --- a/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs +++ b/Src/Common/FwAvalonia/AvaloniaHostControlBase.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Windows.Forms; -using Avalonia.Controls.Primitives; using Avalonia.Win32.Interoperability; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.Common.FwAvalonia.Seams; @@ -23,9 +22,6 @@ public abstract class AvaloniaHostControlBase : System.Windows.Forms.UserControl { /// The Avalonia content host. Protected so derived detail hosts can set content directly. protected readonly WinFormsAvaloniaControlHost Host; - // The one VisualLayerManager for this embedded root, which has no Window chrome to supply - // one. Without it, Control.FocusAdorner has no AdornerLayer to paint into (LT-22688). - private readonly VisualLayerManager _layerManager; private readonly Panel _companionStrip; /// Raised after a hosted detail view reports an edit completed (wired by the derived host). @@ -50,8 +46,6 @@ protected AvaloniaHostControlBase() // deliberate no-op. The Avalonia content still constructs and lays out off-screen. No-op (and // thus identical) on the real Win32 platform. FwAvaloniaPlatform.GuardHeadlessEmbed(Host); - _layerManager = new VisualLayerManager(); - Host.Content = _layerManager; _companionStrip = new Panel { @@ -73,13 +67,13 @@ protected AvaloniaHostControlBase() /// Swaps the hosted Avalonia content and shows the control. protected void SetHostContent(Avalonia.Controls.Control content) { - _layerManager.Child = content; + Host.Content = content; Show(); } /// The current Avalonia content, or null. protected Avalonia.Controls.Control CurrentContent => - _layerManager.Child as Avalonia.Controls.Control; + Host.Content as Avalonia.Controls.Control; public void SetCompanionControls(IReadOnlyList controls) { @@ -174,7 +168,7 @@ public void ShowContextMenu(IReadOnlyList items, public void ShowMessage(string message) { - _layerManager.Child = new Avalonia.Controls.TextBlock { Text = message ?? string.Empty }; + Host.Content = 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 8d30f9018c..865fd4b2ec 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -37,12 +37,6 @@ public sealed class DataTree : UserControl { private readonly IDetailEditContext _editContext; private readonly Action _writingSystemFocused; - // Row index by control, for OnRowGotFocus's focus-to-row lookup. Repopulated by - // AddField on every RebuildItems() call, which clears it first -- a rebuild - // replaces the Form's Items outright rather than toggling IsVisible on cached - // controls, so a stale entry would otherwise point at a control no longer in - // the visual tree (LT-22688). - private readonly Dictionary _controlToRow = new Dictionary(); // Collapsible section toggles, keyed by field stable id, captured at build // time: WireCollapsibleHeaders finds them since the header now wraps in // the field-menu gutter, where the kebab is also a Button. @@ -176,11 +170,9 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, Width = FwAvaloniaDensity.SplitterWidth }; AutomationProperties.SetAutomationId(splitter, "DataTree.Splitter"); - // Chrome, not a field, same reasoning as the field-menu kebab and the - // collapsible-header toggle: a column-resize handle has no row of its own to - // take a TabIndex from, so it was left at Avalonia's default (int.MaxValue) - // and visited out of order; legacy's splitter was never keyboard-focusable - // either (LT-22688). + // Chrome, not a field: a column-resize handle has no row of its own for a + // TabIndex, so it defaults to Avalonia's int.MaxValue and sorts out of + // order. Excluded from the tab order entirely (LT-22688). Avalonia.Input.KeyboardNavigation.SetIsTabStop(splitter, false); Grid.SetColumn(splitter, 1); outerGrid.Children.Add(splitter); // added after the Form so its drag handle stays hit-testable @@ -240,7 +232,7 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, OnSave(); }, Avalonia.Interactivity.RoutingStrategies.Bubble); - // Tracks CurrentRow and scrolls newly focused rows into view (LT-22688). + // Scrolls newly focused rows into view (LT-22688). AddHandler(Avalonia.Input.InputElement.GotFocusEvent, OnRowGotFocus, Avalonia.Interactivity.RoutingStrategies.Bubble); } @@ -287,43 +279,20 @@ private void OnViewKeyDown(object sender, Avalonia.Input.KeyEventArgs e) } } - // Keeps CurrentRow in sync with whichever row actually has focus and scrolls it - // into view (LT-22688). + // Scrolls newly focused rows into view (LT-22688). private void OnRowGotFocus(object sender, Avalonia.Input.GotFocusEventArgs e) { - if (!(e.Source is Control focused)) - return; - - for (var control = focused; control != null; control = control.GetVisualParent() as Control) - { - if (!_controlToRow.TryGetValue(control, out var row)) - continue; - var field = Model.Fields[row]; - if (!ReferenceEquals(CurrentRow, field)) - { - CurrentRow = field; - CurrentRowChanged?.Invoke(this, EventArgs.Empty); - } - break; - } - - focused.BringIntoView(); - } - - // Records that `control` belongs to `row`, so OnRowGotFocus's ancestor walk can - // resolve focus anywhere in the row back to its DetailField (LT-22688). - private void RegisterRowControl(int row, Control control) - { - if (control != null) - _controlToRow[control] = row; + (e.Source as Control)?.BringIntoView(); } - // Applies `row`'s TabIndex to `root` AND every visual descendant, not just - // `root` itself -- TabIndex does not inherit in Avalonia, so a composite - // control's real focusable parts (a header's hotlink Button, a multi-WS - // field's per-WS TextBox, ...) would otherwise keep Avalonia's default - // TabIndex (int.MaxValue) and sort after every explicitly row-indexed - // control regardless of visual position (LT-22688). + /// + /// Applies as the TabIndex of and every + /// visual descendant, not just itself -- TabIndex does not + /// inherit in Avalonia, so a composite control's real focusable parts (a header's + /// hotlink button, a multi-writing-system field's per-value text box, ...) would + /// otherwise keep Avalonia's default TabIndex (int.MaxValue) and sort after every + /// explicitly row-indexed control regardless of visual position (LT-22688). + /// private static void ApplyRowTabIndex(Control root, int row) { Avalonia.Input.KeyboardNavigation.SetTabIndex(root, row); @@ -343,15 +312,6 @@ private static void ApplyRowTabIndex(Control root, int row) /// public event EventHandler EditCompleted; - /// - /// The row that currently has keyboard focus, or null when focus is elsewhere. Matches - /// legacy Slice's ContainingDataTree.CurrentSlice role in this view (LT-22688). - /// - public DetailField CurrentRow { get; private set; } - - /// Raised when CurrentRow changes. - public event EventHandler CurrentRowChanged; - // 14.4: no Save/Cancel buttons -- the legacy view saves as you go. The footer carries // only the // inline validation messages (a failed autosave is never silent). @@ -402,7 +362,6 @@ private void RebuildItems() { _form.Items.Clear(); _vectors.Clear(); - _controlToRow.Clear(); SelectedVector = null; var visible = DetailVisibility.ComputeVisibility(Model.Fields, GetRecordedExpansion); for (var i = 0; i < Model.Fields.Count; i++) @@ -563,11 +522,9 @@ private FieldContent AddField(int row, DetailField field) if (headerKebab != null) HoverReveal.Attach(new[] { headerCell }, new[] { headerKebab }); - // The hotlink strip's "Field Options" Button never gets a TabIndex any - // other way, so without this it defaults to int.MaxValue and is visited - // dead last, well after every explicitly row-indexed field (LT-22688). + // The hotlink strip's "Field Options" button gets no TabIndex any other + // way, so it would default to int.MaxValue and sort dead last (LT-22688). ApplyRowTabIndex(headerControl, row); - RegisterRowControl(row, headerControl); return new FieldContent { Content = headerControl, Label = null }; } @@ -607,19 +564,6 @@ private FieldContent AddField(int row, DetailField field) // Every reachable control in a row shares its TabIndex, so Avalonia visits them in // visual order (editor first, then any affordance below) before moving to the next // row (LT-22688). - // TabIndex is not an inherited property: setting it on `editor` alone does - // nothing for a composite editor's REAL focusable descendants (e.g. - // FwMultiWsTextField's own per-WS TextBox, built directly in its - // constructor) when the composite container itself is Focusable=false. - // Left unset, those descendants sort at Avalonia's default TabIndex - // (int.MaxValue) -- after every row whose editor DID get an explicit, - // small row-number TabIndex -- so Tab could never reach a Button-rooted - // row (FwChooserField, FwReferenceVectorField's buttons) from within a - // TextBox-rooted one: nothing in that tier ever sorts after a lower - // explicit index. Propagating the row's TabIndex to every visual - // descendant closes that gap for any composite editor uniformly - // (confirmed via live-test Debug logging showing TabIndex=2147483647 on - // the TextBox that actually receives focus) (LT-22688). ApplyRowTabIndex(editor, row); if (editor is FwReferenceVectorField vector) { @@ -641,17 +585,12 @@ private FieldContent AddField(int row, DetailField field) if (editor is IHoverAffordanceProvider provider && provider.HoverAffordances.Count > 0) { HoverReveal.Attach(hoverSources, provider.HoverAffordances); - // Matches legacy's per-slice multi-stop shape: Tab reaches the configure gear - // (and any other affordance) right after the field's own value (LT-22688). + // Sequences a row's own affordances (the chooser's configure gear, a + // reference vector's add/gear buttons) right after its editor (LT-22688). foreach (var affordance in provider.HoverAffordances) - { Avalonia.Input.KeyboardNavigation.SetTabIndex(affordance, row); - RegisterRowControl(row, affordance); - } } - RegisterRowControl(row, editor); - RegisterRowControl(row, labelCell); return new FieldContent { Content = editor, Label = labelCell }; } @@ -702,8 +641,7 @@ private Control WrapWithFieldMenu(Control inner, DetailField field, string autom if (hasMenu || hasHotlinks) { var button = DetailChrome.CreateKebabButton(); - // Mouse/right-click reachable only, matching legacy's slice menu, which was - // never its own Tab stop (LT-22688). + // Mouse/right-click reachable only; never its own Tab stop (LT-22688). Avalonia.Input.KeyboardNavigation.SetIsTabStop(button, false); AutomationProperties.SetAutomationId(button, automationId + ".FieldMenu"); AutomationProperties.SetName(button, FwAvaloniaStrings.FieldOptionsMenu); diff --git a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs index b41a4553d3..46ff4f0d9b 100644 --- a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs +++ b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs @@ -944,14 +944,9 @@ private Control CreateValueContentWithFontSwap(DetailField field, string automat }; display.AddHandler(InputElement.PointerPressedEvent, displayPressed, Avalonia.Interactivity.RoutingStrategies.Tunnel); - // Keyboard equivalent of the pointer-press swap above (LT-22688): `box` - // starts invisible and `display` starts non-focusable (TextBlock's - // default), so Tab correctly treats this row as having nothing reachable - // at all -- a value needing true per-run fonts (mixed styling within one - // alternative) was keyboard-unreachable, while a plain value (which never - // takes this display/box swap at all, see the early return above) worked - // fine. Making `display` a normal tab stop and swapping on GotFocus (not - // just PointerPressed) gives Tab the same entry point a mouse click has. + // Makes `display` an ordinary tab stop; GotFocus swaps in the editable + // `box` the same way the pointer-press handler above does, matching a + // mouse click's entry point (LT-22688). display.Focusable = true; EventHandler displayGotFocus = (s, e) => { diff --git a/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs b/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs index 64c18a66ae..10ddcbb3c9 100644 --- a/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs +++ b/Src/Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs @@ -32,7 +32,8 @@ public static bool ShouldClaimKey(Keys keyData, bool hostContainsFocus, bool cla case 0x25: // Left case 0x27: // Right return true; - // Matches legacy SimpleRootSite.IsInputKey's Ctrl+Tab exclusion (LT-22688). + // Ctrl+Tab is excluded so the surrounding application keeps it for its + // own tab/window switching (LT-22688). case 0x09: // Tab return (keyData & Keys.Control) == Keys.None; case 0x0D: // Enter From 4650ab9c02d296cac4df5571a5166cc5c4ae4ff1 Mon Sep 17 00:00:00 2001 From: Ariel Rorabaugh Date: Tue, 22 Sep 2026 09:55:52 -0400 Subject: [PATCH 4/8] LT-22688: Update key-claim tests for Tab now being claimed InputKeyClaimingAvaloniaHostTests.cs and DetailHostControlTests.cs both exercise InputKeyClaimPolicy.ShouldClaimKey through two host scenarios (dialog vs. detail pane), and both predate this ticket's fix that added Tab, except Ctrl+Tab, to the claimed keys. Each still asserted the pre-fix behavior that Tab is never claimed. InputKeyClaimingAvaloniaHostTests.cs: removed the Tab case from OtherKeys_AreNeverClaimed and added TabKeys_AreClaimedWhenFocused, covering both the claimed case and the Ctrl+Tab exclusion. DetailHostControlTests.cs: added TabKey_Bypassed_WhenAvaloniaHostContainsFocus alongside the existing directional-key case, and moved the "not bypassed" Tab case in NonDirectionalKeys_AndUnfocusedHost_AreNotBypassed to the unfocused-host branch, where it is still correct. No production code changes here -- the claiming decision itself was already fixed in an earlier commit. Co-Authored-By: Claude Sonnet 5 Change-Id: I4a23298dcb9485fa63cff13e2038e8de12db74e5 --- .../FwAvaloniaTests/DetailHostControlTests.cs | 8 +++++++- .../InputKeyClaimingAvaloniaHostTests.cs | 10 +++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailHostControlTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailHostControlTests.cs index e4bd5f77db..c48d8879ea 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailHostControlTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailHostControlTests.cs @@ -26,12 +26,18 @@ public void DirectionalKeys_AreBypassed_WhenAvaloniaHostContainsFocus() Assert.That(ShouldBypass(true, 0x27), Is.True); } + [Test] + public void TabKey_Bypassed_WhenAvaloniaHostContainsFocus() + { + Assert.That(ShouldBypass(true, 0x09), Is.True); + } + [Test] public void NonDirectionalKeys_AndUnfocusedHost_AreNotBypassed() { Assert.That(ShouldBypass(false, 0x26), Is.False); + Assert.That(ShouldBypass(false, 0x09), Is.False); Assert.That(ShouldBypass(true, 0x0D), Is.False); - Assert.That(ShouldBypass(true, 0x09), Is.False); } } } \ No newline at end of file diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/InputKeyClaimingAvaloniaHostTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/InputKeyClaimingAvaloniaHostTests.cs index ccf00d3bee..325d51a5b3 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/InputKeyClaimingAvaloniaHostTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/InputKeyClaimingAvaloniaHostTests.cs @@ -27,6 +27,15 @@ public void ArrowKeys_AreClaimedWhenFocused(Keys key) Is.True, "arrow keys are always claimed while the host holds focus"); } + [Test] + public void TabKeys_AreClaimedWhenFocused() + { + Assert.That(InputKeyClaimPolicy.ShouldClaimKey(Keys.Tab, hostContainsFocus: true, claimEnterKey: false), + Is.True, "Tab is claimed while the host holds focus, so Avalonia's own navigation sees it"); + Assert.That(InputKeyClaimPolicy.ShouldClaimKey(Keys.Tab | Keys.Control, hostContainsFocus: true, claimEnterKey: false), + Is.False, "Ctrl+Tab is excluded so the surrounding application keeps it for its own tab/window switching"); + } + [TestCase(Keys.Up)] [TestCase(Keys.Down)] [TestCase(Keys.Left)] @@ -47,7 +56,6 @@ public void Enter_IsClaimedOnlyWhenOptedIn() Is.False, "a detail pane host leaves Enter to WinForms"); } - [TestCase(Keys.Tab)] [TestCase(Keys.A)] [TestCase(Keys.Escape)] public void OtherKeys_AreNeverClaimed(Keys key) From 493ded43f5d68c6c4d9ed1a8832d4a22f54bc108 Mon Sep 17 00:00:00 2001 From: Ariel Rorabaugh Date: Tue, 22 Sep 2026 10:44:47 -0400 Subject: [PATCH 5/8] LT-22688: Add headless Tab-navigation integration tests New file DataTreeTabNavigationIntegrationTests.cs: Each test drives Tab and Shift+Tab through the real headless input pipeline rather than calling internal handlers directly, so a regression in Avalonia's own tab-walk or in the host's key-claiming would fail these the same way a live user would notice it. Covers forward and reverse tab order, containment at the first and last row, skipping a collapsed section's rows, the field-menu kebab never being a tab stop, scrolling an offscreen row into view, native focus state at each stop, and a multi-writing-system row's internal Tab stops before advancing to the next row. Built directly on DetailField/DetailModel/DataTree, with a DialogSnapshot capture per test as labeled evidence. Co-Authored-By: Claude Sonnet 5 Change-Id: I039faaa245d2da32179db8500d8fefc814c85e94 --- .../DataTreeTabNavigationIntegrationTests.cs | 272 ++++++++++++++++++ 1 file changed, 272 insertions(+) create mode 100644 Src/Common/FwAvalonia/FwAvaloniaTests/Detail/DataTreeTabNavigationIntegrationTests.cs diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/DataTreeTabNavigationIntegrationTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/DataTreeTabNavigationIntegrationTests.cs new file mode 100644 index 0000000000..f9d8e8152a --- /dev/null +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/Detail/DataTreeTabNavigationIntegrationTests.cs @@ -0,0 +1,272 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Linq; +using Avalonia; +using Avalonia.Automation; +using Avalonia.Controls; +using Avalonia.Headless; +using Avalonia.Headless.NUnit; +using Avalonia.Input; +using Avalonia.Threading; +using Avalonia.VisualTree; +using NUnit.Framework; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; +using FwAvaloniaTests.VisualChecks; // DialogSnapshot -- the PNG harness + +namespace FwAvaloniaTests.Detail +{ + /// + /// Headless proof for LT-22688's DataTree Tab/Shift+Tab row navigation, one test per item of + /// DataTree-TabNavigation-integration-test-plan.md. Every test drives Tab/Shift+Tab through + /// the real headless input pipeline (never by calling internal handlers directly), so a + /// regression in Avalonia's own tab-walk or in the host's key-claiming would fail these the + /// same way a live user would notice it. + /// + [TestFixture] + public class DataTreeTabNavigationIntegrationTests + { + private static DetailField Field(string id, DetailFieldKind kind = DetailFieldKind.Text, + int indent = 0, string menuId = null, bool isCollapsible = false, + bool isInitiallyExpanded = true, IReadOnlyList values = null) + => new DetailField(id, id, id, null, kind, + EditorClassification.Known, id, null, HostRouting.Inherit, + kind == DetailFieldKind.Text + ? values ?? new List { new DetailWsValue("vern", "value") } + : null, + null, null, + isEditable: kind == DetailFieldKind.Text, indent: indent, + isCollapsible: isCollapsible, isInitiallyExpanded: isInitiallyExpanded, + menuId: menuId, objectHvo: 1234); + + // A non-null menuRequested is what makes WrapWithFieldMenu render a field's kebab at + // all (a null host bridge means no menu button to test); the request content itself is + // irrelevant here. + private static (Window Window, DataTree View) Show(double width, double height, + params DetailField[] fields) + { + var model = new DetailModel("LexEntry", "Normal", fields.ToList(), new List()); + var view = new DataTree(model, menuRequested: request => { }); + var window = new Window { Content = view, Width = width, Height = height }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + return (window, view); + } + + private static T Find(Visual root, string automationId) where T : Visual + => root.GetVisualDescendants().OfType() + .First(c => AutomationProperties.GetAutomationId(c) == automationId); + + private static Control FindFocused(Visual root) + => root.GetVisualDescendants().OfType().FirstOrDefault(c => c.IsFocused); + + private static string FocusedAutomationId(Visual root) + { + var focused = FindFocused(root); + return focused == null ? null : AutomationProperties.GetAutomationId(focused); + } + + private static void Tab(Window window) + { + window.KeyPressQwerty(PhysicalKey.Tab, RawInputModifiers.None); + Dispatcher.UIThread.RunJobs(); + } + + private static void ShiftTab(Window window) + { + window.KeyPressQwerty(PhysicalKey.Tab, RawInputModifiers.Shift); + Dispatcher.UIThread.RunJobs(); + } + + [AvaloniaTest] + public void TabOrderForward_VisitsEveryRowInModelOrder_NeverTheKebab() + { + var (window, view) = Show(480, 300, + Field("Row0"), + Field("Row1", menuId: "mnuDataTree-Help"), + Field("Row2"), + Field("Row3")); + + Find(view, "Row0.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + Assert.That(FocusedAutomationId(view), Is.EqualTo("Row0.vern")); + + var visited = new List(); + for (var i = 0; i < 3; i++) + { + Tab(window); + visited.Add(FocusedAutomationId(view)); + } + + Assert.That(visited, Is.EqualTo(new[] { "Row1.vern", "Row2.vern", "Row3.vern" }), + "Tab visits every row once, in model order, and never the kebab"); + DialogSnapshot.Capture(window, "DataTree-TabNavigation-01-tab-order-forward"); + } + + [AvaloniaTest] + public void TabOrderReverse_MirrorsForwardExactly() + { + var (window, view) = Show(480, 300, + Field("Row0"), + Field("Row1", menuId: "mnuDataTree-Help"), + Field("Row2"), + Field("Row3")); + + Find(view, "Row3.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + Assert.That(FocusedAutomationId(view), Is.EqualTo("Row3.vern")); + + var visited = new List(); + for (var i = 0; i < 3; i++) + { + ShiftTab(window); + visited.Add(FocusedAutomationId(view)); + } + + Assert.That(visited, Is.EqualTo(new[] { "Row2.vern", "Row1.vern", "Row0.vern" }), + "Shift+Tab retraces the forward order exactly in reverse"); + DialogSnapshot.Capture(window, "DataTree-TabNavigation-02-tab-order-reverse"); + } + + [AvaloniaTest] + public void ShiftTabAtFirstRow_StaysContained() + { + var (window, view) = Show(480, 300, Field("Row0"), Field("Row1")); + + Find(view, "Row0.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + + ShiftTab(window); + + Assert.That(FocusedAutomationId(view), Is.EqualTo("Row0.vern"), + "Contained navigation keeps Shift+Tab from leaving the view at the first row"); + DialogSnapshot.Capture(window, "DataTree-TabNavigation-03-first-row-contained"); + } + + [AvaloniaTest] + public void TabAtLastRow_StaysContained() + { + var (window, view) = Show(480, 300, Field("Row0"), Field("Row1")); + + Find(view, "Row1.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + + Tab(window); + + Assert.That(FocusedAutomationId(view), Is.EqualTo("Row1.vern"), + "Contained navigation keeps Tab from leaving the view at the last row"); + DialogSnapshot.Capture(window, "DataTree-TabNavigation-04-last-row-contained"); + } + + [AvaloniaTest] + public void Tab_SkipsRowsOwnedByACollapsedHeader() + { + var (window, view) = Show(480, 300, + Field("Before"), + Field("Section", kind: DetailFieldKind.Header, isCollapsible: true, + isInitiallyExpanded: false), + Field("Inner0", indent: 1), + Field("Inner1", indent: 1), + Field("After")); + + Find(view, "Before.vern").Focus(); + Dispatcher.UIThread.RunJobs(); + + Tab(window); + + Assert.That(FocusedAutomationId(view), Is.EqualTo("After.vern"), + "a collapsed section's own header and every row it owns are unreachable by Tab"); + DialogSnapshot.Capture(window, "DataTree-TabNavigation-05-skips-collapsed-rows"); + } + + [AvaloniaTest] + public void FieldMenuKebab_IsNeverATabStop() + { + var (window, view) = Show(480, 300, Field("Row0", menuId: "mnuDataTree-Help")); + + var kebab = Find