Skip to content

Lt-22688 Avalonia tab navigation - #1150

Merged
aror92 merged 9 commits into
mainfrom
LT-22688-avalonia-keyboard-nav
Sep 22, 2026
Merged

aror92 merged 9 commits into
mainfrom
LT-22688-avalonia-keyboard-nav

Conversation

@aror92

@aror92 aror92 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor
  • 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

  1. 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.

  2. 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 Reviewable

aror92 and others added 6 commits September 15, 2026 09:55
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
@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

Comment hygiene (advisory)

No comment-style violations in the lines this branch adds since origin/main.

@codecov-commenter

codecov-commenter commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.88%. Comparing base (2f1d9db) to head (874e84a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
Src/Common/FwAvalonia/Detail/FwFieldControls.cs 33.33% 6 Missing ⚠️
Src/Common/FwAvalonia/Detail/DataTree.cs 96.00% 0 Missing and 1 partial ⚠️
.../Common/FwAvalonia/Detail/FwStructuredTextField.cs 92.85% 1 Missing ⚠️
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     
Files with missing lines Coverage Δ
Src/Common/FwAvalonia/AvaloniaHostControlBase.cs 41.41% <100.00%> (ø)
.../Common/FwAvalonia/InputKeyClaimingAvaloniaHost.cs 42.85% <100.00%> (+1.68%) ⬆️
Src/Common/FwAvalonia/Detail/DataTree.cs 96.30% <96.00%> (-0.03%) ⬇️
.../Common/FwAvalonia/Detail/FwStructuredTextField.cs 79.05% <92.85%> (+0.66%) ⬆️
Src/Common/FwAvalonia/Detail/FwFieldControls.cs 83.67% <33.33%> (+0.03%) ⬆️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   12m 32s ⏱️ +9s
6 208 tests +11  6 123 ✅ +11  85 💤 ±0  0 ❌ ±0 
6 217 runs  +11  6 132 ✅ +11  85 💤 ±0  0 ❌ ±0 

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.
FwAvaloniaTests.InputKeyClaimingAvaloniaHostTests ‑ OtherKeys_AreNeverClaimed(Tab)
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ FieldMenuKebab_IsNeverATabStop
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ ShiftTabAtFirstRow_StaysContained
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabAtLastRow_StaysContained
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabIntoAMultiWsRow_VisitsBothWritingSystemsBeforeAdvancing
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabIntoARichStructuredTextParagraph_FocusesItsEditor
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabOrderForward_VisitsEveryRowInModelOrder_NeverTheKebab
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabOrderReverse_MirrorsForwardExactly
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabThroughEveryRow_ReportsNativeFocusAtEachStop
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ TabToAnOffscreenRow_ScrollsItIntoView
FwAvaloniaTests.Detail.DataTreeTabNavigationIntegrationTests ‑ Tab_SkipsRowsOwnedByACollapsedHeader
…

♻️ This comment has been updated with latest results.

aror92 and others added 3 commits September 22, 2026 11:59
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
@aror92

aror92 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Ignored one potential bug flagged by Devin.

Row commands lose keyboard access
SetIsTabStop(false) removes collapsible headers and field menus from navigation. Keyboard users cannot expand sections or invoke those row commands.

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

@aror92
aror92 marked this pull request as ready for review September 22, 2026 16:30

@mark-sil mark-sil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mark-sil reviewed 9 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on aror92).

@aror92
aror92 merged commit 23965cd into main Sep 22, 2026
9 checks passed
@aror92
aror92 deleted the LT-22688-avalonia-keyboard-nav branch September 22, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants