LT-22672 Allomorphs - Environments inline editing

Docs/lessons/avalonia-migration/README.md

@@ -15,6 +15,7 @@ tree and the live legacy behavior.
 | Browse virtualization; stable selection; clerk sorting/filtering; bulk edit; RDE; accessibility; activation breadth | [Browse-table activation](browse-table-activation.md) |
 | Picture editing; properties dialog; dormant view-models; localization pair removal; exchange DTO lifetime | [Avalonia picture editing](avalonia-picture-editing.md) |
 | Options-only utilities; features with no WinForms counterpart; parity divergence cost; entry-point unwinding | [Lexicon feature manager](lexicon-feature-manager.md) |
+| Inline editing of reference-vector items; gesture characterization beyond the write-back path; blank input as removal; input guards masking defects; shared-object edits; text-metric and wrapping defects | [Reference-row inline editing](reference-row-inline-editing.md) |
 
 ## How to use these records
 

Docs/lessons/avalonia-migration/reference-row-inline-editing.md

@@ -0,0 +1,111 @@
+# Inline text editing in converted reference rows
+
+Status: proposed as current principle; pending human review
+Sources: PR for LT-22672 (Allomorphs Environments row); `PhoneEnvReferenceView`
+and `PhoneEnvReferenceSlice` as the characterized source
+Human review: pending
+
+## Question tested
+
+Can a converted reference-vector row gain inline typed editing -- so an item
+already on the field can be changed, not only added and removed -- while
+matching a source view whose editing semantics are reference reconciliation
+rather than text entry?
+
+## Observations
+
+- The row's editing semantics were not text semantics. An item's identity was
+  its text with literal spaces removed, so a purely cosmetic re-spacing renamed
+  a shared object across the whole project, while any other change re-pointed
+  the reference and created a target only when none existed.
+- The full behaviour was **not** recoverable from the source view's commit
+  method. One of six user-visible outcomes -- emptying an item to remove it --
+  lived in a helper that decided which lines the commit would consider at all.
+  Reading the commit path alone produced a five-case model that looked complete
+  and was not.
+- An input guard rejecting blank text made the conversion look merely
+  incomplete. It was also masking a second defect: with the guard removed, the
+  unguarded path created an empty domain object for the item to point at. The
+  guard had made a wrong answer unreachable rather than correct.
+- The source view carried no tests for any of this. Its single test covered an
+  adjacent edge case, and was the only written evidence that blank lines were
+  treated differently from populated ones.
+- Two rendering defects were indistinguishable from one another by report. A
+  row that arranged items past its own width, and an editor whose measured
+  width fell short of the text it drew, both presented as "the end of the value
+  is cut off".
+- Headless rendering could not reproduce the measurement defect. The headless
+  text shaper uses uniform advances, so measurement and rendering agree there
+  by construction and the defect cannot arise.
+- Repeated small changes each produced "no visible difference" in the running
+  application. That observation was consistent with a wrong diagnosis, with a
+  correct diagnosis and an insufficient change, and with the build not reaching
+  the application at all. A single deliberately extreme change separated all
+  three at once.
+
+## What failed or was retired
+
+An earlier attempt replaced the row with a bespoke control. It was reverted
+because it lost the chooser, the item menu, reordering, and per-item
+validation, all of which the row already provided. The retained approach
+changed only how a row renders its items, leaving the row's identity intact.
+
+Three diagnoses of the rendering defects were tried and discarded: reducing
+each item's width by removing editor chrome, widening padding to leave room for
+a caret, and treating the row's overflow as the whole cause. The first two were
+wrong; the third was a real and separate defect that did not explain the report.
+
+## Durable lessons
+
+1. Characterize a source view by enumerating the user's gestures against it,
+   not by reading the method that writes changes back. A gesture's handling may
+   live in a helper that filters what the write-back ever sees.
+2. Before adding a guard that rejects an input, establish what the guarded path
+   would do with it. A guard that makes a wrong answer unreachable conceals the
+   defect instead of fixing it, and hides it from tests as well.
+3. Treat blank input as a possible gesture rather than an absent value. In this
+   area emptying an item is how a user removes it.
+4. Where a behaviour depends on text metrics or on arrangement against a real
+   surface, headless tests cannot reproduce it. Pin the invariant that must
+   hold rather than the mechanism, and require a pass in the running
+   application.
+5. When a change yields no visible difference, force an unmistakable variant of
+   it before reasoning further. "No change" cannot distinguish a wrong
+   diagnosis from a change too small to see or from a build that never arrived.
+
+## Evidence needed next time
+
+- The complete gesture list for the row -- add, remove, retype, clear, reorder,
+  menu, and the keyboard equivalents -- each exercised against the source view
+  before any of it is designed.
+- The source view's helper methods, not only its commit method, read for
+  gestures that never reach the write-back.
+- For every behavioural test, a suppression run showing that it, and only it,
+  fails.
+- A manual pass in the running application for anything touching text metrics,
+  wrapping, or available width, with the result recorded rather than assumed.
+- The behaviour of a shared domain object when one reference to it is edited or
+  removed, asserted from a *second* holder of that reference. A single-holder
+  fixture cannot show project-wide reach.
+
+## Decision boundary
+
+This record constrains how a reference-vector row's editing behaviour is
+discovered and evidenced. It does not decide which rows should become editable,
+what a row should do when a single item exceeds the whole row's width, or
+whether matching a surprising source behaviour is preferable to correcting it.
+Those remain domain-owner decisions, taken per row.
+
+## Do not infer
+
+- That reproducing a source view's surprising behaviour is generally correct.
+  It was chosen here because divergence would have made two views disagree
+  about shared data, and the choice was recorded rather than assumed.
+- That every reference-vector row should render editable items. The capability
+  is asked per field and only one row answers to it.
+- That an unconditional write-back on commit is a design to copy. It is matched
+  parity with a specific source view.
+- That the rendering fixes settle row layout generally. Wrapping happens
+  between items, not within one.
+- That the tests here cover the rendering defects themselves. They cover the
+  invariants those defects violated.

Src/Common/FwAvalonia/Detail/DataTree.cs

@@ -87,12 +87,12 @@ public sealed class DataTree : UserControl, IDetailPopupSink
 			new List<(TextBlock Label, double Reserved)>();
 
 		/// <summary>
-		/// Optional expansion-state hooks (11.8): <paramref name="getExpansionState"/> supplies the
+		/// Optional expansion-state hooks: <paramref name="getExpansionState"/> supplies the
 		/// persisted state per header stable id (overriding the layout's initial state) and
 		/// <paramref name="expansionChanged"/> records toggles, so collapse state survives record
-		/// switches/re-shows -- the legacy PropertyTable expansion persistence.
+		/// switches and re-shows.
 		/// <paramref name="getLabelColumnWidth"/>/<paramref name="labelColumnWidthChanged"/> persist
-		/// the splitter position the same way (11.15): the host owns the remembered width so it
+		/// the splitter position the same way: the host owns the remembered width so it
 		/// survives re-shows WITHOUT a process-global field -- each host/window keeps its own.
 		/// </summary>
 		public DataTree(DetailModel model, IDetailEditContext editContext = null,
@@ -162,7 +162,7 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null,
 			var splitter = new GridSplitter
 			{
 				ResizeDirection = GridResizeDirection.Columns,
-				Background = FwAvaloniaDensity.TransparentBrush, // legacy splitter is window-colored/invisible (12.6)
+				Background = FwAvaloniaDensity.TransparentBrush, // the splitter is invisible, not chrome
 				Width = FwAvaloniaDensity.SplitterWidth
 			};
 			AutomationProperties.SetAutomationId(splitter, "DataTree.Splitter");
@@ -219,8 +219,7 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null,
 			AddHandler(Avalonia.Input.InputElement.KeyDownEvent, OnViewKeyDown,
 				Avalonia.Interactivity.RoutingStrategies.Bubble);
 
-			// Auto-save (14.4): legacy slices commit as the user moves on -- any editor losing
-			// focus
+			// Auto-save: the view commits as the user moves on, so any editor losing focus
 			// while a session is open commits it (validation-gated; one undo step per field).
 			AddHandler(Avalonia.Input.InputElement.LostFocusEvent, (s, e) =>
 			{
@@ -354,8 +353,8 @@ private static void ApplyRowTabIndex(Control root, int row)
 		/// </summary>
 		public event EventHandler InteractionCompleted;
 
-		// 14.4: no Save/Cancel buttons -- the legacy view saves as you go. The footer carries
-		// only the inline validation messages (a failed autosave is never silent).
+		// No Save/Cancel buttons: the view saves as you go. The footer carries only the
+		// inline validation messages (a failed autosave is never silent).
 		private Control CreateEditFooter()
 		{
 			_validationBlock = new TextBlock
@@ -482,7 +481,7 @@ private void RebuildItems()
 		}
 
 		// A header's recorded expansion state prefers this session's own toggles over the
-		// host-supplied persisted state (11.8), so a toggle applies immediately rather than
+		// host-supplied persisted state, so a toggle applies immediately rather than
 		// waiting on the host's round-trip.
 		private bool? GetRecordedExpansion(string stableId) =>
 			_expansionState.TryGetValue(stableId, out var v) ? (bool?)v : _getExpansionState?.Invoke(stableId);
@@ -503,8 +502,8 @@ private Control BuildItem(int index, DetailField field)
 			return content;
 		}
 
-		// 12.1: the legacy 1px inter-slice rule renders as a per-item bottom border; the last
-		// field gets none.
+		// The 1px inter-slice rule renders as a per-item bottom border; the last field
+		// gets none.
 		private Control ApplyRule(Control content, int index)
 		{
 			if (index >= Model.Fields.Count - 1)
@@ -578,15 +577,15 @@ private FieldContent AddField(int row, DetailField field)
 						Text = field.Label ?? field.Field ?? string.Empty,
 						FontWeight = FontWeight.Bold,
 						Margin = new Thickness(indent.Left, 4, 0, FwAvaloniaDensity.FieldSpacing),
-						// 14.2: a null background only hit-tests the glyphs; the whole header area
+						// A null background only hit-tests the glyphs; the whole header area
 						// must take the right-click.
 						Background = FwAvaloniaDensity.TransparentBrush
 					};
 				}
 
 				AutomationProperties.SetAutomationId(header, automationId);
 				AutomationProperties.SetName(header, field.Label ?? string.Empty);
-				// 13.3/13.5: the header answers right-click with its slice menu; the hover
+				// The header answers right-click with its slice menu; the hover
 				// "..." field-menu button (in a thin gutter to the left of the header)
 				// opens the section menu/hotlinks.
 				var headerCell = WrapWithFieldMenu(header, field, automationId, out var headerKebab);
@@ -599,9 +598,9 @@ private FieldContent AddField(int row, DetailField field)
 				// through the existing host bridge identically.
 				var hotlinkStrip = CreateHotlinkStrip(field, automationId, indent);
 
-				// Viewing parity (11.15): top-level sections get the legacy heavy-weight separator rule.
-				// The header cell and its inline hotlink strip always travel together (the strip is part of
-				// the header row, hidden/shown with it by the collapse logic).
+				// Top-level sections get the heavy-weight separator rule. The header cell and
+				// its hotlink strip travel together: the strip is part of the header row,
+				// hidden and shown with it.
 				Control headerControl;
 				if (field.Indent == 0 && row > 0)
 				{
@@ -658,15 +657,14 @@ private FieldContent AddField(int row, DetailField field)
 				// this
 				// local value wins for our own TextBlock and keeps labels regular, like legacy.
 				FontWeight = FontWeight.Normal,
-				// 14.2: a null background only hit-tests the glyphs; the whole label area must
-				// take
-				// the right-click for the slice menu.
+				// A null background only hit-tests the glyphs; the whole label area must
+				// take the right-click for the slice menu.
 				Background = FwAvaloniaDensity.TransparentBrush
 			};
 			_labelBlocks.Add((labelBlock, labelReserved));
 			AutomationProperties.SetAutomationId(labelBlock, automationId + ".Label");
 			AutomationProperties.SetName(labelBlock, field.Label ?? field.Field ?? string.Empty);
-			ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // 11.17: legacy label tooltips
+			ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // the label text is its own tip
 			var editor = CreateEditor(field, automationId);
 			editor.Margin = new Thickness(0, 0, 0, FwAvaloniaDensity.FieldSpacing);
 
@@ -680,7 +678,7 @@ private FieldContent AddField(int row, DetailField field)
 				vector.SelectionChanged += OnVectorSelectionChanged;
 			}
 
-			// 13.3: the field's slice menu opens from the label cell's right-click or the
+			// The field's slice menu opens from the label cell's right-click or the
 			// gutter "..." button; the editor's current item rides each request it raises.
 			var labelCell = WrapWithFieldMenu(labelBlock, field, automationId, out var labelKebab,
 				editor as IDetailItemSelection);

Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs

@@ -13,7 +13,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail
 	/// <summary>
 	/// Keeps keyboard focus stable across detail-view re-shows. The host re-resolves and REPLACES the
 	/// whole detail view after every committed edit and every delivered external refresh; without
-	/// this, tabbing out of a field (which auto-commits, 14.4) would tear down the editor the user
+	/// this, tabbing out of a field (which auto-commits) would tear down the editor the user
 	/// just moved into and dump focus on the floor. Capture reads the focused editor's stable
 	/// automation id (and caret) from the outgoing view; restore finds the same id in the incoming
 	/// view and gives it focus -- automation ids are stable per field/writing system by design,

Src/Common/FwAvalonia/Detail/DetailMenuFlyout.cs

@@ -9,7 +9,7 @@
 namespace SIL.FieldWorks.Common.FwAvalonia.Detail
 {
 	/// <summary>
-	/// Framework-neutral context-menu item (15.1): what the host resolved from its menu system
+	/// Framework-neutral context-menu item: what the host resolved from its menu system
 	/// (for FieldWorks, the xCore ChoiceGroup -- labels, enablement, checkmarks, submenus, and an
 	/// execute action that dispatches through the mediator). FwAvalonia renders these natively;
 	/// it knows nothing about xCore, preserving the engine-isolation boundary.
@@ -45,10 +45,9 @@ private DetailMenuItem()
 
 	/// <summary>
 	/// Renders host-built <see cref="DetailMenuItem"/> trees as a native Avalonia
-	/// <see cref="MenuFlyout"/> (15.1) -- the same items, enablement, checkmarks, and submenus
-	/// the
-	/// legacy WinForms adapter menu shows, rendered with native Avalonia controls. Density: every item carries the
-	/// explicit compact padding/height of the legacy WinForms menus
+	/// <see cref="MenuFlyout"/> -- the same items, enablement, checkmarks and submenus the
+	/// host resolved, rendered with native Avalonia controls. Density: every item carries
+	/// explicit compact padding and height
 	/// (<see cref="FwAvaloniaDensity.MenuItemPadding"/>/<see cref="FwAvaloniaDensity.MenuItemMinHeight"/>,
 	/// not the Fluent theme defaults); long menus keep the presenter's scrolling.
 	/// </summary>