Skip to content

LT-22691: Answer the Category Edit menus natively - #1244

Merged
mark-sil merged 1 commit into
mainfrom
claude/lt-22691-posedit-b4e759
Nov 12, 2026
Merged

mark-sil merged 1 commit into
mainfrom
claude/lt-22691-posedit-b4e759

Conversation

@mark-sil

@mark-sil mark-sil commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

The Avalonia detail view answers every Grammar > Category Edit menu
itself, and the Insert menu's category commands. Until now Duplicate and
Move on an affix slot or template, and Insert Subcategory, needed the
hidden DataTree, where Move could not show its category list; the Insert
menu's commands showed only once a right-click had built that tree.

Move asks for the category with the Avalonia chooser dialog, and
Duplicate is one undoable step in the New UI. In both views a move's undo
text is the command's own ("Undo Move Affix Slot") in place of fixed
English, and moving an item to the category it is in does nothing. The
move, duplicate and subcategory rules are shared with WinForms in
DetailRules.

Inserting a list item's sub-item is answered natively too, which covers
the Subitems menu of Exception "Features" and the Lists area's sub-item
menus.

Duplicating an affix slot whose name is still being typed keeps the name
on the original and copies it, which fixes LT-22843.


Reviewer notes

Start here: Src/xWorks/Avalonia/Hosting/RecordEditView.CategoryMenus.cs, the Duplicate and Move answers. Then the rules both views call, in Src/FdoUi/DetailRules: CategoryItemMove.cs, DetailObjectDuplication.cs, and OffersSubItem with the new ResolveTarget overload in DetailObjectInsertion.cs.

The question to bring: what changed in Legacy? Two things, both intended. A move's undo text now comes from the command ("Undo Move Affix Slot", "Undo Move Affix Template") in place of the hardcoded "Undo Move Slot" and "Undo Move Template". Moving a slot to the category it is already in no longer makes an undo step. Legacy keeps its in-row category drop-down with <Any> and More..., its two-step Duplicate, and its MasterRefresh after a slot move.

Where to look

  • Every Category Edit row menu is answered natively: CategoryEditMenuTests.EveryRowMenu_OfACategoryWithSlotsTemplatesAndSubcategories_IsAnsweredNatively; the owned-id pin in LexiconMenuAuthorityTests shows the nine new menus.
  • They render what the adapter path rendered, with no mediator query: TheSlotTemplateAndSubcategoriesMenus_RenderWhatTheAdapterPathRendered_WithoutTheMediator.
  • CategoryItemMove.Move against the removed DTMenuHandler.MoveSlot and MoveTemplate: a template moved to the category above takes its slots unless a slot of that name exists there. Pinned by CategoryItemMoveTests.
  • Insert Subcategory goes under the shown category, and beside a subcategory from that subcategory's row: InsertSubcategory_GoesUnderTheShownCategory_AndBesideASubcategoryFromItsRow.
  • Sub-item inserts are in scope for every list (InsertLeafInScope), so six sub-item menus of the Lists area are owned too. ALeafItsAnswersScopeRefuses_LeavesItsIdUnowned_AndIsNotBuilt keeps the scope-refusal path tested.

Deliberately not here

  • The Move dialog has no <Any> and no More..., preselects nothing, and lists categories in list order, not by name.
  • An affix template's own name row still shows as "Name" with only the Help menu; the template menu is on the "Affix Templates" heading row. Separate change.
  • mnuInflAffixTemplate-TemplateTable: the template table is an Unsupported row in the New UI, so nothing raises that menu.
  • Insert Subcategory still opens the WinForms catalog dialog (MasterCategoryListDlg).

Verification: build, comment and token hygiene clean. Targeted tests passed: 39 rule tests in FdoUiTests, 53 in LexTextControlsTests, 172 over the xWorks menu fixtures, then 54 over the three fixtures that share the new test helper. 13 manual steps on Sena 3 in both UIs passed. Tests only (Sena 3 has no data): a template moved up taking its slots, and Insert subitem under Exception "Features". The Move dialog's modal loop is not run by tests. Full suite not run. Main's two newer commits merge cleanly; the merged tree was not built.

Next: approve, or name a Category Edit menu that still behaves differently from Legacy.

Decisions, and why
  • Move asks with a modal dialog, not an in-row drop-down. The Avalonia ChooserDialog in tree single-select mode already exists (LcmCreatePartOfSpeechLauncher uses it); LcmCategoryChooserLauncher adds only the input and the result. Legacy's TreeCombo with POSPopupTreeManager is left as it is. Rejected: an in-form drop-down tree on the row, which would need a transient editor in a row that is not editing.
  • The dialog preselects nothing and OK waits for a choice. The tree chooser cannot show an initial selection: it only primes the chosen keys, so a preselected category would be invisible with OK enabled.
  • Undo text comes from the command's label in both views. The hardcoded English "Undo Move Slot" / "Undo Move Template" could not be localized; the command's own text can.
  • Moving to the current category does nothing. Legacy did this for a template but made an empty undo step for a slot; the shared rule refuses both.
  • No MasterRefresh after a native move. Legacy publishes it after a slot move to redraw the template table (LT-22240). The New UI has no template table, and its re-show after the edit redraws the rows.
  • Duplicate is one undoable step in the New UI. Legacy inserts the object and then copies onto it, as two steps; it now shares the copy step (DetailObjectDuplication.CopyProperties) and keeps two. The native path creates the copy with the plain model call and copies in the same step, so it is offered only for an object that can copy itself and sits in an owning sequence or collection. The copy's name is the model's: the original's plus "2".
  • Insert Subcategory goes under the shown category. The command's slice="owner" means the owner of the row's object, but Legacy switches to the owner only when a slice shows it, so on the shown record it inserts under that record. ResolveTarget now takes the shown record and does the same.
  • Sub-item inserts are in scope for every list, not only categories. The rule is the same for all of them (OffersSubItem: the command's class must be the shown record's class). Rejected: a scope limited to Category Edit, which would leave the same command answered two ways.
  • Legacy's check that a Subitems slice is showing is not replicated. It stays in DTMenuHandler.CanInsert; the native menu is built from the row that binds it.
  • The Insert menu's category commands use the main-menu hook from LT-22691: Answer the main-menu inserts and View toggles natively #1228 (CreateCommandAuthority), now also registered in Category Edit. A command that does not apply to the shown category is disabled, and since those items are defaultVisible="false" it is hidden, as in Legacy.
  • Strings: none added or removed.
Found in passing

Also happens on main; kept out of this PR.

  • FLEx crashes (NullReferenceException in DataTree.SetCurrentSlicePropertyNames) when the UI mode is switched from New to Legacy on a tool whose list is empty, such as Exception "Features" in Sena 3, and the user then leaves the tool. The switch shows the WinForms tree without initializing it.
Preflight review details

Code Review Summary

Branch: claude/lt-22691-posedit-b4e759
Base: main (merge base c6cd25f; main has 2 newer commits, #1241 and #1198; #1241 touches
Slice.cs in other hunks and merges cleanly; the merged tree was not built)
Date: 2026-11-11
Review model: Claude Fable 5.2
Files changed: 18 (1 commit, fe5b5ff)

Overview

LT-22691 stage 4, Category Edit (with main-menu item 3c). In the New UI, every Grammar >
Category Edit row menu is answered by the detail view, and so are the Insert menu's four
category commands. Until now Duplicate and Move on an affix slot or template, and Insert
Subcategory, needed the hidden DataTree. Move asks for the category with an Avalonia chooser
dialog; Duplicate is one undoable step. The move, duplicate and sub-item rules live in
Src/FdoUi/DetailRules and the WinForms handlers call them. Sub-item inserts are in scope
for every list, which also covers Exception "Features" and the Lists area's sub-item menus.
The native Duplicate fixes LT-22843.

The review was /code-review high on the working tree before the commit, then the author's
own code review. Nothing Critical or Important is open. The branch has no native, build,
installer or resource change.

Contract/API Changes

  • DetailRules (FdoUi): new CategoryItemMove and DetailObjectDuplication;
    DetailObjectInsertion gains SubItemsField, OffersSubItem and a ResolveTarget
    overload that takes the shown record.
  • xWorks: DTMenuHandler.GetPOSSlot (public static), MoveSlot and MoveTemplate removed;
    the rule is CategoryItemMove. DTMenuHandler.CanInsert and Slice.HandleCopyCommand
    call the shared rules.
  • xWorks host: RecordEditView.CategoryMenus.cs (Duplicate, Move, the MoveDestinationChosen
    test seam); the Lexicon cluster authority answers DataTreeCopy and DataTreeMove;
    InsertLeafInScope no longer excludes SubPossibilities; the command authority answers
    DataTreeInsert in Category Edit.
  • LexTextControls: new LcmCategoryChooserLauncher over the Avalonia ChooserDialog.
  • WinForms behaviour: a move's undo text is the command's ("Undo Move Affix Slot", "Undo Move
    Affix Template") in place of the hardcoded "Undo Move Slot" / "Undo Move Template"; moving
    a slot to the category it is in no longer makes an undo step.
  • No strings added or removed. No configuration, native or installer change.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

  • The Move dialog was given the current category as its initial key, but the tree
    chooser cannot show a selection, so nothing was highlighted and OK was enabled
    (fixed:
    the dialog opens with nothing selected and OK waits for a choice)

Minor - Consider

  • The scope-predicate refusal of the cluster authority lost its only test (fixed:
    ALeafItsAnswersScopeRefuses_LeavesItsIdUnowned_AndIsNotBuilt)
  • DataTreeMoveReachabilityTests still said Move cannot work in the Avalonia view
    (fixed: summary and assertion messages reworded; both tests unchanged)
  • DetailObjectDuplication.Duplicate did not say it creates the copy with the plain
    model call, skipping any step a class adds to its Insert
    (fixed: stated in the doc
    comment)
  • CategoryEditMenuTests repeated the menu-test plumbing of two other fixtures
    (fixed: DetailMenuTestSupport.cs, used by all three)
  • Each Insert item resolves the shown record twice (author: leave it as is)
  • After a move the focus goes to the nearest row left, assuming the moved object's
    rows leave the view
    (judged in the review to need no change: the focus then lands on a
    neighbouring row, nothing is lost)

Required Validation / Evidence

  • .\build.ps1 -CommentHygiene -TokenHygiene: clean.
  • .\test.ps1 -CommentHygiene -TokenHygiene -TestFilter ...: FdoUiTests rule fixtures
    39/39; LexTextControlsTests launcher fixtures 53/53; xWorksTests 172/172 over
    CategoryEditMenuTests and the neighbouring menu fixtures, then 54/54 over
    CategoryEditMenuTests, LexiconMenuAuthorityTests and NotebookMenuTests after the shared
    helper.
  • Manual: the author ran 13 steps on Sena 3 in both UIs, including the LT-22843 steps; all
    passed.
  • Tests only (Sena 3 has no data): a template moved to the category above taking its slots;
    Insert subitem under Exception "Features".
  • Not covered by automated tests: the Move dialog's modal loop (tests supply the pick).
  • Full suite not run (targeted runs only). The tree merged with main's 2 newer commits was
    not built.

Positive Observations

  • No new mediator receiver and no new reliance on the hidden tree.
  • The move, duplicate and sub-item rules are shared, not copied; WinForms calls them.
  • The owned-id pin in LexiconMenuAuthorityTests shows the nine newly owned menus.

Interview Notes

Decisions settled with the author during the session (not re-asked here):

  • Move asks with a modal Avalonia chooser dialog; WinForms keeps its in-row drop-down with
    <Any> and More....
  • Undo text comes from the command in both views; picking the current category does nothing.
  • No MasterRefresh after a native move: WinForms publishes it to redraw the template table
    (LT-22240), which the New UI does not have.
  • Duplicate is one undo step natively; WinForms keeps two and shares the copy step.
  • Insert Subcategory goes under the shown category, as in WinForms, and keeps the WinForms
    catalog dialog.
  • Sub-item inserts are in scope for every list; the WinForms check that a Subitems slice is
    showing is not replicated.
  • The affix template's name row ("Name" with only the Help menu) is a layout-import gap and a
    separate change, not this PR.
  • Found during manual testing, also on main, kept out of scope: FLEx crashes when the UI mode
    is switched from New to Legacy on a tool whose list is empty and the user then leaves the
    tool.
  • Jira: the branch carries LT-22691; LT-22843 is fixed by the native Duplicate.

Suggested Review Focus

  • CategoryItemMove.Move against the removed DTMenuHandler.MoveSlot / MoveTemplate:
    same moves, same slot rule.
  • DetailObjectInsertion.ResolveTarget with the shown record: slice="owner" no longer
    starts at the owner when the row's object is the shown record.
  • RecordEditView.CategoryMenus.cs: when Duplicate and Move are absent, disabled or
    enabled.

🤖 Generated with Claude Code


This change is Reviewable

The Avalonia detail view answers every Grammar > Category Edit menu
itself, and the Insert menu's category commands. Until now Duplicate and
Move on an affix slot or template, and Insert Subcategory, needed the
hidden DataTree, where Move could not show its category list; the Insert
menu's commands showed only once a right-click had built that tree.

Move asks for the category with the Avalonia chooser dialog, and
Duplicate is one undoable step in the New UI. In both views a move's undo
text is the command's own ("Undo Move Affix Slot") in place of fixed
English, and moving an item to the category it is in does nothing. The
move, duplicate and subcategory rules are shared with WinForms in
DetailRules.

Inserting a list item's sub-item is answered natively too, which covers
the Subitems menu of Exception "Features" and the Lists area's sub-item
menus.

Menus and commands now answered natively:
* mnuDataTree-POS-AffixSlot, mnuDataTree-POS-AffixTemplate,
  mnuDataTree-POS-SubPossibilities
* mnuDataTree-SubPossibilities, mnuDataTree-SubSemanticDomain,
  mnuDataTree-SubComplexEntryType, mnuDataTree-SubVariantEntryType,
  mnuDataTree-SubAnthroCategory, mnuDataTree-SubLocation
* CmdDataTree-Insert-POS-SubPossibilities,
  CmdDataTree-Insert-POS-AffixTemplate, CmdDataTree-Insert-POS-AffixSlot,
  CmdDataTree-Insert-POS-InflectionClass (Category Edit)

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

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   14m 31s ⏱️ -34s
7 240 tests +28  7 155 ✅ +28  85 💤 ±0  0 ❌ ±0 
7 249 runs  +28  7 164 ✅ +28  85 💤 ±0  0 ❌ ±0 

Results for commit fe5b5ff. ± Comparison against base commit a1f69e7.

This pull request removes 1 and adds 29 tests. Note that renamed tests count towards both.
SIL.FieldWorks.XWorks.LexiconMenuAuthorityTests ‑ InsertLeaves_OutsideTheScope_LeaveTheirIdsWithTheAdapter
LexTextControlsTests.LcmCategoryChooserLauncherTests ‑ BuildInput_OffersEveryCategoryAsATree_WithNothingSelected_AndRequiresAChoice
LexTextControlsTests.LcmCategoryChooserLauncherTests ‑ Resolve_ReturnsTheCategoryAKeyNames_AndNullForNoneOrAnother
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ CategoryOf_IsTheOwnerOfASlotOrATemplate_AndNullForAnythingElse
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ Move_ASlot_PutsItInTheCategory_WithItsAffixes_InOneUndoableStep
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ Move_ATemplateToAnotherCategoryThanTheOneAbove_LeavesItsSlots
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ Move_ATemplateToTheCategoryAbove_LeavesASlotWhoseNameIsUsedThere
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ Move_ATemplateToTheCategoryAbove_TakesItsSlotAlong_InOneUndoableStep
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ Move_ToTheCategoryItIsIn_OrOfAnotherKindOfObject_DoesNothing
SIL.FieldWorks.FdoUi.CategoryItemMoveTests ‑ SlotNamed_FindsASlotOfTheCategoryOrOneAboveIt
SIL.FieldWorks.FdoUi.DetailObjectDuplicationTests ‑ CanDuplicate_AnObjectThatCopiesItself_InAnOwningVector
…

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.53886% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.83%. Comparing base (c6cd25f) to head (fe5b5ff).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...extControls/Avalonia/LcmCategoryChooserLauncher.cs 45.00% 19 Missing and 3 partials ⚠️
...s/Avalonia/Hosting/RecordEditView.CategoryMenus.cs 68.62% 6 Missing and 10 partials ⚠️
Src/xWorks/DTMenuHandler.cs 11.11% 4 Missing and 4 partials ⚠️
Src/Common/Controls/DetailControls/Slice.cs 0.00% 2 Missing ⚠️
Src/FdoUi/DetailRules/DetailObjectDuplication.cs 92.85% 1 Missing and 1 partial ⚠️
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs 60.00% 0 Missing and 2 partials ⚠️
Src/FdoUi/DetailRules/CategoryItemMove.cs 97.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1244      +/-   ##
==========================================
+ Coverage   40.69%   40.83%   +0.13%     
==========================================
  Files        1567     1573       +6     
  Lines      356898   357159     +261     
  Branches    41755    41812      +57     
==========================================
+ Hits       145241   145832     +591     
+ Misses     182321   182022     -299     
+ Partials    29336    29305      -31     
Files with missing lines Coverage Δ
Src/FdoUi/DetailRules/DetailObjectInsertion.cs 89.83% <100.00%> (+0.60%) ⬆️
...Works/Avalonia/Hosting/RecordEditView.MainMenus.cs 78.81% <100.00%> (+0.36%) ⬆️
Src/FdoUi/DetailRules/CategoryItemMove.cs 97.77% <97.77%> (ø)
Src/Common/Controls/DetailControls/Slice.cs 61.03% <0.00%> (+0.78%) ⬆️
Src/FdoUi/DetailRules/DetailObjectDuplication.cs 92.85% <92.85%> (ø)
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs 71.19% <60.00%> (+0.25%) ⬆️
Src/xWorks/DTMenuHandler.cs 31.76% <11.11%> (+4.13%) ⬆️
...s/Avalonia/Hosting/RecordEditView.CategoryMenus.cs 68.62% <68.62%> (ø)
...extControls/Avalonia/LcmCategoryChooserLauncher.cs 45.00% <45.00%> (ø)

... and 32 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 18 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 b4e29e5 into main Nov 12, 2026
9 checks passed
@mark-sil
mark-sil deleted the claude/lt-22691-posedit-b4e759 branch November 12, 2026 02:08
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