Repository navigation
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1172 +/- ##
==========================================
+ Coverage 39.30% 39.39% +0.08%
==========================================
Files 1523 1526 +3
Lines 353047 353234 +187
Branches 40750 40784 +34
==========================================
+ Hits 138782 139142 +360
+ Misses 185001 184834 -167
+ Partials 29264 29258 -6
🚀 New features to boost your workflow:
|
ef764a1 to
4b9fcb6
Compare
| { | ||
| SettleDetailEdits(); | ||
| #pragma warning disable 618 // suppress obsolete warning | ||
| m_mediator.PostMessage("FollowLink", |
There was a problem hiding this comment.
This PostMessage("FollowLink") call was taken from DataTree.OnJumpToToolAndFilterAnthroItem along with the link it posts. It stays a deferred mediator post on purpose: LT-21401 Part 1 (#921) converted only the synchronous SendMessage senders of FollowLink, so this one should be cleaned up together with the other FollowLink PostMessage calls, with the timing analysis that conversion needs.
|
Looks good, just a few minor things that could potentially be addressed:
|
The per-item menu of a reference-vector row is now answered in full from the row and the clicked item: the jumps by asking the item's object UI directly, the anthropology-category filter jumps and the complex-form visibility marks through rules in FdoUi/DetailRules that the WinForms handlers call too. Nothing on the mediator takes part, so the hidden DataTree adapter is never built for it, and a Ctrl+click runs the same default jump without it. This fixes two leaves that were broken on the Avalonia path because they read a WinForms selection the hidden tree never has: Show Subentry under this Component never appeared, and the two filter jumps were enabled but did nothing. Show Subentry is now offered on a complex form's Components row, and the filter jumps carry the clicked category. The environments item menu keeps the colleague path until its own authority lands. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thanks, all five addressed in 7a4d4cb.
|
thejambi
left a comment
There was a problem hiding this comment.
@thejambi resolved 1 discussion.
Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion.
thejambi
left a comment
There was a problem hiding this comment.
@thejambi reviewed 10 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion.
thejambi
left a comment
There was a problem hiding this comment.
@thejambi resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on mark-sil).
|
You are right, Notebook does have an Avalonia detail view; I had it filed with the deferred tools. So the divergence is reachable: on a subrecord, a record-reference chip's "Show Record in Notebook" used to jump to the subrecord's parent (the |
Brings in #1172. Its ReferenceItemMenuAuthority does not implement IDetailMenuAuthority.BuildList yet; the next commit adds it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Start here:
Src/xWorks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs, the one class that answers the menu. Everything else either wires it into the host or moves a rule intoSrc/FdoUi/DetailRulesso WinForms and Avalonia share it.What it does. The per-item menu of a reference-vector row in the Avalonia detail view (
mnuReferenceChoices: the chips on Subentries, Components, Anthropology Categories and other reference fields) is now answered natively. Nothing on the mediator takes part: the hidden DataTree adapter is never pointed at the row, and the item's object UI is no longer registered as a temporary colleague. Ctrl+click runs the same default jump the same way.The question to settle. Does the native menu render exactly what the mediator path rendered? For every item of every vector row of the test entry, yes, with one deliberate difference: two leaves that read a WinForms rootbox selection, which the hidden tree never has, were broken on the Avalonia path and now work. Show Subentry under this Component never appeared; the two anthropology-category filter jumps were enabled but did nothing.
Where to look.
CmObjectUidisplay and execute methods directly, not re-implemented. Pinned by the leaf-coverage and equivalence tests.ComplexFormVisibilityreplaces two hand-written ordered-insert loops inDTMenuHandler; the FdoUi tests pin component order and undo.WithoutLeakedSubentryMarkdrops one item from the equivalence baseline: the adapter leaks Show Subentry as enabled on Publish Sense In. Pre-existing adapter defect; see Evidence.CommandChoice.CommandObjectbecomes public, temporary until E2b.PostMessage("FollowLink")moved fromDataTree; see the inline comment.Deliberately not here. The environments chips (
mnuEnvReferenceChoices) still use the colleague path; their authority is track A row 2, which deletesBuildItemMenuThroughTheColleague,AddMoveCommandsand the Ctrl+click fallback. Converting the deferred FollowLink post belongs with LT-21401's remaining senders.CmdVisibleComplexFormis answered but unreachable: the Complex Forms row composes read-only.Verification. Build with both hygiene gates clean. Locally green: 11 FdoUi DetailRules tests, 41 FwAvalonia parity and menu tests, the whole 54-test
DetailObjectCommandExecutionTestsfixture. CI green. Manual pass of 16 scenarios in Lexicon Edit, including the WinForms twins of the shared rules. Not run: the full suite.Next: approve, or say if the row-2 deletions should land here instead of in their own PR.
Reading this a year from now -- start here
This PR is stage 1, track A, row 1 of the plan that retires the hidden WinForms
DataTreethe Avalonia detail view keeps alive to answer context-menu commands. The unit of work is one menu id, answered in full by oneIDetailMenuAuthority. Earlier rows shippedmnuReorderVector(#1143), the help-topic engine (#1151), the owned-menu bridge (#1153) and the per-object and Help menus (#1161). The plan of record and the per-id estimates live in gitignored working notes underDocs/migration/working/, so the reasoning behind this id is recorded here rather than in the tree.Decisions, and why
Jumps delegate to
CmObjectUirather than being re-implemented. The 34 jump leaves are answered today by the clicked item's object UI through itsOnDisplayJumpToTool/OnJumpToToolpair, including the per-class rules inCmPossibilityUi,LexSenseUi,MoFormUiandPartOfSpeechUi. Calling those methods directly keeps every rule in one place and removes only the mediator hop. The architect agreed an authority may answer a leaf by calling a non-tree object. Re-implementing them natively is engine E7 of the plan, deferred.Shared rules live in
Src/FdoUi/DetailRules.ComplexFormVisibilityandAnthroItemFilterLinkare control-free, and the WinFormsDTMenuHandlerandDataTreenow call them. The reason is that shared code must keep working once WinForms is removed, whichDetailRulesBoundaryTestsenforces by reflection over every signature andusingin the folder.CommandChoice.CommandObjectis public. The authority needs theCommandbehind a leaf to pass to the object UI. The alternative, a host-supplied lookup throughMediator.CommandSet, was equally temporary: E2b, first in stage 2, has the bridge hand authorities a command id and label instead of aChoiceBase, which deletes the dependency either way.One authority for the item menu, parameterized by row context. It owns
mnuReferenceChoicesnow and takesmnuEnvReferenceChoicesin row 2. Authorities partition by the row context they close over, not one per id.Row 2 is a separate PR. The repository squash-merges, so a second commit here would land as one change answering two ids. Row 2 also needs the caret seam for the five environment insert commands, which is new surface rather than composition.
A jump always targets the clicked item. On the mediator path the hidden
DataTreeanswered the click before the item's object UI, andGetGuidForJumpToToolcould substitute the row's owner. For every Lexicon row its branches end at the command'sTargetId, which is the chip: Subentries and Referenced Complex Forms are virtual fields, a Components row's object is aLexEntryRef, and the possibility-list tools hit no branch. The owner branches change the target in one reachable case: in Notebook, which does have an Avalonia detail view, a record-reference chip on a subrecord (See Also, Supporting Evidence, Counter Evidence, Superseded By). There "Show Record in Notebook" used to jump to the subrecord's parent record, because thenotebookEditbranch returns the row object's owner when that owner is a record; it now jumps to the clicked record. The native path asks the item's object UI and clears the shared command'sTargetIdafter the display query, so no later menu inherits it.The FollowLink post stays a mediator call. LT-21401 Part 1 (#921) converted the synchronous
SendMessage("FollowLink")senders to Pub/Sub; noPostMessagesender has been converted. Switching this one toPublisher.Publishwould move the tool switch inside the click handler, which is the per-site timing analysis that conversion still owes.Paths not taken
Surprising findings
DTMenuHandler.OnDisplayAddComponentToPrimaryreturns before setting any display state, and the leaf keeps xCore's default of visible and enabled. The native path hides it. The equivalence test drops that one item from the colleague-path baseline and names the row in its output.DataTree.DisplayJumpToToolAndFilterAnthroItemdereferenced a null current slice when the row's object was not in the record the hidden tree shows. Only reachable through the adapter; the one line this PR already changed now tolerates it.Obj\Debug\Viewsobjects after a views header change in LT-22674: Cache text analysis only for text NFC leaves unchanged #1166, not a product defect: the native build does not rebuild objects whose included header changed. Remedy: delete that folder and rebuild native. LT-22816: Rebuild native objects when an included header changes #1168 makes header edits rebuild their includers.Deferred, and what would unblock it
mnuEnvReferenceChoices(track A row 2): Describe Error in Environment can be answered from the clicked item now; the five insert commands need the caret position of the row's text editors exposed through the detail model, plus a control-free insert helper shared with the Environments tool'sPhEnvStrRepresentationSlice. Owning it deletesBuildItemMenuThroughTheColleague,AddMoveCommands,MoveCommandItemand the Ctrl+click fallback inRunDefaultItemActivation, and retires or re-bases the equivalence test that uses the colleague path as its baseline.Buildtakes a command id and label;CommandChoice.CommandObjectcan go private again.Evidence
The four contracts every authority carries, all in
DetailObjectCommandExecutionTests:ReferenceItemAuthority_AnswersEveryLeafOfItsMenupopulates the realmnuReferenceChoicesgroup, asserts at least 40 leaves each with a configuration node, builds every leaf, then builds the whole id throughXCoreMenuBridgeso a refused submenu fails the test instead of reverting the menu.ReferenceItemAuthority_RejectsALeafItDoesNotAnswerfeeds it the Help leaf and expectsInvalidOperationException.ReferenceItemMenu_NativeAuthority_RendersWhatTheColleaguePathRendered_ForEveryItembuilds each item of each vector row of the test entry (two subentries, one anthropology category) both ways and compares the rendered trees as text.BuildItemMenuThroughTheColleagueis the production method the unowned branch still uses, so the baseline is real code, not a test copy.ReferenceItemMenu_OfASubentry_IsBuiltWithoutTheAdapterOrTheMediatorasserts a display spy on the mediator was never asked and the hidden tree was never built.Behaviour tests:
ShowSubentryUnderComponent_OnAComplexFormsComponentsRow_TogglesThePrimaryLexemetoggles the mark both ways on a real complex form and shows the colleague path cannot offer it;AnthropologyCategoryItem_OffersTheFilterJumps_AndItsListJumpAsTheDefault;SubentriesCtrlClick_ResolvesTheClickedEntry_AndRunsTheDefaultJumpPathcaptures the posted link and checks its tool and target;ItemMenu_IsOwned_ForReferenceChoices_ButNotForEnvironmentsstates in one place which item menu still needs the adapter.Rules:
ComplexFormVisibilityTests(component-order insert, unmark, unlisted component, undo task, variant refs excluded) andAnthroItemFilterLinkTests(field gate, link contents) in FdoUiTests;DetailRulesBoundaryTestsunchanged and passing.Manual (Lexicon Edit, Avalonia detail view unless noted): subentry chip menu contents and both jumps; Ctrl+click; edit saved before a jump; Shift+F10 anchoring; Move Left/Right with end disabling; Show Subentry on a complex form's Components row with checkmark, dictionary effect and undo, and the same in the WinForms view; both anthropology filters from Avalonia and from WinForms; unchanged menus on possibility, Referenced Complex Forms, Environments and label rows.
Preflight review details
Code Review Summary
Branch: LT-22691k
Base: main
Date: 2026-10-01
Review model: Claude Fable 5.1
Files changed: 10
Overview
Stage 1, track A, row 1 of the hidden-DataTree retirement plan (LT-22691): the per-item menu of a reference-vector row (
mnuReferenceChoices) is answered natively by a newReferenceItemMenuAuthority, so the item-menu path no longer points the hidden DataTree adapter at the row or registers the item's object UI as a mediator colleague. The 34 Show-in-tool jumps are answered by calling the item'sCmObjectUidisplay and execute methods directly; the anthropology filter jumps and the two complex-form visibility marks are answered through control-free rules inSrc/FdoUi/DetailRules, which the WinFormsDTMenuHandlerandDataTreenow call too.Analysis found no Critical or Important issues. Two leaves that were broken on the Avalonia path are fixed as a consequence: Show Subentry under this Component (never shown, it read a WinForms selection) and the two anthropology filter jumps (enabled but inert for the same reason).
Contract/API Changes
XCore.CommandChoice.CommandObjectbecomes public (was private). Temporary until E2b hands authorities a command id and label instead of aChoiceBase.ComplexFormVisibilityandAnthroItemFilterLinkinSIL.FieldWorks.Common.DetailRules(FdoUi).RecordEditViewimplements the new internalIReferenceItemMenuHost.RecordEditView.BuildReferenceItemMenu'soutparameter is renameditemUi; the object UI is no longer registered as a colleague for an owned menu id, only formnuEnvReferenceChoices.DataTree.DisplayJumpToToolAndFilterAnthroItemtolerates a null current slice (hidden, as for a non-anthropology field).Findings
Critical - Must address before merge
None.
Important - Should address before merge
None.
Minor - Consider
(author: they serve onlyBuildItemMenuThroughTheColleague,AddMoveCommandsand the Ctrl+click fallback duplicate the first-enabled-jump rule the authority implementsmnuEnvReferenceChoicesand are deleted in track A row 2, a separate PR)The anthropology filter jump posts(author: moved fromFollowLinkthrough the mediatorDataTree; LT-21401 Part 1 converted only the synchronous senders, so the deferred posts are converted together later; inline PR comment records it)A Ctrl+click materializes all 40 leaves to find the default jump(author: small per-click cost; an early exit needs a bridge change better made with E2b)Required Validation / Evidence
.\build.ps1 -CommentHygiene -TokenHygiene: clean..\test.ps1 -TestProject Src/FdoUi/FdoUiTests(DetailRules fixtures): 11/11..\test.ps1 -TestProject Src/Common/FwAvalonia/FwAvaloniaTests(DetailEditorParityTests, DetailMenuRequestTests): 41/41..\test.ps1 -TestProject Src/xWorks/xWorksTests(DetailObjectCommandExecutionTests, whole fixture): 54/54.Obj\Debug\Viewsholds stale objects after a views header change (LT-22674: Cache text analysis only for text NFC leaves unchanged #1166). Remedy: delete that folder and rebuild native.Positive Observations
Src/FdoUi/DetailRuleswith the reflection boundary test unchanged and passing; the WinForms handlers call the same code.Interview Notes
mnuEnvReferenceChoices) is a separate PR, not a second commit here.FollowLinkpost as a mediator call pending LT-21401's per-site timing analysis.CommandChoice.CommandObjectpublic rather than a host-sideCommandSetlookup; either is temporary until E2b.Suggested Review Focus
ReferenceItemMenuAuthority.JumpItem: the jumps are answered by calling the item's object UI directly instead of registering it on the mediator.ComplexFormVisibility.Toggle: the ordered insert must match theDTMenuHandlerloops it replaces.WithoutLeakedSubentryMark: the one divergence dropped from the colleague-path baseline.🤖 Generated with Claude Code