Repository navigation
Conversation
A multi-string field's Writing Systems menu offers more writing systems than the Avalonia composer would resolve, so choosing one the project had not checked did nothing, and then clearing the others showed every writing system instead of the one chosen. Each detail view also worked out the menu for itself, four rules apiece, free to drift. FieldWritingSystemOptions now answers what a field may offer, what it shows and what may be switched off. Both views call it: the WinForms slice for its menu and its visible set, the Avalonia composer for what a multi-string row renders. The Avalonia menu still runs on the old path; that part follows. Two visible changes, both in the Avalonia detail view. Choosing a writing system the project has not checked now shows it in the row, where before nothing happened. Show all right now reveals every option rather than only the checked ones, as the WinForms slice does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NUnit Tests 1 files ± 0 1 suites ±0 13m 36s ⏱️ +3s Results for commit 16784be. ± Comparison against base commit a6d34bd. This pull request removes 2 and adds 27 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
The multi-string label menu was the last shared menu group still routed through the hidden WinForms tree. Its Writing Systems submenu was populated by asking the mediator, which only the hidden adapter's slice could answer, and each toggle dispatched there and copied the result back into the view override afterwards. A new authority owns that menu on multi-string rows. It delegates the Field Visibility, Move Field and Help leaves to the per-object authority and answers the writing-system list from the shared rule the previous commit introduced. Toggles, Show all right now and Configure now write the view override directly, and a Pronunciation row also keeps the project's current pronunciation writing systems in step, through a rule the WinForms slice shares. The bridge learned to take a list-populated submenu's items from an authority instead of populating it through the mediator. Counted from the shipped configuration, 111 of the 118 multi-string parts bind either no menu or the empty Help menu, so their label menus now build without the hidden adapter at all. The other seven keep it only for their own menu id. Rules worth knowing. The authority claims the id only on a genuine multi-string row; any other row that binds it keeps the mediator path. The interceptor's writing-system items are gone, since no leaf reaches them any more. Shared rules read the model; the one write, the pronunciation list, joins a unit of work that is already open rather than failing inside it. Configure treats the same set in any order as no change. A row renders its selection in option order, whatever order it was stored in, so re-checking a writing system never reorders the rows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mark-sil
changed the title
papeh
reviewed
Oct 1, 2026
papeh
left a comment
papeh
left a comment
Contributor
There was a problem hiding this comment.
Looks good overall. Comments so far (two files yet to review)
| // The Writing Systems menu offers writing systems the project has not | ||
| // checked. Render from the SAME rule, or a chosen one cannot appear. | ||
| var spec = WritingSystemSpecOf(node, hvo); | ||
| systems = revealed |
Contributor
There was a problem hiding this comment.
I would expect revealed ? shown : options, but I may be misunderstanding something
Contributor
Author
There was a problem hiding this comment.
revealed means the row is under "Show all right now", so it renders every option; otherwise it renders the shown set. The name reads as if it were the revealed set rather than the row's state.
Contributor
There was a problem hiding this comment.
renaming revealed to showAllWss would help clarify.
papeh
reviewed
Oct 2, 2026
papeh
left a comment
papeh
left a comment
Contributor
There was a problem hiding this comment.
Looks good overall. One question in DetailComposer; most other comments are about wording.
Assert messages now state the expectation rather than the outcome, and one that described the code's history now describes the behaviour. The importer test for a part without optionalWs is renamed for what it asserts, and the comments say "expand" rather than "widen" and drop a mention of the WinForms slice. The menu authority computes a toggle's resulting set when the item is clicked rather than for every option on each menu open. The transition test that compares the native menu with the mediator path says that it is deleted with that path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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>
ReferenceItemMenuAuthority arrived from main without the BuildList member this branch adds to IDetailMenuAuthority, so the merged tree did not compile. mnuReferenceChoices has no list-populated submenu, so the authority refuses every list, as ObjectMenuAuthority and ReorderVectorMenuAuthority do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
papeh
approved these changes
Oct 6, 2026
papeh
left a comment
papeh
left a comment
Contributor
There was a problem hiding this comment.
One more comment, then LGTM.
| // The Writing Systems menu offers writing systems the project has not | ||
| // checked. Render from the SAME rule, or a chosen one cannot appear. | ||
| var spec = WritingSystemSpecOf(node, hvo); | ||
| systems = revealed |
Contributor
There was a problem hiding this comment.
renaming revealed to showAllWss would help clarify.
This was referenced Oct 7, 2026
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
3 participants
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.
Start here:
Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs, thenSrc/FdoUi/DetailRules/FieldWritingSystemOptions.cs, the rule it answers from.On a multi-string row in the Avalonia detail view, the Writing Systems submenu is now built and executed natively. Toggles, Show all right now and Configure write the view override directly, a Pronunciation row also keeps the project's pronunciation writing systems in step, and nothing in that menu reaches the hidden WinForms tree. Both detail views take the option list, the default set, the checkmarks and the cannot-empty rule from one shared rule, which also fixes the Avalonia composer rendering a narrower set than its own menu offered.
Two commits, reviewable in order: the shared rule and the WinForms rewire (f3fa244), then the native menu (114ed0a).
The question you arrive with is whether the legacy UI moved. It did not. Equivalence tests compare the rule with the slice's own writing-system query across four shapes, and the one place the first commit did change the slice, rendering a stored selection in stored order, is reverted in the second: rows render in option order, as the slice always did.
Where to look
MultiStringMenuAuthority.Ownsclaims the id only on a genuine multi-string row. The Reversal Entries row binds the same id and keeps the mediator path. Contract test inDetailObjectCommandExecutionTests.RecordEditView.ShowWritingSystemsis the one write path for toggles and Configure. The pronunciation sync runs only when every chosen id resolves, so a stale id can never reset the project list. Toggle tests reduce and restore the set.XCoreMenuBridge.ConvertOwnedSubmenutakes a list-populated submenu fromIDetailMenuAuthority.BuildList; the bridge's not-supported path is gone. One bridge test.PronunciationWritingSystems.Syncis the only shared rule that writes. Both slices call it, and it joins a unit of work already open.FieldWritingSystemOptions.Selectreturns option order. Its test was renamed to say so.Not here: the Configure dialog stays WinForms, its conversion being outside LT-22691; the host and composer each build the rule's spec from the same four facts, kept because unifying them is a layering decision for FwAvalonia; the Reversal Entries row keeps the mediator path; LT-22777, the always-show-data rule, is a composer change. Details below.
Verification: build and both hygiene gates clean. Targeted suites in the table below; full suite not run. Six manual scenario groups by the author on Sena 3, in both views; details below.
Next: approve, or tell me to split the two commits into separate PRs.
Reading this a year from now -- start here
This branch is one step of LT-22691, which retires the invisible WinForms
DataTreethat still answers the Avalonia detail view's context-menu commands. The work goes one menu id at a time, and each id needs its display and execution rules answered from the Avalonia row alone.mnuDataTree-MultiStringSlicewas the last shared menu group on the hidden path. Counted from the shipped configuration, 111 of the 118 multi-string parts bind either no menu or the empty Help menu, so after this PR their label menus build without the hidden adapter at all. The other seven keep it only for their own menu id.The first commit makes the rule behind the Writing Systems submenu exist somewhere both UI stacks can reach, and rewires the WinForms slice to it. The second owns the menu. They are separately reviewable, and the first also fixes a live defect on its own.
Decisions, and why
Five design questions were settled before the second commit was written. (1)
IDetailMenuAuthoritygainsBuildList(menuId, listId)for list-populated submenus, since aListPropertyChoicehas no configuration node and no help id to answer from. (2) The native toggle writes the JSON view override only, never the mediator's selected-writing-systems property. (3) The shared rule lives inSrc/FdoUi/DetailRules/. (4) The WinFormsConfigureWritingSystemsDlgis reused for now. (5) A new authority owns the id and delegates the six shared leaves to the per-object authority rather than extending it.Authorities partition by row context, not one per menu id. Measured across the in-scope ids: 110 ids, 77 distinct messages, 71 ids using one message or none. Expect a handful of authority classes in total.
The option list is built from the same writing-system queries the WinForms view makes, argument for argument. Equivalence by construction rather than by inspection. This is why the rule takes the object and the force-English flag.
A spec naming no writing-system set offers nothing, deliberately unlike the render-side resolver, which falls back to the analysis set. Without the guard, every part that omits
optionalWs, which is all but one, would offer the analysis writing systems.Shared rules read the model. The one write, the pronunciation list, is its own rule, and it joins a unit of work already open rather than failing inside one. Seeding the project's pronunciation list stays with the composer; putting it in the shared rule made a WinForms Pronunciation row gain a writing system and broke a render baseline.
Configure treats the same set in any order as no change, and a row renders its selection in option order whatever order it was stored in. xCore appends a re-checked writing system at the end of the stored list, so rendering in stored order made a WinForms row reorder after leaving the entry and coming back.
Single-writing-system rows keep the old resolver. They have no Writing Systems menu and collapse to one writing system regardless.
Surprising findings
The WinForms slice always shows a writing system whose alternative holds data, checked or not; the checkmarks govern only empty alternatives. Nothing on this branch touches that, but it made the manual test look broken: three rows shown with one checked, and unchecking a writing system that held data did not remove its row. The Avalonia composer shows exactly the selection and hides data. That is LT-22777, whose Show-all half this PR fixes and whose data-bearing half remains.
The two views do not share a stored selection. WinForms persists a row's choice as a
visibleWritingSystemsattribute in the project's.fwlayoutfile; the Avalonia view persists it in the.viewoverride.jsonfiles beside it. A toggle in one view never shows in the other. The one thing both write is the project's pronunciation list, under LT-9620.Seeding the pronunciation list is not a neutral read. Folding it into the shared rule grew a WinForms Pronunciation row by one writing system and three pixels. A render baseline caught it.
An empty stored selection is unreachable through the UI. The never-blank fallback cannot override a deliberate "show nothing" choice because three guards block an empty selection: the project writing-systems dialog refuses to close, the per-field Configure dialog's list box refuses the last uncheck, and the menu does not offer the last shown entry for unchecking. The trap when checking this is that the Configure dialog's guard lives in the list box's item-check handler, not beside OK or the selection property.
The Reversal Entries row's hidden-tree target is another row. Its field is a virtual property the metadata cache does not know, so the host's slice match falls back to the first realized slice on the sense. The Writing Systems submenu it shows on the mediator path is that other row's. Pre-existing, recorded on the plan.
Paths not taken
Having the bridge walk the menu XML itself for an owned id, instead of going through
ChoiceGroup. This is the agreed long-term shape and is booked as the first item of the next stage, because it changes the authority contract and every authority written before it must migrate. Not for this stage.Teaching
ChoiceGroupa non-mediator population path for list submenus. Rejected: it puts Avalonia knowledge into xCore, which is being retired.Extending the per-object authority to own this id too. Rejected: ownership is per menu id, and the per-object authority would then claim an id on rows where it is not a multi-string row's menu.
Narrowing the menu to match the composer. Loses parity: the Pronunciation field would stop offering the vernacular writing systems and every project would stop offering its active-but-unchecked ones.
Carrying one spec object on the row to remove the duplicate spec builder. The spec type lives in FdoUi and the row type in FwAvalonia, which has no LCModel or FdoUi reference and is kept apart from DetailRules by a boundary test in the other direction. Whether FwAvalonia may depend on LCModel is a decision for the whole Avalonia model layer.
Rewiring the Reversal Entries row. It binds the id but is not a multi-string row, and routing it through the shared rule would narrow what it shows.
Deferred, and what would unblock it
Evidence
Build and hygiene:
./build.ps1 -CommentHygiene -TokenHygieneclean, 0 warnings, 0 errors, after each commit.Targeted suites, final run of the second commit before the row-order fix, then the suites touching the rule re-run after it:
The full suite was not run. Native tests were skipped; no native code changed.
Manual scenarios, performed by the author on Sena 3 and reported passing. Avalonia detail view unless stated.
Review details
First commit: findings, interview notes and the validation log are in
.review/summary.mdon the author's working copy;.review/is gitignored. Six findings raised and fixed, one retracted after the code disproved it, one accepted as a deliberate decision. Second commit: a high-effort agent review raised seven findings; six fixed (a displaced doc comment, two per-object authorities built per menu, Configure treating a reordered set as a change, the pronunciation write using the unit-of-work form that fails inside an open task, the sync resolving through a fallback, the dead interceptor path), one skipped (the duplicate spec builder, above). The author reviewed every line before each commit.🤖 Generated with Claude Code
This change is