Lt-22688 Avalonia tab navigation - #1150
Conversation
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 <noreply@anthropic.com>
Three related but distinct bugs found by live-testing DataTree's Tab/Shift+Tab row navigation, in code that landed as part of b79b8fc. 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 <noreply@anthropic.com> Change-Id: If5f1f48d3b3a4871b5cc554ff863abcec5c4ed8e
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 <noreply@anthropic.com> Change-Id: Icf006618c9eaec2ece2cbfe2fc3919569818dd38
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 <noreply@anthropic.com> Change-Id: I4a23298dcb9485fa63cff13e2038e8de12db74e5
Change-Id: I261858fbdde8f0c6a17c2d7c836562c94299eb79
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 <noreply@anthropic.com> Change-Id: I039faaa245d2da32179db8500d8fefc814c85e94
Comment hygiene (advisory)No comment-style violations in the lines this branch adds since |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1150 +/- ##
==========================================
- Coverage 38.89% 38.88% -0.01%
==========================================
Files 1518 1518
Lines 352262 352309 +47
Branches 40597 40601 +4
==========================================
+ Hits 137005 137009 +4
- Misses 185976 186018 +42
- Partials 29281 29282 +1
🚀 New features to boost your workflow:
|
NUnit Tests 1 files ± 0 1 suites ±0 12m 32s ⏱️ +9s Results for commit 874e84a. ± Comparison against base commit 2f1d9db. This pull request removes 1 and adds 12 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
A structured-text paragraph with more than one run (e.g. two different fonts) renders a read-along per-run-font TextBlock and keeps its editable TextBox collapsed until clicked. Neither control was an eligible tab stop, so Tab skipped the whole row. FwStructuredTextField.cs, CreateValueContentWithFontSwap: ported the Focusable=true plus GotFocus swap-in wiring that FwMultiWsTextField's copy of this method already had, so focusing the display swaps in the box the same way a click does. FwStructuredTextField.cs, CreateIconButton: the Add/Delete paragraph buttons were the only per-row triggers missing Focusable=false, so once the row became reachable, Tab landed on Add instead of the editor. Now consistent with the paragraph-style and char-style/writing-system pickers. DataTreeTabNavigationIntegrationTests.cs: added TabIntoARichStructuredTextParagraph_FocusesItsEditor, with a ShowWithEditContext helper alongside Show(), since a structured- text row only wires up paragraph editing (and so this swap) with a non-null edit context. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Change-Id: I1789449c296b6de1acae59bf5ea5652bc3e9a20e
DataTree.cs's _collapsibleToggles dictionary was written once per collapsible header button and never read anywhere in the codebase. Its own comment named a WireCollapsibleHeaders method that was never written -- referencing code that does not exist. Collapsible headers stay mouse/right-click-only by design, matching the WinForms DataTree, so there is no keyboard consumer to build for this registry either. Removed the field, its stale comment, and its one write site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Change-Id: I33efb2faed287796eeaf7d3a2e1e1184c5107f3c
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Change-Id: Ia3079cd5926d6c0065ce265f38ef0cd9e7ebfe74
|
Ignored one potential bug flagged by Devin.
This is not a bug. We want tab stop to be false for collapsible headers and field menus, and for these to be removed from keyboard navigation, because this matches the old Winforms UI behavior |
mark-sil
left a comment
There was a problem hiding this comment.
@mark-sil reviewed 9 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on aror92).
Add keyboard row-to-row Tab/Shift+Tab navigation to the Avalonia
DataTree (Src/Common/FwAvalonia/Detail/DataTree.cs), replacing
WinForms Slice.TabIndex/ContainerControl navigation with Avalonia's
native KeyboardNavigation primitives.
Update InputKeyClaimingAvaloniaHotTests and DetailHostControlTests.
These tests included explicit checks that Tab key was not claimed.
Updated to check that Tab key is claimed, as we need for tab navigation.
Add headless Tab-navigation integration tests.
Covers forward and reverse tab order, containment at first and last row,
field-menu kebab never being a tab stop, scrolling an offscreen row into view,
and native focus at each stop.
IMPORTANT NOTE: two differences from legacy Winforms UI behavior
tab navigation is contained within the first and last row.
Versus in Winforms UI: tab navigation would continue up to the next layer
(e.g. after traversing through all of a lex entry's details, it would move to
traversing through the list of lex entries.)
This is a temporary divergence that we intend to fix toward the end of
Avalonia migration; needed for now because we do not want to pass
tab navigation back and forth between Avalonia and Winforms objects.
tab navigation stops at Field Options button
This is a divergence simply because the Field Options button did not
exist in Winforms UI. If we end up not keeping the button, this divergence
will go away. If we do keep the button, we need a tab stop there.
This change is