Skip to content

LT-22691: Reach reference-vector items from the keyboard - #1160

Open
mark-sil wants to merge 2 commits into
mainfrom
LT-22691i
Open

mark-sil wants to merge 2 commits into
mainfrom
LT-22691i

Conversation

@mark-sil

@mark-sil mark-sil commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Src/Common/FwAvalonia/Detail/FwFieldControls.cs, the FwReferenceVectorField item loop and the key handler right after it. The other two files are tests.

Tab now enters a reference-vector row at its first item, Left/Right and Home/End move between the items, and Ctrl+Left/Right move the current item on a row that can reorder. Until now a mouse click was the only way to make an item current: Tab skipped the items and landed on the hover-hidden "+" launcher, so Backspace/Delete, the context-menu key and Move Left/Right all needed the mouse first. This closes the follow-up agreed on #1133.

The question a reviewer arrives with is whether this collides with #1152, which turned retypable-row items into editors. It does not: the Tab-stop rule is retypable || index == 0, so every editor on a retypable row stays its own stop, and a TextBox marks the arrow keys handled before the row's handler sees them. The question worth your time is the Tab-order contract itself.

Where to look

  • Tab order per row is first item, "+" launcher, gear, next row; Shift+Tab retraces it. Pinned by ReferenceVectorKeyboardTests.
  • Tab always enters at the first item, even when another is current. A deliberate rule, not a gap; see the accordion.
  • The reorder chord goes through the same TryMoveReferenceItem seam and immediate commit the item menu uses; read-only and non-reorderable rows stay inert.
  • The arrows resolve the physical key through the row's FlowDirection, so a mirrored row still moves toward the side the key names.
  • Only a focused item answers the keys; the launcher and gear keep theirs.

Deliberately not here

  • No remembered position across leaving a row, and no Enter activation on an item.
  • Keyboard reach on retypable-row editors: the editor owns its arrows, so reorder there is menu-only for now. Tracked as an LT-22691 follow-up.
  • The Environments row's item menu is still empty, and entering the pane from WinForms costs an extra Tab. Both are separate items.

Verification. Merges into main on top of #1152. Build with the comment and token hygiene gates; the keyboard, menu-request, tab-navigation, focus-memory and retypable-item fixtures, 90 tests. Manual pass in Sena 3: Tab entry, arrows, Home/End, Ctrl+Right on a subentry with focus following it, Backspace, Shift+F10, and the Environments row. Not run: the full FwAvaloniaTests suite.

Next: approve, or say whether Tab should remember the last item instead of entering at the first.


Reading this a year from now -- start here

The reasoning for this branch was worked out in conversation and in the LT-22691 working notes rather than in documents on the branch, so it is recorded here. The code says what the row does; these sections say why it does it that way and what was rejected.

Decisions, and why

One Tab stop per row, always the first item. The WinForms VectorReferenceLauncher destroys the view's selection in OnLeave, and tabbing back in selects nothing, so there was no precedent for a remembered position. A row revisited minutes or hours later would land somewhere the user no longer expects. One rule with no exceptions is easier to predict, and it is the smallest code: the first chip is marked the stop once, at construction. Provisional pending wider use.

The stop is a chip, not a focusable row. A focusable container that forwards focus to an item makes Shift+Tab from the first item land on the container, which forwards straight back to the item.

Left/Right navigate everywhere; Ctrl+Left/Right reorder. In WinForms, VectorReferenceView.HandleKeyDown makes the bare arrows reorder on reorderable rows and never navigate, PossibilityVectorReferenceView blanks that override so the arrows navigate, and collection rows swallow the arrows. One meaning for the bare arrows, with reorder on a chord, keeps the reorder capability without the inconsistency.

Physical key resolved through FlowDirection. No row is mirrored today and neither WinForms path is direction-aware, but the text-box arrow code in the same file resolves the physical side against RightToLeft, and the guard is one line.

No Enter activation. Ctrl+Click's default jump has no WinForms keyboard equivalent, and Enter already means commit at the view level.

Paths not taken
  • Remembering the current item as the Tab stop. Built first: the stop followed the selection. Reverted before review for the reasons above. Invisible in the squash.
  • A focusable row that forwards focus. Rejected for the Shift+Tab loop.
  • Bare-arrow reorder on reorderable rows (WinForms parity). Rejected because the same keys navigate on possibility rows.
  • An end-to-end headless test that focus follows a moved item across the re-show. The re-show runs in DetailHostControl, a WinForms host no headless test drives; a stand-in would only re-test DetailFocusMemory. Verified manually instead.
Deferred, and what would unblock it
  • Keyboard reach on retypable-row editors (after LT-22672 Allomorphs - Environments inline editing #1152). The TextBox handles Left/Right/Home/End and Ctrl+arrows itself, so item-to-item movement there is Tab and reorder is menu-only. Unblocked by choosing a chord the editor does not consume, if one is wanted.
  • The Environments row's item menu. Right-click and Shift+F10 raise the request, but every leaf of the reference-choices menu is a lexical jump that does not apply to a phonological environment, so the list is empty. Needs a class-specific rule in the reference-choices menu work.
  • The extra Tab to enter the pane from WinForms. The host wrapper is its own WinForms tab stop with nothing focused inside. It disappears when the surrounding shell is Avalonia.
Evidence
  • test.ps1 -CommentHygiene -TokenHygiene -SkipNative -TestProject Src/Common/FwAvalonia/FwAvaloniaTests -TestFilter <ReferenceVectorKeyboardTests | DetailMenuRequestTests | DataTreeTabNavigationIntegrationTests | DetailFocusMemoryTests | RetypableVectorItemTests> on the rebased branch: both gates clean, build succeeded, 90 passed, 0 failed.
  • Earlier runs on the pre-rebase tree added DetailEditingTests, HoverRevealTests and XmlLayoutImporterMenuBindingTests, all green.
  • A dry merge (git merge-tree) against LT-22672 Allomorphs - Environments inline editing #1152 before it landed showed exactly one conflicting line, the Tab-stop assignment, resolved as retypable || index == 0. The item lookup in the key handler is by reference, so the list's element type changing under LT-22672 Allomorphs - Environments inline editing #1152 needed no other change.
  • The first CI run failed one test, the dispose-parity handler count for a read-only row (it expected one handler; the row now also wires its key handler). Fixed in 8b77496; no product code changed.
  • Manual pass in Sena 3, entry ekh for the Environments row: Tab entry and the launcher round trip, arrows holding at the ends, Home/End reaching the chips through the WinForms host, Ctrl+Right moving a subentry with focus following it after the re-show, Ctrl+arrows inert on a non-reorderable row, Backspace removal, Shift+F10 opening the item menu under the chip.
Preflight review details

Code Review Summary

Branch: LT-22691i
Base: main
Date: 2026-09-25
Review model: Claude Fable 5.1
Files changed: 3

Overview

Keyboard reach of reference-vector items in the Avalonia detail view (the
DataTree rendering of a DetailModel). Until now a mouse click was the only
way to make an item of a reference-vector row current: Tab skipped the items and
landed on the row's hover-hidden "+" launcher, so Backspace/Delete, the
context-menu key and Move Left/Right all needed the mouse first. The WinForms
VectorReferenceView is incomplete in its own way (tabbing in selects nothing;
its bare arrows reorder on reorderable rows and navigate on possibility rows),
so the target is better than it, not parity.

The branch makes a row's items exactly one Tab stop (always the first item),
adds Left/Right, Home/End and Ctrl+Left/Right handling on a focused item, and
resolves the physical key through the row's FlowDirection. The analysis found
no Critical or Important issues; the minor findings were fixed or deliberately
routed to manual testing during the author's own review pass, and the manual
pass in Sena 3 was completed by the author.

Contract/API Changes

None. FwReferenceVectorField's public surface is unchanged; only the chips'
IsTabStop values and a new private key handler differ.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

  • Left/Right mapped to index, not the visual side, under a mirrored row
    (fixed during review: the arrows and the reorder chord resolve the physical
    key through the row's FlowDirection; mirrored-row test added)
  • e.Source as TextBlock would silently disable the keys on TextBox
    items after LT-22672 Allomorphs - Environments inline editing #1152
    (fixed during review: the pressed item is found by
    reference with FindIndex; the LT-22672 Allomorphs - Environments inline editing #1152 merge then needed only the one-line
    retypable || index == 0 resolution)
  • Four switch arms repeated FocusItemAt plus e.Handled (fixed during
    review: one target index, one FocusItemAt, one Handled)
  • Read-only rows' Ctrl+arrow path untested (fixed during review: test
    added)
  • Remembered Tab position lost when another vector row is entered
    (resolved by decision: Tab always enters a row at its first item; the WinForms
    launcher keeps no selection across focus loss, so there was no precedent;
    provisional pending wider use)
  • No headless test carries focus to the moved item across a re-show
    (the re-show runs in DetailHostControl, a WinForms host no headless test
    drives; a stand-in would only re-test DetailFocusMemory; covered by the
    author's manual pass: focus followed the moved item)
  • Home/End are not claimed by the WinForms host key policy (verified
    in the product by the author: Home and End reach the chips)

Required Validation / Evidence

  • . est.ps1 -CommentHygiene -TokenHygiene -SkipNative -TestProject Src/Common/FwAvalonia/FwAvaloniaTests -TestFilter <keyboard, menu-request, tab-navigation, focus-memory, retypable-item fixtures> after the rebase onto
    the merged LT-22672 Allomorphs - Environments inline editing #1152: both hygiene gates clean, build succeeded, 90 tests passed.
  • Earlier runs on the pre-rebase tree: the same fixtures plus
    DetailEditingTests, HoverRevealTests and
    XmlLayoutImporterMenuBindingTests, all green.
  • Manual pass by the author in Sena 3 (2026-09-24): Tab entry to a vector row,
    Tab on to the launcher and back, arrows and Home/End, Ctrl+Right/Left on a
    subentry with focus following the moved item, Ctrl+arrows inert on a
    non-reorderable row, Shift+F10 and Backspace on a focused chip, and the
    Environments row (chips, launcher, create-on-type picker).
  • Not run: the full FwAvaloniaTests suite; native tests (no native change).

Developer-only ticket question: not applicable, the change is user-visible and
carries LT-22691.

Positive Observations

  • Removes a hard-coded "never a tab stop" and avoids a selection-dependent Tab
    rule that would have needed maintaining on every selection change.
  • Every behaviour claim has a headless test driving real key presses through
    the input pipeline, in the style of the LT-22688 tab-navigation tests.
  • The by-reference item lookup made the LT-22672 Allomorphs - Environments inline editing #1152 merge a one-line resolution.

Interview Notes

  • Title kept as the commit title: "LT-22691: Reach reference-vector items from
    the keyboard".
  • Reviewer scope note: skipped (small PR).
  • Nothing else to flag. Context a reviewer may want: the always-first Tab rule
    is provisional pending wider use; the Environments row's item menu is still
    absent, which is a separate, already-tracked item and not changed here.
  • Decisions recorded during the author's review: base on main and absorb the
    LT-22672 Allomorphs - Environments inline editing #1152 conflict (done); one Tab stop per row; Left/Right navigate, Home/End
    jump, Ctrl+Left/Right reorder; no Enter activation; the extra Tab needed to
    enter the pane from WinForms is a host-level artifact of the mixed period and
    is not this branch's to fix.

In-Review Quality Check

Fixes above were made before the final gate run; the post-rebase run listed
under validation covers all of them.

Suggested Review Focus

  • FwReferenceVectorField item loop: IsTabStop is retypable || index == 0 after the LT-22672 Allomorphs - Environments inline editing #1152 merge.
  • The row-level KeyDown handler: chord first, then plain-modifier gate,
    then one target; only a focused item answers.
  • ReferenceVectorKeyboardTests: the Tab-order expectations encode the
    always-first rule.

🤖 Generated with Claude Code


This change is Reviewable

Until now a mouse click was the only way to make a reference-vector item
current in the Avalonia detail view: Tab skipped the items and landed on
the row's hover-hidden "+" launcher, so Backspace/Delete, the context-menu
key and Move Left/Right all needed the mouse first. The WinForms
VectorReferenceView is incomplete in its own way (tabbing in selects
nothing, and its bare arrows reorder on reorderable rows while they
navigate on possibility rows), so the target is better than it, not parity.

A row's items now cost exactly one Tab stop, always the first item: the
WinForms launcher drops its selection whenever focus leaves the row, so a
remembered position has no precedent, and one rule with no exceptions is
the easier one to predict. The "+" launcher and configure gear stay their
own stops after the items. From a focused item, Left/Right step between
items, Home/End jump to the ends, and Ctrl+Left/Right move the item on a
row that can reorder, through the same edit-context move and immediate
commit the item menu uses. The arrows and the chord follow the visual side
on a mirrored row. Enter does nothing on an item, by decision.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   12m 45s ⏱️ + 4m 14s
6 278 tests +12  6 193 ✅ +12  85 💤 ±0  0 ❌ ±0 
6 287 runs  +12  6 202 ✅ +12  85 💤 ±0  0 ❌ ±0 

Results for commit 8b77496. ± Comparison against base commit 9607242.

This pull request removes 1 and adds 13 tests. Note that renamed tests count towards both.
FwAvaloniaTests.DetailMenuRequestTests ‑ VectorItems_AreFocusable_ButNotTabStops
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ ArrowKeys_OnTheLauncher_LeaveTheItemsAlone
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ CtrlLeftAndRight_DoNothing_OnAReadOnlyRow_EvenOneThatCouldReorder
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ CtrlLeftAndRight_DoNothing_OnARowThatCannotReorder
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ CtrlLeftAndRight_MoveTheCurrentItem_ThroughTheEditContext_AndCommit
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ HomeAndEnd_JumpToTheFirstAndLastItem
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ LeftAndRight_FollowTheVisualSide_OnAMirroredRow
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ LeftAndRight_StepBetweenItems_AndHoldAtTheEnds
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ ShiftTab_RetracesTheRowExactly
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ Tab_EntersAReadOnlyVectorRow_AtItsFirstItem_AndLeavesStraightToTheNextRow
FwAvaloniaTests.Detail.ReferenceVectorKeyboardTests ‑ Tab_EntersAVectorRow_AtItsFirstItem_EvenWhenAnotherItemIsCurrent
…

♻️ This comment has been updated with latest results.

@codecov-commenter

codecov-commenter commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 38.99%. Comparing base (9607242) to head (8b77496).

Files with missing lines Patch % Lines
Src/Common/FwAvalonia/Detail/FwFieldControls.cs 97.22% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1160   +/-   ##
=======================================
  Coverage   38.99%   38.99%           
=======================================
  Files        1520     1520           
  Lines      352711   352746   +35     
  Branches    40681    40693   +12     
=======================================
+ Hits       137532   137568   +36     
+ Misses     185883   185882    -1     
  Partials    29296    29296           
Files with missing lines Coverage Δ
Src/Common/FwAvalonia/Detail/FwFieldControls.cs 83.95% <97.22%> (+0.49%) ⬆️
🚀 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.

The read-only reference-vector row now wires two teardown entries: the
item's select handler and the row's item-key handler, which every row
gets so keyboard navigation works on read-only rows too. The handler
count test still asserted one and failed #1160's first CI run.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@thejambi thejambi 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.

:lgtm:

@thejambi made 1 comment.
Reviewable status: 0 of 4 files reviewed, all discussions resolved.

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