LT-22672 Allomorphs - Environments inline editing by thejambi · Pull Request #1152 · sillsdev/FieldWorks

thejambi · 2026-09-23T13:51:36Z

An environment already on an allomorph can now be edited in place: click it,
type, tab away. A new one can be typed at the end of the row without opening the
chooser. Until now the Avalonia Environments row was a chooser only -- add and
remove, no caret anywhere.

Before reading the diff: editing an environment is not a text edit. Its
identity is its string with literal spaces removed, so /_# to / _ # creates
nothing and moves no reference -- it renames the shared PhEnvironment, and
every other allomorph pointing at it shows the new spelling. Change the stripped
text and the reference re-points instead, creating a target only if the project
hasn't one. All five cases follow from that rule, which PhoneEnvReferenceView
has always implemented and never tested. Review this as reference
reconciliation, not as a text box.

Where to look

Respacing reaches the whole project. Deliberate parity, pinned by a test
asserting from a second allomorph.
Resolution prefers an unclaimed match already on the field, so retyping one
item cannot steal another item's environment.
Staging waits for the edit to finish, not per keystroke: every stage
reconciles, so /_zz would otherwise mint three strays.
Clearing an item's text removes it, whitespace included, creating nothing.
Backspace and Delete no longer remove the focused item on a retypable row.
That binding came with LT-22691: Menus for reference-vector rows in the Avalonia detail view #1133; on an editor those keys are text editing.
The row is now a WrapPanel, reaching every reference-vector row in the
view, not only Environments.

Deliberately not here

The five insert commands and the caret seam they need -- proposed for
LT-22691, where the menu code already lives.
Right-click on an item editor is inert: the item menu needs that same caret
seam, and a menu that cannot honour half its commands is worse than none.

The full xWorksTests Avalonia filter was not run. It crashes its test host
after 67 tests, in the BulkEdit tests -- reproduced identically on a clean tree
with this branch stashed, so pre-existing. It does mean the composer tests here
have only run under a narrowed filter.

Reading this a year from now -- start here

The design notes for this work lived in Docs/migration/working/Allomorphs/,
which is gitignored. They were never merged, by intent. What was worth keeping
from them is below, rewritten rather than pasted.

The single most useful fact, if you are here because something about
environments is behaving oddly: an environment's identity is its text with
literal spaces removed (RemoveSpaces is s.Replace(" ", null) -- spaces
only, not tabs). Nearly every surprise in this area follows from that.

What editing an environment does, in all five cases

Read from PhoneEnvReferenceView.ConnectToRealCache, its FindPhoneEnv helper
and EnvsBeingRequestedForThisEntry. No test in the tree pinned any of it
before this branch -- PhoneEnvReferenceViewTests contains a single test, and
only the removal row below is even adjacent to it.

Edit	Result
`/_#` to `/ _ #` (stripped text unchanged)	Nothing created, reference does not move. The shared `PhEnvironment` is renamed, so every other allomorph referencing it displays `/ _ #`.
`/_#` to `/_a`, already in the project	Reference swaps to the existing `/_a`. The old environment is untouched and stays in the inventory.
`/_#` to `/_zz`, not in the project	A new `PhEnvironment` is created in `PhonologicalDataOA.EnvironmentsOS` and the reference swaps to it.
Two items on one allomorph whose stripped text matches	Resolution refuses to let both claim the same environment while an unclaimed match exists; with none free, both land on the first match.
Anything malformed	Created and assigned regardless. Constraint checking drives the squiggle only -- nothing is blocked, corrected or discarded.
Cleared to nothing, or to whitespace	The item is removed from the allomorph and nothing is created. The `PhEnvironment` stays in the project, since other allomorphs may still reference it.

The removal row is the one most easily missed when converting: it is not in
ConnectToRealCache at all, but in the helper deciding which lines the commit
even considers. Ours rejected blank text one layer higher up, and had it not,
would have minted an empty environment -- the guard was hiding a second defect.

Two details that shaped the implementation:

The write-back is unconditional. Not "if changed": every commit writes the
typed string onto the resolved environment. The authoring comment gives the
reason -- "Maybe the ws has changed, so change the real env in database, in
case" -- so it carries writing-system changes, not only spelling.
Resolution prefers what is already on this field, then the whole project,
skipping anything claimed by an earlier item in the same commit.

Row 1 is the one to be deliberate about: editing an environment's spacing on one
allomorph silently re-spells it everywhere. Decision: replicate it. Diverging
would make the two views disagree about shared data, which is worse than
reproducing a quirk. Recorded here because a user who hits it will report it as
a bug, and the answer should be "matched deliberately" rather than a fresh
investigation. It is very likely the mechanism behind near-identical duplicate
environments seen in the wild.

Decisions, and why

The row stays a reference-vector row; only its item rendering changes. An
earlier attempt replaced it with a bespoke control and lost the chooser; it was
reverted for that reason. The picker, Remove, the item menu, reordering and
per-item validation all already worked and were all wanted.

IReferenceItemCreation became IReferenceTextEditing. It now covers
creating a target from typed text and retyping an existing item, which is the
capability-area naming IStructuredTextEditing already uses. Acquired by
ctx as IReferenceTextEditing rather than widening IDetailEditContext, so
every other row is untouched by it.

Items are addressed by key, not index. PhoneEnv is a reference
collection (card="col"; only infix Position is card="seq"), so an index
means no more than "wherever it sat when the row was composed".
ConnectToRealCache sidesteps this by rebuilding the whole vector on commit;
addressing by key is the same guarantee for a single edit.

The selection highlight paints only a label. An editor shows its own focus,
and painting its background fights its chrome. Editors are tab stops so the
keyboard walks the items; labels stay out of the tab order, as before.

Focusing the typed slot clears the row's current item, so a menu request
raised from the slot carries no item rather than a stale one.

Two layout defects, and three wrong diagnoses before them

Reported as one symptom -- an environment losing its last character -- which is
why it took so long to separate.

The item editor measured its own text short. A TextBox derives its width
from its content, and that width came out about a character narrower than what
it then drew. FloorWidthToText measures the text with FormattedText, the way
a TextBlock does, and holds the box to at least that. The first measure waits
for the box to be in the visual tree, because font size and family arrive with
the theme, and it repeats on TextChanged so the box keeps up while typing.

The row never wrapped. Found while chasing the first. PhoneEnvReferenceView
puts every item in ONE Views paragraph, which breaks to a new line when it runs
out of width. The row was a horizontal StackPanel, which arranges each child at
its desired width whatever the row's width is, so items continued past the right
edge and were cut there. Measured at a row width of 150, the third item was
arranged out to 169.

Three diagnoses were tried and discarded first, each of which looked sound:

Tried	Why it was wrong
Flat editors cost less width, so the row fits again	Moved the threshold; removed no limit.
A content-sized TextBox leaves no room for the caret past the last glyph	Measured: 21px of presenter space for 17px of text. The presenter already reserves the caret. Widening the padding changed nothing.
The row overflows, so the last item falls off the edge	A real defect, and fixed -- but not the reported one, which had room to spare on the row.

What eventually worked was not more analysis: it was forcing an unmissable
change -- an absurd 180px minimum width on every item -- which separated three
possibilities in a single glance (were builds reaching the application at all,
was width the cause, was the text complete). Every earlier round rested on "no
change", which cannot distinguish those. Worth reaching for first, not fourth.

What the tests can and cannot pin

Headless Avalonia shapes text with a stub of uniform advances rather than real
font metrics, so a TextBox's measurement matches its rendering there and the
clipping defect cannot arise in a headless test. The tests therefore pin the
invariants rather than the mechanisms:

an item editor is never narrower than its own text;
no item is arranged past the row's own width.

Both were confirmed by suppression -- removing the floor fails with a MinWidth
of 0 against a text measuring 20; reverting the panel fails reporting an item
reaching x=169 in a row 150 wide.

The five behaviour cases are tested at the composer/edit-context layer, where
the behaviour actually lives, plus a sixth rejecting a key the field does not
carry and a seventh pinning that the row displays the whole string
representation rather than an abbreviated ShortName.

What this does NOT authorize

It does not settle how a reference-vector row should behave when a single item
is wider than the whole row. It wraps between items, not within one.
It does not make every vector row retypable. CanEditReferenceItemText is
asked per field, and only the environments row answers yes.
The unconditional write-back is matched parity, not a considered design for
new code. Do not cite it as precedent.

This change is

The Environments row could add and remove, never change. Raised by Mark K: an environment already on the field cannot be edited, and the five insert commands have no caret to insert at. This is the domain half. An environment is identified by its text with spaces stripped, and every case follows from that: - Text stripping to what the item already names leaves the reference alone and writes the new spelling onto the shared PhEnvironment, so every allomorph referencing it shows it. That reach is surprising and it is what PhoneEnvReferenceView.ConnectToRealCache does; matched deliberately rather than softened, so the two views cannot disagree about shared data. - Text stripping differently re-points the item, creating the target only when the project has no match. - Resolution prefers a match the field already carries that no OTHER item claims, which stops retyping one item from stealing another's environment. - A malformed string is staged and kept verbatim, never corrected. Addressed by item key, not position: PhoneEnv is a reference COLLECTION, so an index means only "wherever it sat when the row was composed". ConnectToRealCache sidesteps this by rebuilding the whole vector; addressing by key is the same guarantee for a single edit. IReferenceItemCreation becomes IReferenceTextEditing, since it now covers creating from typed text AND retyping an existing item -- the capability-area naming IStructuredTextEditing already uses. Eight references, all in-repo. Six tests, none trusted until falsified. Dropping the write-back fails the shared-rename case with the other allomorph still reading "/_#"; ignoring what other items claim fails the no-stealing case. Nothing in this area had a test before: PhoneEnvReferenceViewTests covers one unrelated edge case. Editable items in the view are the next commit; the insert commands need the caret seam and belong with the menu work in LT-22691. FwAvaloniaTests 748 passed, xWorksTests filter Avalonia 1648 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The view half of retyping an environment. A row whose edit context reports it can reconcile typed text renders its items as text boxes instead of labels, so an item already on the field can be changed and not only removed and re-added. Every other vector row is untouched: the capability is asked for per field, and a row that does not claim it keeps the labels it had. Staged when an edit FINISHES -- focus leaves, or Enter -- not per keystroke, which is the opposite of how the text rows work and is deliberate. Each stage reconciles against the project, so staging every keystroke of "/_zz" would leave environments behind for "/", "/_" and "/_z". The handler sits on the box, so it runs before the view's focus-loss autosave commits. Three knock-on decisions: - Backspace and Delete no longer remove the focused item on such a row; on an editor those keys are text editing. Removal stays on the item menu. - The selection highlight paints only a label. An editor shows its own focus, and painting its background would fight its chrome. - Editors are tab stops, so the keyboard walks the items; labels stay out of the tab order as before. Five tests, falsified by forcing the row to render labels: the editor test fails on the missing box and the three staging tests follow. The helper that finds an editor now asserts rather than returning null, so that failure reads as "rendered no editor" instead of a NullReferenceException further down. FwAvaloniaTests 753 passed, xWorksTests filter Avalonia 1648 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

PhoneEnvReferenceView keeps an always-present empty line at the end of its view, so a new environment is typed rather than chosen. The Avalonia row had only the "+" picker. This adds the slot; the picker stays, because it is the other route and not a worse one. No new domain work: the slot commits through TryCreateAndAddReferenceItem, which already exists and is what the picker's create row calls. Committed when the edit finishes rather than per keystroke, for the same reason retyping is -- each commit reconciles against the project. It also gives an empty row somewhere to start. Until now an Environments row with nothing on it offered only the "+" button and a thin separator bar. The slot names no item, so focusing it clears the row's current one. Without that, a menu request raised from the slot would carry whichever item was clicked beforehand and act on that instead -- reproduced by suppressing the clear, which fails the new test with 'Expected: null, But was: "e1"'. Its own right-click menu stays inert for now. The host resolves an item menu through the selected key and returns early without one, so nothing opens. Making a caret-bearing, item-less request build the insert commands belongs with the menu work in LT-22691. Five more tests, falsified by suppressing the clear and by forcing the slot off. FwAvaloniaTests 758 passed, xWorksTests filter Avalonia 1648 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The new-item slot borrowed DataTree.PickerMinWidth, which is 180 and sizes a dropdown's selection panel. Inline among items reading "/_#" it dwarfed them. Its own token instead, at 70 -- a little wider than the writing-system gutter, and only the empty size: the slot grows with what is typed. FwAvaloniaTests 758 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Each item editor carried the default TextBox border and fill, so it cost more width than the label it replaced and a row that used to fit started clipping at its right edge. Every other editor in this view is already flat -- no border, no background -- and these now match. The row itself has never wrapped or scrolled: it is a horizontal StackPanel, so enough items have always overflowed and been clipped. Flat editors put that threshold back roughly where the labels had it, but they do not raise it. FwAvaloniaTests 758 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

FwReferenceVectorField was a horizontal StackPanel, which arranges every child at its desired width whatever the row's own width is. Once the items exceeded the value column they carried on past the right edge and were cut there, mid-item if that is where the width ran out -- reported as an environment losing its last character. PhoneEnvReferenceView puts every item in one Views paragraph, which breaks to a new line at the edge instead. A WrapPanel is the same thing: only Orientation was ever used, and WrapPanel has it. The test arranges five items in a row too narrow for them and asserts that none is arranged past the row's own width. Reverted to StackPanel it fails, reporting an item reaching x=169 in a row 150 wide. This reaches every reference-vector row, not only Environments. All of them could be cut this way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A TextBox derives its width from its content, and that width comes out about a character narrower than what it then draws, so an environment lost its last character -- at any row width, focused or not. FloorWidthToText measures the text with FormattedText, the way a TextBlock does, and holds the box to at least that. The first measure waits for the box to be in the visual tree, since font size and family arrive with the theme, and it repeats on TextChanged so the box keeps up while typing. The typed slot takes the same floor above its empty width. Suppressing the floor fails the test, reporting a MinWidth of 0 against a text that measures 20. Headless cannot reproduce the defect itself: Avalonia's headless platform shapes text with a stub of uniform advances, so a TextBox's measurement matches its rendering there. The test pins the invariant rather than the mechanism -- an editor is never narrower than its text. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The reason recorded on the comment was that an item costing more width than the label it replaced would make a row that fitted start clipping. The row wraps now, so it does not, and the reason was never the whole one: the editors are flat because every other editor in this view is. No behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

PhoneEnvReferenceView removes a blank line rather than keeping it: EnvsBeingRequestedForThisEntry drops any line whose text trims to nothing, and the rebuilt vector then goes back without it. Its one existing test pins the same thing from the other side -- three cached lines, one with text, one result. Ours could not do this at all. TrySetReferenceItemText rejected blank text outright, and had it not, the handler would have fallen through to find-or-create and minted an empty PhEnvironment for the item to point at. So the guard was hiding a second defect behind it. Blank now removes the item and creates nothing. Whitespace alone counts as blank, matching the Trim. The environment itself stays in the project, since other allomorphs may still reference it. Suppressing the removal fails exactly the three cases that cover it and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Five rules from converting the Environments row, and the evidence they came from. Two are worth naming here: a gesture's handling may live in a helper that filters what the write-back ever sees, so reading the commit path alone produces a model that looks complete and is not; and a guard that rejects an input can conceal what the guarded path would have done with it, which is how an empty domain object nearly got created. Status is left as proposed, and human review as pending, since the area README reserves that judgement for a reviewer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…-inline-editing

main added TryResetReferenceOrder to IDetailEditContext while this branch was out. FakeTextEditing implements that interface, so the merge does not compile -- and a local build on the un-merged branch could not see it, since the member did not exist there. CI builds the merge, and failed at the build step with every test skipped. Returns false, as InMemoryDetailEditContext and the other minimal fakes do. Nothing here exercises the order reset. Full suite on the merged branch: 6227 run, 6165 passed, 62 skipped, none failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-09-23T15:55:16Z

NUnit Tests

1 files ± 0 1 suites ±0 12m 49s ⏱️ -12s
6 240 tests +23 6 155 ✅ +23 85 💤 ±0 0 ❌ ±0
6 249 runs +23 6 164 ✅ +23 85 💤 ±0 0 ❌ ±0

Results for commit 45419b7. ± Comparison against base commit 7f93de0.

codecov-commenter · 2026-09-23T15:56:04Z

Codecov Report

❌ Patch coverage is 82.51121% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.93%. Comparing base (7f93de0) to head (45419b7).

Files with missing lines	Patch %	Lines
Src/Common/FwAvalonia/Detail/FwFieldControls.cs	81.29%	20 Missing and 6 partials ⚠️
Src/xWorks/Avalonia/Composer/DetailComposer.cs	88.00%	3 Missing and 6 partials ⚠️
...rks/Avalonia/Composer/ComposedDetailEditContext.cs	42.85%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1152      +/-   ##
==========================================
+ Coverage   38.91%   38.93%   +0.02%     
==========================================
  Files        1519     1519              
  Lines      352403   352605     +202     
  Branches    40622    40658      +36     
==========================================
+ Hits       137126   137286     +160     
- Misses     185990   186018      +28     
- Partials    29287    29301      +14

Files with missing lines	Coverage Δ
Src/Common/FwAvalonia/FwAvaloniaDensity.cs	`96.55% <100.00%> (+0.06%)`	⬆️
...AvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml	`100.00% <100.00%> (ø)`
...rks/Avalonia/Composer/ComposedDetailEditContext.cs	`54.44% <42.85%> (-0.98%)`	⬇️
Src/xWorks/Avalonia/Composer/DetailComposer.cs	`69.98% <88.00%> (+0.62%)`	⬆️
Src/Common/FwAvalonia/Detail/FwFieldControls.cs	`83.24% <81.29%> (-0.44%)`	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Zachary Burnham and others added 12 commits September 22, 2026 12:59

Merge remote-tracking branch 'origin/main' into LT-22672-environments…

d760748

…-inline-editing

Navigation Menu

Uh oh!

LT-22672 Allomorphs - Environments inline editing - #1152

LT-22672 Allomorphs - Environments inline editing#1152
thejambi wants to merge 12 commits into
mainfrom
LT-22672-environments-inline-editing

thejambi commented Sep 23, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

codecov-commenter commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

thejambi commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Where to look

Deliberately not here

Uh oh!

github-actions Bot commented Sep 23, 2026

NUnit Tests

Uh oh!

codecov-commenter commented Sep 23, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thejambi commented Sep 23, 2026 •

edited

Loading