Skip to content

Commit 70157c1

Browse files
thejambiZachary Burnhamclaude
authored
LT-22672: Compose the Allomorphs slice, and the shared detail-view fixes it surfaced (#1135)
* LT-22672: Compose allomorph rows at legacy nesting depth The Allomorphs section composed five levels deep where legacy shows three. WalkSequence emitted a banner for the sequence node and a numbered header per item, above item rows their own layouts already label -- so an entry showed "Allomorphs", then "AlternateForms", then "AlternateForms 1", before reaching the "Stem Allomorph" row. Neither the caller part ref nor the <seq> element authors a label, so both synthesized rows fell back to the raw model field name. That also made them untranslatable: layout labels localize through StringTable, which a field name bypasses entirely. WalkSequence now suppresses both rows when the sequence sits directly under a section header that already names it and the items carry their own labelled root row. Sequences that own their only banner keep it -- Senses, whose numbering carries information the item row does not, and Pronunciations, which has no enclosing section header at all. DescendInto splits so a caller can state the child depth directly; the flattened path uses it to land item rows at the depth the suppressed banner would have taken. The Environments test is expected to fail: that row still has no editor and is the next piece of work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LT-22672: Render boolean slices as editable checkboxes Boolean fields composed as the labeled Unsupported row, so Is Abstract Form on an allomorph showed "This field is not yet supported in the new view" instead of the checkbox legacy draws. Adds DetailFieldKind.Boolean, a CellarPropertyType.Boolean branch in WalkOtherField, and the owned FwBooleanField: a checkbox whose caption is the row's own label, as in legacy CheckBoxSlice. No yes/no text is drawn, so the row carries no options and needs no new localized strings. The value stages through the existing TrySetOption as the literal "true"/"false". A boolean is a two-state choice, so it reuses the option path rather than widening IDetailEditContext -- FieldEditHandler.Option already had the right shape. A key that is not one of the two literals is refused without opening the fenced session, like a chooser key outside its list. FwBooleanField overrides StyleKeyOverride. Avalonia keys a ControlTheme on the exact type and the shared checkbox style selects OfType<CheckBox>, so without it the row got no template at all: it drew nothing and took no input. FwChooserField carries the same guard for Button. The change is shared, not allomorph-specific: 14 boolean slices across five layout files pick it up, including Active on compound rules and ad-hoc prohibitions and PublishAsMinorEntry. Four tests that pinned the previous "checkbox editing dropped" behavior are updated to the new one. Is Abstract is authored twice over the same field. The allomorph part uses editor="Checkbox" and now composes as a boolean row; the lexeme form's AllomorphStatus part uses a closed editor="enumComboBox" and stays an Unsupported row, so a closed list cannot degrade into an editor that persists invalid values. A test asserts both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: record the synthesized chooser-link divergence The detail view shows a list-editor jump ("Edit the ... list") on rows legacy leaves without one -- Morph Type among them, whose part authors only a chooserInfo title. Nothing said this was deliberate, so the next reader would have taken it for a bug and narrowed it. It diverges from legacy on two axes. Legacy synthesizes a chooserInfo only for editor="autoCustom" custom-list reference fields, and shows a jump link elsewhere only when the layout authors one; the composer applies its fallback to any possibility-list row with no authored link. Legacy also puts the link inside the chooser dialog, where the row gear promotes it to the row itself. Kept because the jump is useful on every list-backed row and the narrower legacy rule reads as an accident of which parts happened to author a link. The dispatch path is unchanged, so the jump behaves as legacy's does. Recorded per the divergence convention with the reason and the approver, in CreateChooserLinks where the fallback lives, cross-referenced from the gear that renders it, and added to the exemplar map's behavior map so the next conversion finds it without reading the composer. This behavior predates the branch; only the documentation is new. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: add the manual-finding loop to the conversion skills Both conversion skills told the developer what to manually test and stopped there. Nothing said what to do when the manual pass finds something the green suite missed -- which is exactly when the suite has a hole worth closing, not just the code. Adds the loop to both: triage which layer the defect lives in, write the missing test at that layer, verify it actually catches the defect by reverting the fix and watching it fail, re-run every suite the change reaches, and record the cause and the too-weak assertion. The verify step earns its place. A test asserting a control "renders" can pass against a control drawing nothing: an untemplated Avalonia control still measures to its padding and still has visual children. A test that has never failed proves nothing. convert-slice also picks up two things it had lost against convert-dialog despite claiming to mirror it: stop and wait for the developer's findings, and the legacy-mode smoke check -- which for a slice must name what else a shared composer or control change reaches. CONTEXT.md gains Hotlinks and Semantic snapshot, and an invariant recording that DataTree Insert commands run headlessly while Delete, Move and Merge need a laid-out visible legacy tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: draw the writing-system gutter only on multistring rows A single-alternative row showed a writing-system abbreviation beside its value, so an allomorph's custom field rendered as "Por nota" where legacy shows the value alone. Legacy labels each alternative of a MultiStringSlice and leaves a StringSlice unlabelled. DataTree.CreateEditor passed showWritingSystemAbbreviation: true for every row, so the view never consulted DetailField.IsMultiStringRow at all. The gutter now follows the row's own kind. This predates the singular-selector custom-field change (#1109). Before it, such a field composed as a multistring row and drew the gutter for that reason; after it, the model classified the row correctly and the view drew the gutter anyway. Only the route changed, never the symptom. No production row loses a label it should have: a string editor always resolves exactly one writing system, and no shipped layout pairs editor="string" with a plural ws selector. Three existing fixtures built text rows without setting IsMultiStringRow and asserted the gutter renders. They now build multistring rows, which is what they meant -- the gutter is what they are testing. IsMultiStringRow gains a second consumer, so its documentation now describes it as the row's identity rather than one feature's switch, and records that value count is not the test: a multistring row with one configured writing system still labels it. Found by manual testing after the Allomorphs work. The new tests render through DataTree rather than SliceFactory, because the model was already right and only the view could show the defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep authored chooser links on generic reference vectors AddGenericReferenceVector built its row without chooser links, so a field whose layout authors "Edit the ... list" lost the jump. The possibility-list paths already carried them; only this one dropped them. CreateChooserLinks is called without a list, so only AUTHORED links ride along. The synthesized list-editor fallback stays with the possibility-list paths, and this does not widen the approved divergence recorded there. Restores the "Edit the Environments" jump, for a project that already owns at least one valid environment (the state a real project like Sena 3 is in): with a candidate available, PhoneEnv composes through this path and now carries the authored link, confirmed live in the New UI. An earlier version of this message claimed the row never reaches this path at all -- that was traced to an unrelated test-fixture defect (an environment string referencing an unregistered phoneme, so it never validated), not to the row's real behavior; see the fixture fix for the investigation. The jump still does not reach an EMPTY project, whose row is the read-only fallback with no links at all. That waits on the environment editor, along with everything else about that row. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: record the add-only reference-vector picker divergence The "+" picker passes the field's current members as the chooser's unavailable keys, so they list greyed and un-toggleable rather than checked, and removal is the per-item right-click. Legacy's SimpleListChooser instead pre-checks the current members and treats the dialog as setting the whole membership, so unticking removes. Nothing said this was deliberate, and the inline comment claimed the picker worked "like the legacy multi-check chooser" -- which invited a future reader to "fix" it to pre-check and call that parity. Kept because "+" means add, which is what the symbol promises, and the row already owns removal. The greyed rows still show what is present, so the membership is never hidden. Recorded per the divergence convention with the reason and the approver, in the FwReferenceVectorField class doc, cross-referenced from the picker construction, and added to the exemplar map's behavior map so the next conversion finds it without reading the control. Behavior predates the branch; only the documentation and the misleading comment change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: use a phoneme-free environment string, isolated per test AllomorphEnvironmentAndEditingTests seeded the project with "/ _ a", referencing phoneme "a". The in-memory test project registers no phonemes, so PhonEnvRecognizer rejected the string, IPhEnvironmentRepository .AllValidInstances excluded it, and ReferenceTargetCandidates(PhoneEnv) came back empty even with an environment present. The Environments row fell to the read-only fallback in both data states, never the editable generic reference-vector chooser real projects compose -- so P7's "not a ReferenceVector" assertion would pass once the environment editor lands without ever having exercised the row users actually get. Switched to "/ # _", a pure word-boundary environment PhonEnvRecognizer's own test suite documents as valid with no phoneme inventory (also the pattern MorphTypeAtomicLauncherTests already uses). That surfaced a second, real bug: environments added via a non-undoable UOW are project-level state that ReferenceTargetCandidates scans in full, and TestTearDown never reversed them, so one test's environment leaked into the next test's "empty project" assumption. Switching the helper to UndoableUnitOfWorkHelper did not fix it -- probing CanUndo() and CurrentDepth after a failure showed the undo stack was already fully drained, yet the environment was still present, meaning Undo does not reliably reverse this create-then-set-property pattern in this shared-cache harness. The fixture now tracks what it creates and deletes it explicitly in TestTearDown, independent of the base class's Undo-based cleanup. Confirmed order-independent by running the fixture twice in a row. With both fixed, the "editor identity" test fails on the real defect -- the populated-project row composes as ReferenceVector, confirmed live in the app -- rather than on fixture noise, and the "chooser link" test passes, confirming the earlier chooser-link fix (5b78c19a6) does reach this row after all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * LT-22672: Let the environments picker create by typing The Environments row is a chooser AND a typed editor in legacy, not one or the other: PhoneEnvReferenceLauncher opens a SimpleListChooser over the project's environments, while its inline PhoneEnvReferenceView lets the user type a new environment string that ConnectToRealCache reconciles into the project. An earlier attempt built the typed half as a REPLACEMENT and removed the working chooser; that was reverted, and the work is preserved on wip/FwEnvironmentField. This adds the missing half to the row that already ships. The picker's filter box is already a typing surface, so it offers a "Create ..." row when the text matches no option; the row's chrome is unchanged. Two opt-in seams keep every other chooser row untouched. FwOptionChooser gains an allowCreate flag and a CreateRequested event, and the edit context gains IReferenceItemCreation, acquired with "ctx as" exactly as IStructuredTextEditing is. Keying the composer's registration on the editor string rather than on PhoneEnv means infix Positions is served by the same code -- MorphologyParts.xml uses phoneEnvReference six times, 4x PhoneEnv and 2x Position. Behavior worth naming: The create row TRAILS the matches. Leading, it would take the default highlight, so Enter on a partial query like "Sk" would mint a new "Sk" instead of picking the "Sky" the user was reaching for. Matching strips spaces, as ConnectToRealCache does, so "/ # _" and "/#_" resolve to one environment rather than creating a second. A malformed string still creates. Legacy attaches the PhEnvironment regardless of CheckConstraints and annotates it; refusing would discard the typed text. One behavior fix: the vector walker gated on having candidates, so a project with zero environments fell to a read-only row -- nothing to do in exactly the state where the user needs to type the project's first environment. Verified by reverting the gate: 4 tests fail, expecting ReferenceVector but finding Text. Not yet built: the inline annotation for a malformed item (legacy draws a squiggly). The string is stored correctly; only the display is missing. xWorksTests Avalonia 1614 passed, FwAvaloniaTests 703 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LT-22672: Annotate reference items the domain reports invalid Legacy marks an invalid environment with a red squiggly and keeps it. The Avalonia row stored such an environment correctly but drew it identically to a good one, so the only signal that a string was malformed was silence. An item now carries the DOMAIN's verdict. DetailChoiceOption gains a ValidationMessage, filled from ICmObject.CheckConstraints -- the same oracle legacy's squiggly runs on, rather than a second validity rule that could drift from it. The item renders in the error colour, underlined, with the explanation as its tooltip and automation help text; colour alone would carry the whole signal, which fails anyone who cannot see it. Avalonia has no wavy decoration, so the underline is straight. Composing must not write, and CheckConstraints can write: it mints a ConstraintFailure annotation and rewrites the string to carry the squiggly. Called with createAnnotation false and the squiggly overload untouched it computes no change and opens no unit of work, which a test pins by watching LanguageProject.AnnotationsOC and the stored string -- NOT the undo count, which would be a false green, because those writes go through a non-undoable UOW. Scope this reaches: exactly three classes override CheckConstraints -- PhEnvironment, MoMorphAdhocProhib and MoAlloAdhocProhib. Everything else inherits a base that returns true without allocating, so this stays free for every other row. Only the per-item shape is built here; the two adhoc co-prohibitions validate the ROW's own object and can reuse ValidationMessageFor when someone builds that. Verified by suppressing each half in turn: all three new tests fail, and the other seven environments tests keep passing. xWorksTests Avalonia 1616 passed, FwAvaloniaTests 704 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: highlight a chooser's current value when its list opens Opening a Morph Type chooser highlighted the first option, which reads as a claim about the row's value -- the list appeared to say "stem" was selected when the field held something else. The picker was never told what the row holds: FwChooserField kept the current key to itself and constructed FwOptionChooser without it, so ApplyFilter always landed on the first available row. The picker now takes the current key and highlights it, scrolled into view. A value far down a long list is the case where pointing at the top misleads most. Typing still wins: an empty filter highlights the current value, but once the user types, the first MATCH is highlighted, because typing is a search and Enter should take what was searched for rather than snapping back to the old value. Clearing the filter returns to the current value. The highlight moves only on a COMMITTED change. A rejected edit leaves the list pointing at what the row actually holds, so the picker cannot infer this from the commit itself -- the host calls SelectByKey inside the success branch. InsertEntryDlgViewModel already needed this and had hand-rolled it: an index search poking OptionsList.SelectedIndex. It now calls the same SelectByKey, so there is one implementation rather than a second copy in the detail view. Multi-select pickers are unaffected -- their checked set carries membership, and there is no single current value to highlight. Verified by suppressing the lookup: the four tests asserting the new behavior fail, while the two pinning the unchanged behavior (no current value, and typing beats it) keep passing. FwAvaloniaTests 710 passed, xWorksTests Avalonia 1616 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: commit a checkbox toggle immediately, like legacy Ticking Is Abstract left Ctrl+Z with nothing to undo until the user clicked away. The box only STAGED into the fenced session: SliceFactory built FwBooleanField without forwarding the context's Save, so nothing reached the undo stack until the focus-loss autosave happened to fire. A toggle is a discrete gesture, not an in-progress edit. Legacy's CheckBoxSlice writes it as it lands, and the reference vector's add/remove already commits the same way through the same gestureCompleted callback. The checkbox now does too. Only a SUCCESSFUL toggle completes the gesture. A refused edit puts the box back and commits nothing, rather than pushing an empty step onto the undo stack. Verified by removing the wiring: CommitsImmediately fails while the three tests pinning stage-and-revert behavior keep passing. FwAvaloniaTests 712 passed, xWorksTests Avalonia 1616 passed, 0 failed. Found by the developer's manual pass, which is also the only thing that could have found it -- a staged-not-committed edit looks identical in a headless test that never moves focus. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: hold the detail re-show until a click finishes Clicking a control while another row held a staged edit only focused it; a second click was needed to act. The developer isolated it precisely: typing nothing in the other field first made the click work, typing anything made it fail. That is the open-session guard on the autosave. The press moves focus, the LostFocus handler sees an open session and commits, the host re-shows, and the view rebuilds BETWEEN the press and its release -- the host schedules through WinForms BeginInvoke, which pumps between WM_LBUTTONDOWN and WM_LBUTTONUP. The release then reaches a detached control, and a CheckBox toggles on release over ITSELF, so nothing happens. Not a checkbox bug. Any click-activated control reached in one click from an edited field was affected: the chooser, the vector's add button, the gear. The commit still happens on focus loss, so one undo step per field is unchanged. Only the re-show waits, until the pointer is released. Several commits in one gesture -- the autosave of the field being left plus a commit by the control being clicked -- coalesce into one re-show. A gesture ending outside the view delivers no release here; the held re-show then rides the next release rather than being lost. PointerCaptureLost is not the net for that: it is a Direct routed event, so it fires on no Bubble registration and does not route up from the descendant that lost capture. A handler for it was written, and removed once a test proved it could never fire. The coalescing test needed rewriting for the same reason. Written as two focus-loss autosaves it passed with the fix suppressed, because the fake context reports IsOpen only until its first commit, so the second autosave never fired. It now drives two reference-remove gestures, which are not gated that way. Verified by suppressing the hold: all three tests fail. FwAvaloniaTests 715 passed, xWorksTests Avalonia 1616 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: hold the host refresh too while a click is in flight The previous commit held the re-show driven by EditCompleted, and the developer reported the bug unchanged: the +, the gear and the checkbox still needed two clicks. That fix guarded the wrong path. LcmDetailEditSession.Commit clears IsOpen BEFORE calling EndUndoTask, and EndUndoTask is what raises PropChanged. So by the time the refresh controller sees the commit's own notification, its "is the user editing?" guard reads false, and it schedules a recompose immediately -- before OnSave has even returned to raise EditCompleted. The view was rebuilt by the notification, not by the completion. A click in flight now counts as busy for that guard, exactly as an open session does, and the gesture's end releases whatever was held. DataTree publishes the gesture; the host control forwards it; RecordEditView folds it into the predicate and calls the controller's new ReleaseHeldRefresh, which delivers only what a hold actually kept -- an ordinary click still costs no recompose. Verified by suppressing the release: the holding test fails while the nothing-held test keeps passing. xWorksTests filter Detail 218 passed, FwAvaloniaTests 715 passed, 0 failed. The full "Avalonia" filter could not be used as evidence here: it aborts with a crashed test host after 59 tests. That reproduces on this commit's parent with the working tree stashed, so it is not from this branch, and no FieldWorks or stale test host was running. It wants separate investigation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: guard the whole teardown unsubscribe, not just its first line The previous commit added a second unsubscribe under a brace-less "if (m_avaloniaEntryForm != null)", so it ran unguarded and threw a NullReferenceException whenever the view was torn down without ever having built an Avalonia entry form -- a Legacy tool, for instance. The developer hit it by jumping to Morph Type through the gear and then pressing Back, and the crash dialog named the line. Braces around both unsubscribes. Audited the rest of the branch for the same shape -- a statement appended after a brace-less control statement's body, silently landing outside it -- and this was the only one. xWorksTests filter Detail 218 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep the view busy while a picker it opened is still up Holding the re-show until the pointer release fixed the checkbox, because a toggle completes on release. It did not fix the "+": the developer saw the dropdown appear and then vanish. That click OPENS a picker, and the picker is anchored to a button INSIDE the view, so releasing the held re-show at the pointer release rebuilt the view and took the anchor -- and the picker -- with it. Busy now means a click in flight OR a picker opened from this view still up, and the held re-show lands only when both are clear. So the rebuild waits until the user has finished choosing. The signal follows the seam the factory already uses for Save: the slice factory's context carries a popup-open callback, the two flyout owners report through it, and the view counts them. A host that never rebuilds passes null and nothing changes. Renamed with the meaning: IsInteractionInFlight, InteractionCompleted. The old names said pointer gesture, which is now only half of it. Verified by dropping the picker from the busy condition: both new tests fail, while the three pointer-gesture tests keep passing -- the picker rule is additive, not a replacement for what already worked. FwAvaloniaTests 717 passed, xWorksTests filter Detail 218 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: report every popup the detail view opens, not just its pickers Right-clicking an environment after typing showed Remove, which then vanished -- the same shape as the "+" dropdown, one popup further along. The previous fix threaded a callback to the two flyout owners it knew about, which is exactly how the next popup gets missed: the detail view opens seven, and five of them live in classes that never received the callback. A popup now finds the view itself. PopupReporting resolves an IDetailPopupSink by walking up from the flyout's own target, so a site reports by wiring one call and nothing has to be threaded to it. The sink is resolved on open and remembered, because the target can be gone by the time Closed fires and an unmatched close would wedge the view as permanently busy. All seven now report: both option pickers, the per-item Remove menu, the text row's context menu, and the style, writing-system and link flyouts. Verified by making the sink unresolvable: all three popup tests fail. The two written before this change still pass through the new mechanism, which is what says the walk actually finds the view. FwAvaloniaTests 718 passed, xWorksTests filter Detail 218 passed, 0 failed. Src/Common/FwAvalonia/Detail/FwFieldControls.cs was reset and the change reapplied before committing: repeated -CommentHygiene runs had auto-rewrapped pre-existing doc comments across the file, growing the diff to 267 insertions of mostly churn -- including orphaned fragments like a line reading only "/// and". The real change is 57. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * LT-22672: Create environments in the vernacular writing system An environment created through the picker's create row was tagged with the ANALYSIS writing system. Legacy types them in the vernacular: PhoneEnvReferenceView builds its editable string with m_wsVern, and an environment string is phonology in the vernacular script. Found by the developer from the rendering, not from the data. Two visually identical environments on one allomorph drew with different underscores in the legacy view -- one typed in legacy, one created here -- because a view that renders a string's own writing system picked different fonts for them. The new UI could not show that difference, which is why it survived every test so far: reference-vector items render in a plain TextBlock with no per-writing- system font, so both looked the same there. That gap is real and separate, and is NOT fixed here -- it is what hid this bug rather than what caused it. The fixture seeded its environments as analysis too, which would have made the wrong writing system look normal. It now seeds vernacular, as legacy does. Verified by putting the analysis writing system back: only the new test fails, the other nine pass either way. Environments already created through the create row keep the wrong writing system; the fix applies to newly created ones. xWorksTests filter Detail 218 passed, FwAvaloniaTests 718 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * LT-22672: Ask the domain whether a field applies before composing it Legacy asks ICmObject.IsFieldRelevant before building a slice (DetailControls/SliceFilter.cs:60). The Avalonia composer never asked, so it showed rows the domain says do not apply to that object. The developer found it: legacy drew Stem Allomorph Label on one allomorph of four, the New UI on all four. The four differed only by morph type, and MoStemAllomorph.IsFieldRelevant returns false for StemName unless the type is a root, stem, bound root, bound stem or phrase. A clitic or particle has no stem name to give. Relevance is NOT a hidden field. Show Hidden Fields was on in that comparison and legacy still withheld the row, so this is checked whatever the setting says -- the test asserts it with showHiddenFields true for exactly that reason. Checked in Walk, the single entry every node passes through, so it covers the whole surface rather than the row that exposed it. Five classes override IsFieldRelevant in liblcm and three are allomorph fields: StemName here, MoAffixForm.InflectionClasses, and MoAffixAllomorph.Position. The base returns true without allocating, so this is free for everything else. Legacy's propsToMonitor set is discarded. It exists so a live slice can re-evaluate when the property it depends on changes; this view recomposes on PropChanged, so changing an allomorph's morph type re-shows and the row appears or disappears on its own. Verified by removing the gate: the new test fails, the other 31 pass either way. xWorksTests filter Detail 218 passed, FwAvaloniaTests 718 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: let an empty chooser fill its cell so it can be clicked An atomic chooser with no value had no text to wrap around, and the control hugged its content: the click target measured SIX pixels wide. The developer found the Stem Allomorph Label picker only by noticing a thin strip that greyed on hover. It was thin in both directions -- an empty TextBlock has no width and no height -- so the control now stretches on both axes and fills the value cell, the way legacy's launcher does. Only the target grows; the text still sits left, and a chooser that has a value looks as before. Every atomic chooser row in the app is affected, not just this one. Grammatical Info looked fine only because it usually has a value to wrap around. The test measures Bounds rather than reading back the alignment property: the property would have been set correctly while the control still collapsed, which is the same false green the blank checkbox taught. Verified by restoring the hug -- the test fails with "But was: 6.0d", the developer's strip, measured. FwAvaloniaTests 719 passed, xWorksTests filter Detail 218 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: fill the cell only for a chooser with no value Filling it always was too much, as the developer pointed out: a chooser that has a value stretched past both the value and the gear, so hovering lit the whole cell. A valued chooser IS its text, and the text is target enough. Only the empty row stretches now, which is the case that was broken -- it has no text to wrap and collapsed to six pixels. The alignment re-applies when a value is picked, so a row stops filling the cell as soon as it has something to show rather than waiting for the next re-show. Considered and not taken: docking the gear to the far right, which is where legacy puts its "..." button. It would give the extra width a purpose, but only on rows that HAVE a gear -- CreateConfigureGear returns null unless the row's list resolves an editor target -- so most chooser rows would still stretch into empty space. Worth doing on its own terms as parity polish, not as this fix. Both halves are now pinned. Verified by making it always stretch again: the valued case fails with "But was: 300.0d" in a 300px cell, the behavior that was reported. FwAvaloniaTests 720 passed, xWorksTests filter Detail 218 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * LT-22672: Cover the remaining field-relevance rules the composer honours The relevance gate runs on every field node in every composed view, and only MoStemAllomorph.StemName had a test. Adds coverage for the rest of the detail-view relevant overrides: MoAffixForm.InflectionClasses, relevant only when the entry's MSAs include an inflectional affix MSA, and MoStemMsa.InflectionClass, irrelevant until a part of speech is chosen. Both were verified to fail with the gate removed. A third test, for MoAffixAllomorph.Position, does NOT exercise the domain rule: it passes with the gate removed, because the layout's own choice/where guidequals filter already excludes Position for a non-infix, so IsFieldRelevant is never consulted there. It is kept for the negative case, which was otherwise untested, and its doc says plainly what it covers so nobody reads it as coverage of the rule. That rule remains unexercised. Every test runs with showHiddenFields true. Relevance is not a hidden field -- legacy withholds an irrelevant row even with Show Hidden Fields on -- and the flag stops an ifdata row from vanishing for the wrong reason and passing by accident. VirtualOrdering also overrides IsFieldRelevant but is not shown in a detail view, so there is nothing for the composer to gate. xWorksTests Avalonia 1630 passed, FwAvaloniaTests 720 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: state comment reasoning without pointing at history or files A comment audit against the repo commenting standard found eight banned-content violations in this branch's comments and seven more of the same shape. All comment-only; no code, assertion message, automation id or resx value changed, and both suites are byte-identical after. Finding codes and an internal doc pointer: P8, P9 and P7b named items in Docs/migration/working, which is gitignored -- a fresh clone has no such file to resolve them against. One fixture summary cited "the LT-22672 test plan" the same way. The reasoning they carried is now stated where it is needed. Absence narration: a test summary explained what it "previously asserted" and that an implementation "was reverted". It now states the standing consequence -- asserting the row is not a chooser would drive an implementation that drops half of what legacy does -- which is the part a future reader can act on. A cross-file pointer, "see the class doc for the reason and approver", pointed at a link nothing checks. The reason is short enough to state in place. Consumer framing, "every consumer shares this one path", named callers that can change without the comment noticing. Seven summaries opened with "Found in the app" or "Found by the developer". Each now states the mechanism instead. The picker one is the clearest gain: "releasing the held re-show at the pointer release rebuilds the view, and these flyouts anchor to a control INSIDE it" tells the next reader what breaks if they undo the fix, where the old text told them a story about last week. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: rewrap this branch's comments so no line is a stranded word The comment-hygiene auto-fixer re-wraps over-long comment lines on every build, and its overflow lands as a line of its own. Across this branch's builds that left fragments like a line reading only "rebuild", "through" or "is" in the middle of otherwise ordinary sentences. Re-flows each affected paragraph to the 98-column limit from .editorconfig, counting a tab as four. Comment-only: no code, assertion message, automation id or resx value changed, and both suites report the same counts as before. Two limits, both deliberate. Only paragraphs every line of which this branch wrote are touched. Re-flowing one that is even partly pre-existing rewrites its lines into the diff, which drags main's own long comments into this branch's hygiene scope -- a first attempt did exactly that and put a 912-character comment from main under this branch's name. The license header is a hard boundary. Every file carries the same three lines, and in a file this branch added they all count as branch-written, so an earlier run collapsed them into two. Fragments of the same kind exist on main -- DataTree.cs had 28 short comment lines before this branch, DetailModel.cs 136 -- and are left alone. This stops the branch adding more; the backlog and the auto-fixer that produces it want their own change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Comment cleanup * Comment cleanup * LT-22672: Invert a checkbox slice that declares toggleValue A slice marked toggleValue="true" shows and stores the LOGICAL INVERSE of its property, on read and on write -- legacy's CheckBoxSlice inverts in both directions. The Boolean composer read and wrote the property straight, so those rows were backwards in both. Every shipped checkbox-editor slice declares it. "Active" is MoAdhocProhibGr.Disabled and MoMorphAdhocProhib.Disabled in four places, plus "Requires more derivation" on MoDerivAffMsa.Final and "Show Minor Entry" on LexEntry.HideMinorEntry. The visible effect was not only a wrong tick. An enabled rule showed Active CLEAR, and ticking Active then wrote Disabled true -- so a user correcting what looked like a wrong checkbox silently disabled the rule. Wrong data, not just a wrong picture. Read and write now share one XOR against the flag, so the two directions cannot drift apart the way separate branches could. The existing coverage could not catch it: AdhocCoProhibComposeTests asserted the row's KIND and never its value, and a composer ignoring toggleValue still produces a Boolean row. The other boolean tests use IsAbstract, which declares no toggleValue. The new test asserts the value in both directions and fails before this change with Expected "True", But was "False". Found by an adversarial review pass over the branch. xWorksTests Avalonia 1631 passed, FwAvaloniaTests 720 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: leave a picker's highlight alone unless it was given a value Re-highlighting on every open applied to every picker built through the shared flyout, including those that never carry a current value. Several hosts compute an index themselves and assign it in the line before ShowAt, so the open-time pass overwrote it and the list highlighted its first row instead. Affected the character-style and writing-system pickers on a rich-text row, the structured-text field's three pickers, and the complex-form type dropdown, whose view model mirrors the key separately -- so its collapsed label and stored key disagreed after a reopen. The open-time pass now applies only to a picker that was given a current value, which is the case it exists for: that picker instance is reused across opens, so its highlight would otherwise go stale. A picker without one keeps whatever its host set. The new test assigns an index and then shows the flyout, and fails before this change with Expected 2, But was 0. The four tests covering the seeded case still pass, so the behaviour this preserves is pinned from both sides. Found by an adversarial review pass over the branch. FwAvaloniaTests 721 passed, xWorksTests Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: report the popups built outside the field controls too The earlier claim that all seven popups reported themselves counted only the flyouts built in FwFieldControls. Three more open over the detail view and reported nothing: the row and gear context menus the host builds through DetailMenuFlyout, and the per-paragraph style picker and the character-style and writing-system span pickers inside FwStructuredTextField. Any of them could be up when a rebuild landed, and the rebuild dismissed it mid-choice. Wiring now happens where a popup is BUILT rather than at each owner, so a new site cannot forget it. DetailMenuFlyout.Build reports every host menu, and FwOptionChooser.CreateOptionFlyout -- the single construction path for every option flyout in the app -- reports every picker. The four owners in FwFieldControls that wired their own inherit it instead and no longer hold a teardown for it. Dropping those teardowns is safe because Avalonia closes a flyout when its anchor leaves the visual tree. A test pins that: it opens a picker, replaces the window content, and asserts the view reports itself idle again. Verified by suppressing the wiring: the host-menu and style-picker tests fail with "Expected: True, But was: False". FwAvaloniaTests 724 passed, xWorksTests filter Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: end a pointer gesture whose release never reaches the view A press set the in-flight flag from a tunnelled handler, so every press inside the view started a gesture, but only a release that bubbled back through the view ended one. A press on a spot that takes no pointer capture, released after the pointer has left, therefore left the flag set: the view reported itself busy until the user next clicked inside it, holding every refresh in between. The view now also listens for the release at the top level, added on attach and removed on detach, so a release anywhere in the window ends the gesture. A release inside the view still ends it at the view first, leaving that path unchanged; the top-level handler then finds nothing to do. Record navigation was never gated by this flag -- it calls ShowAvaloniaEntry directly rather than going through the refresh controller -- so the symptom was a stale view, not a stuck one. Verified by removing the top-level handler: the new test fails with "Expected: False, But was: True". FwAvaloniaTests 725 passed, xWorksTests filter Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: give the real reason the constraint check does not write The comment said createAnnotation false makes CheckConstraints read-only. It does not: three classes override it, and only PhEnvironment honours the flag. MoMorphAdhocProhib and MoAlloAdhocProhib call MakeAnnotation on any failure whatever the caller asked for. Nothing is wrong today -- adhoc prohibitions appear in the model as owning collections only, and this runs on reference vector items -- but that is the reason, and it is the reason a future caller has to preserve. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: give the two relevance rules a positive control Each test asserted only that a row was absent, which a row that never composes for any reason satisfies just as well. Both now compose the same fixture twice with one precondition changed -- an inflectional affix MSA with a part of speech, and a part of speech on the stem MSA -- and assert the row appears. Suppressing the gate fails both negative halves with "Expected: False, But was: True" while both positive halves keep passing, which is what says the gate is the thing removing the row. The relevance set is now allocated once per walk and cleared per node instead of per node, and its comment no longer claims the call allocates nothing. The overrides fill the set to tell a live slice what to watch; the composer recomposes on PropChanged and never reads it. xWorksTests filter Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: size the abbreviation gutter from the rows that draw one One width serves every writing-system gutter in the view, measured from the widest abbreviation the model carries. Single-string rows stopped drawing a gutter earlier on this branch but kept their say in that measurement, so a plain string row with a long abbreviation padded a column it is not in: 60px became 84.9px in the test that now pins it. The measurement now skips rows that are not multi-string, which is the same predicate the view uses to decide whether to draw the gutter at all. FwAvaloniaTests 726 passed, xWorksTests filter Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: skip the relevance gate for a cross-target condition Legacy resolves no flid for an <if>/<ifnot> whose target is not "this", so it applies no relevance gate to one (DataTree.GetFlidFromNode). The composer resolved the field name against the CURRENT object's class instead, which is a different object's field, and could suppress the whole conditional subtree on the answer. Two shipped layouts carry the form, both with target="owner": <if target="owner" field="MappingType" intmemberof="4,9,14"> <if target="owner" field="MorphoSyntaxAnalyses" lengthatleast="1"> Neither bites today -- MappingType does not resolve on the owned class, and no liblcm override claims MorphoSyntaxAnalyses -- so this has NO test that fails without it, and there is no seam to inject a layout that would give it one. It is here because the rule the composer implemented was not legacy's, and the next override added to liblcm decides whether that stays harmless. xWorksTests filter Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: commit a chooser pick, so Undo has something to revert Picking a Morph Type staged the edit and left the fenced session open. The next Undo therefore hit the session guard, which settles the pending edit and cancels the gesture (DetailEditContextHolder.OnDoingUndoOrRedo) -- so Undo appeared to do nothing except lock the change in, and only a second Undo reverted it. Reported from manual testing; it affects every chooser row in every tool, not just Morph Type. The chooser was the only interactive row kind that never received context.Save: vector, structured-text, boolean and multi-string rows all did. The rule it breaks is stated two lines below its own case in SliceFactory, in the comment explaining why a toggle commits -- "staging only leaves Ctrl+Z nothing to undo until focus moves". A pick is as discrete a gesture as a toggle. It commits only when the pick actually staged, so a refused edit and a re-pick of the value already shown push no empty step onto the undo stack. Both are covered. Verified by wiring the callback but not invoking it: the commit test fails with "Expected: 1, But was: 0" while the two negative tests still pass. FwAvaloniaTests 729 passed, xWorksTests filter Avalonia 1631 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: match typed text the way the row's domain matches it Reported by Mark on the PR. The picker filtered option names literally while the environments domain compares them with spaces stripped, so the two ends disagreed about the same text. Typing "/ _ #" filtered the stored "/_#" out of the list, the create-row guard then had nothing to suppress against, and the only row offered was Create "/ _ #". Both outcomes were wrong. When the environment existed but was not on the field, taking that row attached the existing one -- right result, but the row promised to create it, and the user never saw the option they could have picked. When it was already on the field, ReferenceCreate returned false and the create handler has no else branch, so the flyout closed and nothing was said. The row now publishes the comparison its own domain uses, and the picker runs both the filter and the create-row guard through it. One normalizer fixes both halves: the stored environment is listed (greyed, since current members ride as unavailable keys) and the create row is suppressed because the list already has what the text names. Rows that mint nothing from text publish no comparison and keep the literal matching they had. Verified by making the comparison the identity: the two positive tests fail with 'Expected: some item equal to "/_#" But was: < "Create "/ _ #"" >', which is the reported symptom, while the two guard tests still pass. FwAvaloniaTests 748 passed, xWorksTests filter Avalonia 1639 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Zachary Burnham <zachary_burnham@sil.org> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 11eebec commit 70157c1

36 files changed

Lines changed: 3403 additions & 85 deletions

‎.claude/skills/convert-dialog/SKILL.md‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,38 @@ Build conventions the result must satisfy (confirm each in the diff):
337337
mode for them.
338338
4. Legacy-mode smoke: with the toggle OFF, every launch site from the
339339
analysis document still opens the legacy dialog unchanged.
340-
5. Add any new exemplars created during this conversion to the exemplar map
340+
5. Work each manual finding through
341+
[The manual-finding loop](#the-manual-finding-loop).
342+
6. Add any new exemplars created during this conversion to the exemplar map
341343
(the promotion row from Planning the replacement, if not already landed).
342-
6. Land: comment audit against the repository standard, preflight the
344+
7. Land: comment audit against the repository standard, preflight the
343345
branch, and PR per the repo's conventions.
346+
347+
### The manual-finding loop
348+
349+
A green suite that missed a defect the developer found by looking is evidence
350+
the SUITE has a hole, not just the code. Close both.
351+
352+
1. **Triage the layer before writing anything.** ViewModel, launcher edge, or
353+
rendered view? Existing green tests narrow it in one step: if the ViewModel
354+
tests pass and the dialog looks wrong, the state is right and the defect is
355+
below it.
356+
2. **Write the missing test at the layer the defect lives in**, before the
357+
fix. A ViewModel-only suite cannot see a control that draws nothing; a
358+
render-layer defect needs a render-layer test.
359+
3. **Verify the new test actually catches the defect: revert the fix (or
360+
write the test first) and watch it FAIL.** This is not ceremony. A test
361+
asserting a control "renders" can pass against a control drawing nothing --
362+
an untemplated Avalonia control still measures to its padding and still has
363+
visual children. A test that has never failed proves nothing, and a
364+
false-green test is worse than no test because it silences the next person.
365+
4. **Fix, then re-run the owning suite AND every suite the change reaches.**
366+
A shared control, style, or seam means the blast radius is not the dialog.
367+
5. **Record it in the working documents**: the cause, the fix, and -- when the
368+
first test attempt was a false green -- which assertion was too weak and
369+
which one actually bites. That lesson is the reusable part.
370+
371+
If the finding turns out to be a scope decision rather than a defect (a
372+
control nobody planned, a behavior the plan never covered), do not absorb it
373+
silently: put it to the developer as a decision, and if they take it in, add
374+
plan items for it rather than implementing untested.

‎.claude/skills/convert-slice/SKILL.md‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,25 @@ convert-dialog: `Docs/migration/working/<SliceClass>/` holding
4444
`<SliceClass>-design.md`; detect-and-resume on re-invocation; the documents
4545
remain after completion (retention is the developer's call).
4646

47+
**Every working document opens with a LINKED ticket reference whenever a ticket
48+
is involved.** Write the Jira key as a markdown link to
49+
`https://jira.sil.org/browse/<KEY>`, never as bare text, and link the parent or
50+
epic the same way when there is one:
51+
52+
```markdown
53+
Ticket: [LT-22672](https://jira.sil.org/browse/LT-22672) "[Avalonia] Convert
54+
Allomorphs slice", parent [LT-22622](https://jira.sil.org/browse/LT-22622).
55+
```
56+
57+
These documents are gitignored and outlive the session that wrote them, so a
58+
reader arriving later has no other route back to the ticket. It applies to the
59+
analysis, test plan, design report, and state manifest alike.
60+
61+
This does NOT license ticket keys as pointers in code comments. Comment hygiene
62+
governs there: a bare finding-code such as `P1:` referring to one of these
63+
documents is a banned doc-pointer, because the document it points into is not
64+
in the repository.
65+
4766
### Citing other documents and sections
4867

4968
Every reference -- to another document OR to a section of the document you
@@ -184,8 +203,46 @@ is worse than an honest Unsupported row for preview users).
184203
1. create-integration-test against the plan (tests land per the mirroring
185204
rule: control tests in `FwAvaloniaTests/Detail/`, composer tests in
186205
`xWorksTests/Avalonia/Composer/`).
187-
2. Developer manual test in live FieldWorks, New UI on: the field at its
188-
real position, edit interactions per the analysis document, focus-loss
189-
autosave, single Ctrl+Z per save, cross-framework PropChanged refresh both
190-
directions, tool-switch mid-edit settles cleanly.
191-
3. Land the exemplar-promotion row for anything new (human-gated, same PR).
206+
2. Ask the developer to manually test -- then STOP and wait for their
207+
findings. They own the app; do not drive it or change its UI mode for
208+
them. In live FieldWorks, New UI on: the field at its real position, edit
209+
interactions per the analysis document, focus-loss autosave, single Ctrl+Z
210+
per save, cross-framework PropChanged refresh both directions, tool-switch
211+
mid-edit settles cleanly.
212+
3. Legacy-mode smoke: with the UI-mode toggle OFF, every field the change
213+
touched still behaves unchanged. A composer or shared-control change
214+
reaches far beyond the slice being converted, so name what else it reaches
215+
and have that checked too.
216+
4. Work each manual finding through
217+
[The manual-finding loop](#the-manual-finding-loop).
218+
5. Land the exemplar-promotion row for anything new (human-gated, same PR).
219+
220+
### The manual-finding loop
221+
222+
A green suite that missed a defect the developer found by looking is evidence
223+
the SUITE has a hole, not just the code. Close both.
224+
225+
1. **Triage the layer before writing anything.** Model (composer), dispatch
226+
(factory), or render (control)? Existing green tests narrow it in one step:
227+
if the composer tests pass and the row looks wrong, the model is right and
228+
the defect is below it.
229+
2. **Write the missing test at the layer the defect lives in**, before the
230+
fix. A model-only plan cannot see a control that draws nothing; a
231+
render-layer defect needs a render-layer test.
232+
3. **Verify the new test actually catches the defect: revert the fix (or
233+
write the test first) and watch it FAIL.** This is not ceremony. A test
234+
asserting a control "renders" can pass against a control drawing nothing --
235+
an untemplated Avalonia control still measures to its padding and still has
236+
visual children. A test that has never failed proves nothing, and a
237+
false-green test is worse than no test because it silences the next person.
238+
4. **Fix, then re-run the owning suite AND every suite the change reaches.**
239+
Shared code (the composer, `SliceFactory`, a shared control style) means
240+
the blast radius is not the slice.
241+
5. **Record it in `<SliceClass>-state.md`**: the cause, the fix, and -- when
242+
the first test attempt was a false green -- which assertion was too weak
243+
and which one actually bites. That lesson is the reusable part.
244+
245+
If the finding turns out to be a scope decision rather than a defect (a row
246+
nobody planned, a behavior the plan never covered), do not absorb it silently:
247+
put it to the developer as a decision, and if they take it in, add plan items
248+
for it rather than implementing untested.

‎.claude/skills/fieldworks-winforms-to-avalonia-migration/references/control-exemplar-map.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ migration burden.
6868
| Localization | `FwAvaloniaDialogsStrings` accessor + neutral resx (dialog-conversion.md §5) |
6969
| Clipboard in text fields | `IFwClipboard` seam (`Src/Common/FwAvalonia/Seams/`) |
7070
| Headless ViewModel/view tests | `FwAvaloniaDialogsTests/EntryGoDialogTests.cs` shapes; launcher-over-real-cache: `LcmLinkMsaDialogLauncherTests.cs` |
71+
| List-editor jump from a chooser row ("Edit the … list") | `DetailGearChrome.CreateConfigureGear` (row gear → `DetailLinkRequest` → `FollowLink`); links built by `DetailComposer.CreateChooserLinks`. **Approved divergence:** the detail view synthesizes the link for ANY possibility-list row the layout left linkless and puts it on the row, where legacy synthesizes only for `autoCustom` and puts it inside the chooser dialog. Reason and approver are in the `CreateChooserLinks` doc — do not "fix" it back to the legacy trigger. |
72+
| Adding to a reference-vector field | `FwReferenceVectorField`'s "+" opens `FwOptionChooser` in multi-select; the per-item right-click removes. **Approved divergence:** current members ride as UNAVAILABLE keys, so they list greyed and un-toggleable rather than pre-checked; legacy's `SimpleListChooser` pre-checks them and treats the dialog as setting the whole membership. "+" means add, and the row owns removal. Reason and approver are in the `FwReferenceVectorField` class doc. |
7173

7274
## 3. Gap register — the first implementation becomes the exemplar
7375

‎CONTEXT.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ It is intentionally not a full architecture manual. It should stay biased toward
7272
- **Cross-namespace twin**: A deliberate reuse of a legacy type name for its Avalonia counterpart (`FwAvalonia.Detail.DataTree` beside `DetailControls.DataTree`; `SliceFactory`, `MSAGroupBox`). Disambiguate with a `using` alias at the call site; never rename the type to dodge the collision. The twins cover only the half of the legacy responsibility that still fits — the Avalonia `DataTree` renders a prebuilt `DetailModel` and does not compose it.
7373
- **Slice plugin**: A custom Avalonia row editor (`ISlicePlugin`) registered in `SlicePluginRegistry` under the exact legacy layout `class=` string, so migrating a slice edits no layout XML. Registration throws on a double claim: one owner per legacy class.
7474
- **Unsupported row**: The labeled row `SliceFactory` builds when no field kind, plugin, or control matches. Unmigrated work is always visible and never silently omitted; the set of these rows is the remaining conversion worklist.
75+
- **Hotlinks**: The always-visible strip of section-level commands under a DataTree section header — the layout `hotlinks=` attribute, `DetailField.HotlinksId`, the `-Hotlinks` menu-id suffix, legacy `SummaryCommandControl`, Avalonia `DataTree.CreateHotlinkStrip`. Say **hotlinks**, not "quick links": the latter appears nowhere in the repo. Legacy paints one clickable label per command with per-command enabled state.
7576
- **Seam**: A framework-neutral substitution point that lets the Avalonia layer consume product behavior without referencing LCModel or WinForms — `IEditSession`, `IUiScheduler`, `IDetailLifetime`, `IDetailRefreshCoordinator`, `IXCoreCommandBridge`, `IRecordNavigationContext` in `FwAvalonia/Seams`, alongside boundary data contracts such as `IFwClipboard`. Product implementations live at the xWorks edge. Deliberately narrow: direct Avalonia APIs remain allowed at the UI edge, and `Seams/` holds only real substitution points — not helpers that merely live near one.
7677
- **Fenced edit session**: One `IEditSession` wrapping one LCModel undo task, opened lazily on the first staged edit and ended exactly once by Commit or Cancel. Prevents orphaned undo tasks; implemented by `LcmDetailEditSession`.
7778
- **Settle**: The single auto-save policy for an open edit session — commit if valid, otherwise roll back and notify. Invoked on navigation, window deactivate, undo, and teardown (`DetailEditContextHolder.Settle`).
@@ -88,6 +89,7 @@ It is intentionally not a full architecture manual. It should stay biased toward
8889
- User-visible UI strings belong in `.resx`, not hardcoded source.
8990
- Installer work lives under `FLExInstaller/`.
9091
- Integration tests often depend on deterministic sample data such as `TestLangProj/`.
92+
- DataTree object commands split by test environment. Insert commands are always enabled, so they run headlessly end-to-end through `MockFwXWindow` -> `XCoreMenuBridge` -> mediator -> unit of work (see `DetailObjectCommandExecutionTests`). Delete, Move, Demote, and Merge only materialize and enable when xCore display handlers can read live slice-sequence context from a laid-out, VISIBLE legacy DataTree, which headless runs do not have; those tests are `[Explicit]` and desktop-environment only. Do not write a plan or a claim that implies headless coverage of the second group.
9193
- Worktree-aware scripts are preferred because concurrent work across git worktrees is supported.
9294
- Visual Studio and MSVC toolset versions are policy-driven from `Build/FieldWorks.Toolchain.props`; never hardcode them in projects, scripts, or docs.
9395
- FLEx/Language Explorer is built with architectural boundaries, new dependencies between existing projects must be justified.

‎Src/Common/FwAvalonia/AvaloniaHostControlBase.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ public abstract class AvaloniaHostControlBase : System.Windows.Forms.UserControl
2727
/// <summary>Raised after a hosted detail view reports an edit completed (wired by the derived host).</summary>
2828
public event EventHandler DetailEditCompleted;
2929

30+
/// <summary>Raised when the hosted detail view goes idle: the click finished and no
31+
/// picker it opened is still up.</summary>
32+
public event EventHandler DetailInteractionCompleted;
33+
34+
/// <summary>Whether a click is in flight in the hosted detail view, or a picker it opened
35+
/// is still up. Rebuilding while this is true destroys what the user is interacting
36+
/// with.</summary>
37+
public virtual bool IsDetailInteractionInFlight => false;
38+
3039
protected AvaloniaHostControlBase()
3140
{
3241
FwAvaloniaRuntime.EnsureInitialized();
@@ -64,6 +73,9 @@ protected AvaloniaHostControlBase()
6473

6574
protected void RaiseDetailEditCompleted() => DetailEditCompleted?.Invoke(this, EventArgs.Empty);
6675

76+
protected void RaiseDetailInteractionCompleted()
77+
=> DetailInteractionCompleted?.Invoke(this, EventArgs.Empty);
78+
6779
/// <summary>Swaps the hosted Avalonia content and shows the control.</summary>
6880
protected void SetHostContent(Avalonia.Controls.Control content)
6981
{

‎Src/Common/FwAvalonia/Detail/DataTree.cs‎

Lines changed: 120 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail
3232
/// undo step per field, no Save/Cancel buttons. Validation failures show inline and block the
3333
/// commit; Escape rolls the session back. Without a context the view is read-only display.
3434
/// </summary>
35-
public sealed class DataTree : UserControl
35+
public sealed class DataTree : UserControl, IDetailPopupSink
3636
{
3737
private readonly IDetailEditContext _editContext;
3838
private readonly Action<string> _writingSystemFocused;
@@ -217,8 +217,41 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null,
217217
if (_editContext != null && _editContext.IsOpen)
218218
OnSave();
219219
}, Avalonia.Interactivity.RoutingStrategies.Bubble);
220+
221+
// A click on another row autosaves and re-shows, rebuilding controls between press
222+
// and release. Tunnel, to beat any descendant to the press.
223+
AddHandler(Avalonia.Input.InputElement.PointerPressedEvent, (s, e) =>
224+
_pointerGestureActive = true,
225+
Avalonia.Interactivity.RoutingStrategies.Tunnel, handledEventsToo: true);
226+
// Bubble, so descendants finish the gesture (a checkbox toggles, and may itself
227+
// commit) before the held re-show is released.
228+
AddHandler(Avalonia.Input.InputElement.PointerReleasedEvent, (s, e) => EndPointerGesture(),
229+
Avalonia.Interactivity.RoutingStrategies.Bubble, handledEventsToo: true);
230+
}
231+
232+
// A press that takes no capture, released after the pointer leaves the view, bubbles
233+
// nowhere near this control. The top level catches it, so no gesture outlives its
234+
// button and wedges the view busy.
235+
protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
236+
{
237+
base.OnAttachedToVisualTree(e);
238+
_topLevel = e.Root as Avalonia.Input.InputElement;
239+
_topLevel?.AddHandler(Avalonia.Input.InputElement.PointerReleasedEvent,
240+
OnTopLevelPointerReleased, Avalonia.Interactivity.RoutingStrategies.Bubble,
241+
handledEventsToo: true);
242+
}
243+
244+
protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
245+
{
246+
_topLevel?.RemoveHandler(Avalonia.Input.InputElement.PointerReleasedEvent,
247+
(EventHandler<Avalonia.Input.PointerReleasedEventArgs>)OnTopLevelPointerReleased);
248+
_topLevel = null;
249+
base.OnDetachedFromVisualTree(e);
220250
}
221251

252+
private void OnTopLevelPointerReleased(object sender,
253+
Avalonia.Input.PointerReleasedEventArgs e) => EndPointerGesture();
254+
222255
/// <summary>
223256
/// Re-derives everything that depends on the label column's width, from that width. The
224257
/// grid column is the source and this is its only consumer, so the three derived values
@@ -270,9 +303,21 @@ private void OnViewKeyDown(object sender, Avalonia.Input.KeyEventArgs e)
270303
/// </summary>
271304
public event EventHandler EditCompleted;
272305

306+
/// <summary>
307+
/// Whether a pointer press is in flight anywhere in this view. A host must not rebuild
308+
/// these controls while it is true: the release would reach a detached control and the
309+
/// click would do nothing.
310+
/// </summary>
311+
public bool IsInteractionInFlight => _pointerGestureActive || _openPickers > 0;
312+
313+
/// <summary>
314+
/// Raised when the view goes idle again -- the click finished and no picker it
315+
/// opened is still up -- so a host holding a refresh can deliver it.
316+
/// </summary>
317+
public event EventHandler InteractionCompleted;
318+
273319
// 14.4: no Save/Cancel buttons -- the legacy view saves as you go. The footer carries
274-
// only the
275-
// inline validation messages (a failed autosave is never silent).
320+
// only the inline validation messages (a failed autosave is never silent).
276321
private Control CreateEditFooter()
277322
{
278323
_validationBlock = new TextBlock
@@ -303,16 +348,84 @@ private void OnSave()
303348

304349
_validationBlock.IsVisible = false;
305350
_editContext.Commit();
306-
EditCompleted?.Invoke(this, EventArgs.Empty);
351+
RaiseOrDeferEditCompleted();
307352
}
308353

309354
private void OnCancel()
310355
{
311356
_validationBlock.IsVisible = false;
312357
_editContext.Cancel();
358+
RaiseOrDeferEditCompleted();
359+
}
360+
361+
// True between a pointer press and its release anywhere in this view.
362+
private bool _pointerGestureActive;
363+
// The window this view is in, while it is in one: the backstop for a release the view
364+
// itself never sees.
365+
private Avalonia.Input.InputElement _topLevel;
366+
// Pickers opened FROM this view. Their flyouts anchor to a control inside it, so a
367+
// rebuild destroys the anchor and the picker vanishes mid-choice.
368+
private int _openPickers;
369+
private bool _editCompletedHeld;
370+
371+
/// <summary>
372+
/// Raises the completion the host re-shows on -- unless a pointer gesture is in flight,
373+
/// in which case it waits for the release. The commit itself is NOT delayed; only the
374+
/// re-show is, because rebuilding mid-click destroys the control the press landed on.
375+
/// </summary>
376+
private void RaiseOrDeferEditCompleted()
377+
{
378+
if (IsInteractionInFlight)
379+
{
380+
_editCompletedHeld = true;
381+
return;
382+
}
383+
313384
EditCompleted?.Invoke(this, EventArgs.Empty);
314385
}
315386

387+
// One re-show per gesture however many commits it produced: the focus-loss autosave of
388+
// the field being left, and any commit by the control being clicked, coalesce into this.
389+
private void EndPointerGesture()
390+
{
391+
if (!_pointerGestureActive)
392+
return;
393+
_pointerGestureActive = false;
394+
DeliverWhenIdle();
395+
}
396+
397+
/// <summary>
398+
/// A click that opens a popup is not finished when the button is released -- the user is
399+
/// still choosing. Rebuilding then would close it under them.
400+
/// </summary>
401+
public void PopupOpenChanged(bool open)
402+
{
403+
if (open)
404+
{
405+
_openPickers++;
406+
return;
407+
}
408+
409+
if (_openPickers > 0)
410+
_openPickers--;
411+
DeliverWhenIdle();
412+
}
413+
414+
private void DeliverWhenIdle()
415+
{
416+
if (IsInteractionInFlight)
417+
return;
418+
if (_editCompletedHeld)
419+
{
420+
_editCompletedHeld = false;
421+
EditCompleted?.Invoke(this, EventArgs.Empty);
422+
}
423+
424+
// Always, even with no edit of our own: the commit's PropChanged may have left the
425+
// host holding a refresh that only this signal releases.
426+
InteractionCompleted?.Invoke(this, EventArgs.Empty);
427+
}
428+
316429
// Viewing parity (11.x): a header owns every more-indented row up to the next row at its
317430
// own indent or shallower; a row stays visible only while every owning header is
318431
// expanded.
@@ -670,7 +783,9 @@ private Control CreateEditor(DetailField field, string automationId)
670783
linkRequested: _linkRequested,
671784
clipboard: _clipboard,
672785
save: _editContext == null ? (Action)null : OnSave,
673-
showWritingSystemAbbreviation: true,
786+
// Legacy labels each alternative of a MultiStringSlice and leaves a StringSlice's
787+
// single value unlabelled, so the gutter follows the row's own kind.
788+
showWritingSystemAbbreviation: field.IsMultiStringRow,
674789
wsAbbrevColumnWidth: _wsAbbrevColumnWidth));
675790
}
676791
}

0 commit comments

Comments
 (0)