Repository navigation
LT-22691: Answer the main-menu inserts and View toggles natively - #1228
Merged
Merged
Conversation
In the New UI, Lexicon Edit's Insert menu and pane-bar inserts and conversions, and View > Show Hidden Fields and Show Dictionary Preview, are answered by the detail view. Until now only the hidden DataTree answered them, so they worked only after a right-click had built it, against the row it last showed. Menus, toolbars and pane bars now ask the window's command authorities before the mediator, and the mediator never sees what one answers; a menu raised on a row, item or header does not ask them. The detail view registers one while it is shown. Its commands act on the row that last had keyboard focus in the shown record, else on the shown entry. The commands it answers: * CmdInsertSense, CmdInsertSubsense, CmdInsertExtNote, CmdDataTree-Insert-AlternateForm, CmdDataTree-Insert-Pronunciation, CmdDataTree-Insert-Etymology * CmdInsertMediaFile, CmdInsertPicture, CmdInsertVariant * CmdChangeToComplexForm, CmdChangeToVariant * the ShowHiddenFields and Show_DictionaryPubPreview toggles, in every New UI tool Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thejambi
approved these changes
Nov 4, 2026
thejambi
left a comment
Contributor
There was a problem hiding this comment.
@thejambi reviewed 29 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on mark-sil).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the New UI, Lexicon Edit's Insert menu and pane-bar inserts and
conversions, and View > Show Hidden Fields and Show Dictionary Preview, are
answered by the detail view. Until now only the hidden DataTree answered
them, so they worked only after a right-click had built it, against the row
it last showed.
Menus, toolbars and pane bars now ask the window's command authorities
before the mediator, and the mediator never sees what one answers; a menu
raised on a row, item or header does not ask them. The detail view
registers one while it is shown. Its commands act on the row that last had
keyboard focus in the shown record, else on the shown entry.
The commands it answers:
CmdDataTree-Insert-AlternateForm, CmdDataTree-Insert-Pronunciation,
CmdDataTree-Insert-Etymology
New UI tool
Reviewer notes
Start here:
Src/XCore/xCoreInterfaces/Choice.cs, the hook: a command or check-mark item asks the window's registeredICommandAuthorityfirst, and goes to the mediator only when none answers. ThenSrc/xWorks/Avalonia/Hosting/RecordEditView.MainMenus.cs, the detail view's answers.The question to bring: does the hook take over menus it shouldn't? It answers only when all of these hold: a New UI detail view is active in the window and has registered; the item is not in a menu raised on a target (
ChoiceGroup.IsRaisedOnTarget, set byXWindow.ShowContextMenuandGetContextMenuChoiceGroup); and the authority holds an answer for that message or property. Everything else reaches the mediator as before, and the Legacy UI never registers an authority.Where to look
LexiconMenuAuthorityTestsequivalence tests against the WinForms baseline stay green.LexiconMenuExecutionTests.MainMenu.cs.DetailCurrentRow,DetailCurrentRowTests.DetailObjectInsertion.CanInsert(..., commandId, ...),EntryConversion,ShownRecordCommandScope,DictionaryPreviewAvailability;DTMenuHandler,DataTreeandLexEntryMenuHandlercall them.SwitchingToTheLegacyUI_LeavesTheCommandsToTheMediator.Deliberately not here
GetChoiceGroupForMenuand WinForms hotlinks (GetWindowsFormsContextMenu) are not flagged as target menus. None carries a message these authorities answer; noted for stage 4.Verification: build, comment hygiene and token hygiene clean; 265 targeted tests passed, then 97 after the review fixes; 15 manual steps on Sena 3 in the New UI passed. Not covered by automated tests: the pop-up window gate and the keyboard-shortcut path (no command here has a shortcut). Full suite not run.
Next: approve, or name a menu that should not ask the authorities.
Decisions, and why
On*/OnDisplay*methods onRecordEditViewwould have added mediator use that the Pub/Sub work must later remove. The hook is a direct interface call. Pub/Sub cannot carry it, because enabling an item is a question that needs an answer back.IsRaisedOnTargetflag rather than the existingChoiceGroup.IsContextMenu, which means "defined under<contextMenus>" and so also covers the pane-bar menus.Preflight review details
Code Review Summary
Branch: claude/lt-22691-main-menu-ba73fa
Base: main (merge base 936ae91; main has 6 newer commits that touch none of these files and merge cleanly)
Date: 2026-11-04
Review model: Claude Opus 5.5
Files changed: 29 (1 commit, 93d83ba)
Overview
LT-22691 main-menu PR (items 3a and 3b). In the New UI, Lexicon Edit's Insert menu and
pane-bar inserts and conversions, and View > Show Hidden Fields and Show Dictionary Preview,
had only the hidden DataTree as answerer; after the sense-menu PR (#1214) nothing in Lexicon
Edit builds that tree, so they were permanently disabled. The detail view now answers them
itself through a new XCore hook: menus, toolbars and pane bars ask the window's registered
command authorities before the mediator, and the mediator never sees what one answers. Menus
raised on a target (a row, item or header) are kept out of the hook. Commands act on the
"current row" (the row of the shown record that last had keyboard focus), else the shown
entry.
The four analysis passes found no Critical or Important issue. Earlier
/code-review highfindings were resolved or decided with the author (below).
Contract/API Changes
xCoreInterfaces): newICommandAuthorityandCommandAuthorities(per-property-tableregistry, most recent first);
Command.InvokeCommand(ICommandAuthority);CommandChoice.QueryDisplayProperties(Mediator, PropertyTable, ...);ChoiceGroup.IsRaisedOnTarget/IsInMenuRaisedOnTarget;ChoiceBase.CommandAuthority.XWindow.ShowContextMenu(both overloads) andGetContextMenuChoiceGroupflag their groups.DataTree.RowFocused;DetailCurrentRow;DetailHostControl.CurrentRow;optional field name on
DataTree.RequestObjectFocusOnRebuildandDetailHostControl.FocusObjectOnNextShow;DetailFocusMemory.Memento.ObjectField.DetailObjectInsertion.CanInsert(root, row, commandId, field, out index),EntryConversion,ShownRecordCommandScope,DictionaryPreviewAvailability. WinFormsDTMenuHandler,DataTreeandLexEntryMenuHandlercall them; behaviour unchanged exceptthat an invalid root now disables Insert Variant and the conversions.
Findings
Critical - Must address before merge
None.
Important - Should address before merge
None.
Minor - Consider
Item menus built through(author: no such menu carries a message these authorities answer today; leave it and note it for stage 4)GetChoiceGroupForMenu(FdoUiCore) and WinForms hotlinks viaGetWindowsFormsContextMenuare not flagged as raised on a target, so they would ask the window's authoritiesThe Lexicon Edit Popup window shares the main window's property table; its pane-bar inserts stay unanswered in the New UI(author: fix in the Notebook stage 4 PR, where Notebook's Insert Entry "Create and Edit" is the route that opens the pop-up in the New UI)Three wrappers read className/restrictToTool before(author: leave as is)ShownRecordCommandScope.AppliesCommandAuthoritiescopied its list on every display query (fixed: iterates by index)SwitchingToTheLegacyUI_LeavesTheCommandsToTheMediator)Required Validation / Evidence
.\build.ps1 -CommentHygiene -TokenHygiene: clean (comment and token hygiene clean)..\test.ps1 -CommentHygiene -TokenHygiene -TestFilter ...: 265/265 over CommandAuthoritiesTests,DetailObjectInsertionTests, EntryConversionTests, ShownRecordCommandScopeTests,
DictionaryPreviewAvailabilityTests, DetailRulesBoundaryTests, DetailCurrentRowTests,
DetailFocusMemoryTests, LexiconMenuExecutionTests, LexiconMenuAuthorityTests,
DetailObjectCommandExecutionTests, DetailMenuRequestTests, DetailEditorParityTests,
WindowConfigurationIntegrityTests, DataTreeMoveReachabilityTests,
UIFrameworkAndHostContractTests; then 97/97 after the review fixes.
IsInOwningWindow) and thekeyboard-shortcut path through the hook (no 3a/3b command has a shortcut).
Positive Observations
moved to
Src/FdoUi/DetailRulesinstead of being copied.colleague, and that the hidden tree is never built.
Interview Notes
Decisions settled with the author during the session (not re-asked here):
leaves the view, re-found after rebuilds, cleared on a record change; a variant's rows count
as none; with no current row, commands act on the shown entry.
object to be a sense.
lands on that row's "..." button; the author chose to keep it there.
On*/OnDisplay*mediator receivers so the change doesnot expand mediator use.
Suggested Review Focus
Choice.cs/Command.cs/xWindow.cs: the hook only answers when an authority is registered and the item is not in a menu raised on a target.RecordEditView.MainMenus.cs: registration lifecycle andMainMenuRow.DataTree.RowFocusedandDetailCurrentRow: how the current row survives rebuilds.🤖 Generated with Claude Code
This change is