Skip to content

LT-22691: Answer the item menus of the Lexicon detail view natively - #1207

Merged
mark-sil merged 1 commit into
mainfrom
LT-22691r
Oct 27, 2026
Merged

mark-sil merged 1 commit into
mainfrom
LT-22691r

Conversation

@mark-sil

@mark-sil mark-sil commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs -- the four new answerers (Move Up/Down, Insert Sound or Movie, Picture Properties) and the CommandActsOn guard, at the end of the file.

In Lexicon Edit's Avalonia detail view, the row menus of an etymology, an extended note, a note's examples, an example, a pronunciation, Variant Info and a picture now display and run without the hidden WinForms DataTree. Seven ids switch over (PinnedOwnedIds 41 -> 48). Two shared rules carry the logic, and the legacy handlers now call them: DetailSequenceMove (Move, with the EntryRefs complex-form slot) and MediaFileInsertion (Insert Sound or Movie).

Does legacy behave differently now? Only the undo text: "Undo Create Pronuciation" is now spelled correctly and comes from xWorksStrings. Move enablement, the EntryRefs rule and media link order are unchanged. Legacy appends each pick at the old end of the list, so names land reversed, and the file dialog lists the last pick first.

mnuDataTree-Picture has no Avalonia row yet. The composer never loads GeneratedParts.xml, which holds LexSense-Detail-Pictures (LT-22772). The id is owned and proved on a built Caption row against the hidden tree, which does render pictures. The tripwire LexiconMenuAuthorityTests.NoAvaloniaRowBindsThePictureMenu_UntilPicturesCompose fails once picture rows compose. Its message lists the clean-up for whoever fixes LT-22772, and every workaround comment is tagged LT-22772.

Where to look

  • DetailSequenceMove.CanMoveUp against the old DTMenuHandler display rule -- DetailSequenceMoveTests.
  • CommandActsOn hiding Delete/Move on the owner rows -- DeleteLeaf_IsAbsentWhereTheRowsObjectIsTheOwner, the equivalence contract.
  • One Undo for a new pronunciation plus its links -- InsertSoundOrMovie_OnAnEntryWithoutPronunciations_....

Deliberately not here

  • LT-22772 itself, and the CmMedia rows (LT-22850).
  • E6 merge, E10 delete-reference, E12 lexical relations: D2.
  • mnuDataTree-Sense (demote, promote, split): PR E.

Verification. Merges into main, no dependency. build.ps1 -CommentHygiene -TokenHygiene is clean. Targeted runs passed: 56/56 for the authority, execution and media fixtures, 157/157 for the neighbouring menu, integrity and focus fixtures, and the new rule fixtures. A 17-step manual pass in Sena 3 passed in both UIs.

Next: approve, or flag a divergence below to revisit.


Reading this a year from now -- start here

Stage 2 PR D1 of LT-22691, which retires the hidden WinForms DataTree command adapter from the Avalonia detail view one context-menu id at a time. The predicate-owned LexiconMenuAuthority owns an id once every leaf under it has a native answerer, and LexiconMenuAuthorityTests.PinnedOwnedIds pins the exact owned list. This PR adds four answerers, and the seven ids they complete switch over. No working documents were kept on the branch; the reasoning is below.

Deliberate rules
  • EntryRefs first slot. While an entry's first LexEntryRef is a complex form, the second ref cannot move up. The rule covers Move Up only, as DTMenuHandler's display rule always has. It lives in DetailSequenceMove.CanMoveUp, which the legacy handler now calls.
  • Item commands act only on the item. RecordEditView.CommandActsOn passes when the command's className is the row object's class, or names the owner whose field holds the object (the picture commands name LexSense/Pictures). A command with no className, or one naming a class the model lacks (CmdDataTree-Delete-Subentry misspells ReversalIndexEntry), acts on any object. Delete and Move use it.
  • Media link order. Each new link goes at the position where the links ended before the insert, so the picked names land in reverse. The file dialog lists the last pick first, so picks end up in pick order, as legacy always did.
  • One Undo step natively. On an entry without a pronunciation, the New UI creates the pronunciation only once files are picked, and the pronunciation and its links undo together.
  • Focus after Move goes to the moved object's first editor row (FocusObjectOnNextShow). Row automation ids are position-based (item0, item1), so a row request would land on the neighbour.
  • New strings in xWorksStrings: ksUndoUpdatePicture, ksRedoUpdatePicture, ksUndoCreatePronunciation, ksRedoCreatePronunciation.
Avalonia vs legacy divergences
Where Legacy New UI
"Pronunciations" section header (object: the entry) No such row Insert Pronunciation and Insert Sound or Movie only; no Delete/Move
A note's "Examples" header (object: the note) No such row Insert Example in Note only
An empty note's "Example" prompt row Ghost slice also shows Move Example Up/Down, which move the note Insert Example in Note only
After Move Up/Down The clicked slice stays current Focus goes to the moved object's first editor row
Insert Sound or Movie, entry without a pronunciation Pronunciation created before the dialog; Cancel leaves it; two Undo steps Created only once files are picked; one Undo step

The header rows exist only in Avalonia. #1201 removed the banners over senses, etymologies and extended notes; these two remain. If they go too, three tests that look the banner up fail on purpose and need a small update: MoveLeaves_FollowThePosition_AndAreAbsentOffAnItemsOwnRows, DeleteLeaf_IsAbsentWhereTheRowsObjectIsTheOwner, and the equivalence contract's RowIsTheSequencesOwner case.

The LT-22772 picture-row workaround, and its clean-up
  • Sense pictures do not compose in the Avalonia view because DetailComposer loads only Language Explorer/Configuration/Parts, while LexSense-Detail-Pictures is in DistFiles/Parts/GeneratedParts.xml (LT-22772, also LT-22856).
  • -Picture is owned now so that nothing else changes when that is fixed. PictureMenu_IsOfferedForAPicture_AndRendersWhatTheAdapterPathRendered checks it against the hidden tree on a built Caption row (PictureRow()), and PictureProperties_PointsThePictureAtTheFileTheUserPicks_InOneUndoableStep runs it end to end.
  • NoAvaloniaRowBindsThePictureMenu_UntilPicturesCompose fails once the composed entry has a row bound to mnuDataTree-Picture. Its message lists the clean-up: add -Picture to LexiconLabelMenuIds; replace PictureRow() and the built row in LexiconMenuExecutionTests; delete the comments tagged LT-22772 in both files; delete the test.
Decisions, and why
  • E3 move-in-sequence ships here, not in D2. Every D1 id has Move Up/Down, so D1 is E3's first consumer. D2 keeps E6's merge half, E10 and E12.
  • Own -Picture now rather than defer it. The answerer is small, and owning it means the LT-22772 fix needs no menu work. The tripwire keeps the workaround from outliving its reason.
  • Reuse the WinForms dialogs through seams. PicturePropertiesDialog and the media file dialog with MoveOrCopyFilesController are reached through PictureFileChosen and MediaFilesToInsert, as DeleteConfirmation and ConversionLossAccepted are, so every native command runs headlessly in tests.
  • The class guard rather than per-row special cases. One rule keyed on the command's own parameters covers every owner row, today's and any later banner.
Paths not taken
  • Appending media links in name order. It looked like a fix to legacy's fixed-index insert, but that index is what keeps picks in pick order, because the dialog lists the last pick first. The rule keeps the old order.
  • Fixing LT-22772 here. It changes what the composer loads for every entry and has its own tickets. Kept out of scope.
  • Focusing the clicked row after Move. Row ids are position-based, so the request would land on the row that took the moved object's place.
Evidence
Claim Test
The owned list grows by exactly the seven ids OwnsExactlyThePinnedIds_AndAnswersEveryLeafOfEachOne
Owned menus render what the adapter path rendered, divergences named OwnedMenu_RendersWhatTheAdapterPathRendered_OnEveryLexiconRowThatBindsOne
No owned menu asks the mediator or builds the hidden tree (35 rows) OwnedMenus_AreBuiltWithoutTheAdapterOrTheMediator
Move rule: ends, EntryRefs slot, one Undo DetailSequenceMoveTests (5), VariantInfoMoveUp_IsDisabled_BehindTheComplexForm, MoveVariantInfoUp_MovesTheRef_NeverPastTheComplexForm
Media rule: target, created pronunciation, order, one Undo MediaFileInsertionTests (6), both InsertSoundOrMovie_*
Picture Properties PictureMenu_*, PictureProperties_*
Shared rules stay UI-free DetailRulesBoundaryTests

Twelve new tests in xWorksTests, eleven in FdoUiTests. Manual: a 17-step pass in Sena 3 covering etymology, extended note, note example, example, Variant Info and pronunciation menus in both UIs, plus multi-file media order in legacy. All passed.

Preflight review details

Code Review Summary

Branch: LT-22691r (local claude/lt-22691r-72ec79)
Base: main (merge base c734bff)
Date: 2026-10-26
Review model: Claude (Claude Code)
Files changed: 10 (1 commit)

Overview

Stage 2 PR D1 of LT-22691: the Avalonia detail view answers seven more Lexicon item menus
natively instead of building the hidden WinForms DataTree: mnuDataTree-Etymology,
-ExtendedNote, -ExtendedNote-Examples, -Example, -Pronunciation, -VariantSpec and
-Picture. The new answerers are Move Up / Move Down (shared rule DetailSequenceMove, with the
EntryRefs first-slot rule), Insert Sound or Movie (shared rule MediaFileInsertion) and Picture
Properties; Delete and the Inserts were already native. The WinForms Move and Insert Sound or
Movie handlers call the same rules.

The analysis found the implementation complete for every reachable id. -Picture has no Avalonia
row until the composer loads GeneratedParts.xml (LT-22772); it is owned and proved on a built
row against the hidden tree, and a tripwire test fails once picture rows compose.

Contract/API Changes

  • Added public static classes SIL.FieldWorks.Common.DetailRules.DetailSequenceMove and
    MediaFileInsertion (FdoUi).
  • DTMenuHandler: public handler signatures unchanged; private InsertMediaFile reshaped; new
    internal static ChooseMediaFiles.
  • New xWorksStrings keys: ksUndoUpdatePicture, ksRedoUpdatePicture,
    ksUndoCreatePronunciation, ksRedoCreatePronunciation.
  • No serialized format, settings, COM, native or script contract changes.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

  • Src/xWorks/DTMenuHandler.cs: the WinForms Insert Sound or Movie still creates the
    entry's first pronunciation before the file dialog, so Cancel leaves an empty pronunciation; the
    New UI creates it only once files are picked.
    (WinForms behaviour deliberately left
    unchanged.)
  • Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs: EditPictureProperties
    repeats PictureSlice.showProperties (dialog plus the UpdatePicture step).
    (The dialog is
    WinForms and stays in each host.)
  • RecordEditView.CommandActsOn: Delete and Move are hidden on the Avalonia-only owner rows
    (Pronunciations header; a note's Examples header and empty-note prompt row).
    (Parity ruling
    by the author: those rows' object is the owner, so the item commands do not act there.)

Required Validation / Evidence

  • ./build.ps1 -CommentHygiene -TokenHygiene: clean.
  • ./test.ps1 -TestFilter (class names, totals checked): LexiconMenuAuthorityTests +
    LexiconMenuExecutionTests + MediaFileInsertionTests 56/56; DetailSequenceMoveTests,
    DetailRulesBoundaryTests, DetailObjectDeletionTests green; DetailObjectCommandExecutionTests,
    WindowConfigurationIntegrityTests, DataTreeMoveReachabilityTests, DetailMenuRequestTests,
    DetailObjectInsertionTests, DetailFocusMemoryTests, DetailEditorParityTests 157/157.
  • Manual: the author ran the full 17-step pass in Sena 3, both UIs, and confirmed all passed.

Positive Observations

  • Shared rules live in Src/FdoUi/DetailRules/ under the boundary guard; the WinForms path calls
    them, so the two UIs cannot drift.
  • Dialogs are reached through replaceable seams (MediaFilesToInsert, PictureFileChosen), so
    every native command is tested end to end headlessly.
  • The equivalence contract now covers 21 owned ids on 35 rows and names each divergence it allows.
  • The LT-22772 tripwire makes the picture-row workaround self-reporting.

Interview Notes

  • E3 move-in-sequence ships here rather than in D2: D2 had not started and D1 is E3's first
    consumer. D2 keeps E6 merge, E10 and E12.
  • -Picture is owned now although no Avalonia row reaches it; the author chose authority-level
    proof plus a tripwire over deferring the id. LT-22772 is not fixed here.
  • The class guard (CommandActsOn) is a parity ruling: WinForms' only precedent for an item id on
    its owner is the ghost slice.
  • Manual test 17 found the first draft reversed multi-file media order; the order rule was
    restored (each link at the old end) on both paths and re-tested.
  • The author asked for the create-pronunciation undo texts to move into resources; done.
  • Nothing else to flag.

Suggested Review Focus

  • DetailSequenceMove.CanMoveUp's EntryRefs rule against the old DTMenuHandler display rule.
  • RecordEditView.CommandActsOn and the owner rows it hides Delete and Move on.
  • MediaFileInsertion.Insert ordering, and the one-step Undo in InsertMediaFiles.
  • The LT-22772 tripwire message and the tagged comments.

🤖 Generated with Claude Code


This change is Reviewable

The Avalonia detail view now answers the menus of an etymology, an
extended note, a note's examples, an example, a pronunciation, Variant
Info and a picture itself: Move Up and Move Down, Delete, the Inserts,
Insert Sound or Movie and Picture Properties all display and run from
the row. Until now each of these menus still built the hidden WinForms
DataTree to answer them.

The WinForms view's Move and Insert Sound or Movie commands now run the
same shared rules.

Owned by the native authority from this change:
* mnuDataTree-Etymology
* mnuDataTree-ExtendedNote
* mnuDataTree-ExtendedNote-Examples
* mnuDataTree-Example
* mnuDataTree-Pronunciation
* mnuDataTree-VariantSpec
* mnuDataTree-Picture, which no Avalonia row reaches until the composer
  loads the generated parts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   15m 0s ⏱️ +18s
6 855 tests +23  6 770 ✅ +23  85 💤 ±0  0 ❌ ±0 
6 864 runs  +23  6 779 ✅ +23  85 💤 ±0  0 ❌ ±0 

Results for commit 6d63ef3. ± Comparison against base commit ddf8b5a.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.34783% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.99%. Comparing base (c734bff) to head (6d63ef3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs 72.32% 16 Missing and 15 partials ⚠️
Src/xWorks/DTMenuHandler.cs 13.33% 19 Missing and 7 partials ⚠️
Src/FdoUi/DetailRules/DetailSequenceMove.cs 97.77% 0 Missing and 1 partial ⚠️
Src/FdoUi/DetailRules/MediaFileInsertion.cs 97.67% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1207      +/-   ##
==========================================
+ Coverage   39.93%   39.99%   +0.06%     
==========================================
  Files        1542     1544       +2     
  Lines      355165   355256      +91     
  Branches    41317    41345      +28     
==========================================
+ Hits       141833   142087     +254     
+ Misses     183999   183839     -160     
+ Partials    29333    29330       -3     
Files with missing lines Coverage Δ
Src/FdoUi/DetailRules/DetailSequenceMove.cs 97.77% <97.77%> (ø)
Src/FdoUi/DetailRules/MediaFileInsertion.cs 97.67% <97.67%> (ø)
Src/xWorks/DTMenuHandler.cs 25.81% <13.33%> (+1.86%) ⬆️
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs 73.60% <72.32%> (-0.48%) ⬇️

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

@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 reviewed 10 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on mark-sil).

@mark-sil
mark-sil merged commit 173a803 into main Oct 27, 2026
9 checks passed
@mark-sil
mark-sil deleted the LT-22691r branch October 27, 2026 10:55
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