LT-22858: Show notebook subrecord fields; drop dead prompt rows (LT-22844)

Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs

@@ -129,16 +129,33 @@ public void ConditionalElements_WithSubstitutionValues_StillDropWithDiagnostics(
 			Assert.That(model.Roots, Is.Empty, "an unevaluable condition must not import its content");
 		}
 
+		// LT-22858: Notebook's SubRecord layout brings a subrecord's fields in this way.
 		[Test]
-		public void SublayoutElement_RaisesInfoDropDiagnostic()
+		public void SublayoutElement_ImportsAsANode_NamingItsLayoutAndChoiceField()
 		{
 			var model = Import(@"
-<layout class='LexEntry' type='detail' name='T'>
-  <sublayout name='domainLabel' group='para'/>
+<layout class='RnGenericRec' type='detail' name='T'>
+  <sublayout name='Normal' layoutChoiceField='Type'/>
 </layout>");
 
+			var sublayout = model.Roots.Single();
+			Assert.That(sublayout.Kind, Is.EqualTo(ViewNodeKind.Sublayout));
+			Assert.That(sublayout.TargetLayout, Is.EqualTo("Normal"));
+			Assert.That(sublayout.Field, Is.EqualTo("Type"));
+			Assert.That(model.Diagnostics.Any(d => d.Code == "sublayout-dropped"), Is.False);
+		}
+
+		[Test]
+		public void SublayoutElement_WithoutAName_IsDroppedWithAWarning()
+		{
+			var model = Import(@"
+<layout class='RnGenericRec' type='detail' name='T'>
+  <sublayout layoutChoiceField='Type'/>
+</layout>");
+
+			Assert.That(model.Roots, Is.Empty);
 			Assert.That(model.Diagnostics.Single(d => d.Code == "sublayout-dropped").Severity,
-				Is.EqualTo(ViewDiagnosticSeverity.Info));
+				Is.EqualTo(ViewDiagnosticSeverity.Warning));
 		}
 
 		[Test]

Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs

@@ -46,7 +46,14 @@ public enum ViewNodeKind
 		/// (<c>&lt;where&gt;</c>/<c>&lt;otherwise&gt;</c>); only the FIRST branch whose condition
 		/// passes renders, matching <c>DataTree.ProcessSubpartNode</c>'s choice handling.
 		/// </summary>
-		ChoiceGroup
+		ChoiceGroup,
+
+		/// <summary>
+		/// Another layout of the same object laid out in place (<c>&lt;sublayout&gt;</c>):
+		/// <see cref="ViewNode.TargetLayout"/> names it, and <see cref="ViewNode.Field"/>,
+		/// when set, is the field whose value picks its <c>choiceGuid</c> variant.
+		/// </summary>
+		Sublayout
 	}
 
 	/// <summary>Field visibility, mirroring the legacy <c>visibility</c> attribute.</summary>

Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs

@@ -137,12 +137,20 @@ private void ProcessContainer(
 						break;
 					}
 					case "sublayout":
-						diagnostics.Add(new ViewDiagnostic(
-							ViewDiagnosticSeverity.Info,
-							"sublayout-dropped",
-							$"<sublayout name='{(string)el.Attribute("name")}'> is a publishing construct and is not imported for detail views.",
-							$"{parentPath}/#{output.Count}"));
+					{
+						var stableId = $"{parentPath}/#{output.Count}";
+						var layoutName = Attr(el, "name");
+						if (string.IsNullOrEmpty(layoutName))
+						{
+							diagnostics.Add(new ViewDiagnostic(ViewDiagnosticSeverity.Warning, "sublayout-dropped",
+								"A <sublayout> has no 'name' attribute.", stableId));
+							break;
+						}
+						output.Add(MakeLeaf(stableId, ViewNodeKind.Sublayout, null, null,
+							Attr(el, "layoutChoiceField"), null, EditorClassification.GroupingNone, null,
+							ViewVisibility.Always, ViewExpansion.NotApplicable, indented, layoutName));
 						break;
+					}
 					default:
 						diagnostics.Add(new ViewDiagnostic(
 							ViewDiagnosticSeverity.Warning,

Src/xWorks/Avalonia/Composer/DetailComposer.cs

@@ -276,6 +276,9 @@ private sealed class ComposeState
 			private readonly IFwMetaDataCacheManaged _mdc;
 			private readonly HashSet<(int hvo, string layout)> _visited = new HashSet<(int, string)>();
 			private readonly DetailSummaryLines _summaryLines;
+			// The sequence item whose header was just emitted with its summary lines, until the
+			// next row is added; that item's own opening summary row is not emitted again.
+			private int _itemSummaryHvo;
 
 			public readonly List<DetailField> Fields = new List<DetailField>();
 			// One edit handler per composed field, keyed by StableId. A field's text/rich-text/option/
@@ -342,6 +345,7 @@ public FieldEditHandler HandlerFor(string stableId)
 			// binding is derived from the compiled layout's root, which can differ per choice variant. It is
 			// correct ONLY because descent currently compiles every embedded object with choiceGuid=null
 			// (CompileForObjectWithOverrides), so within one compose there is no choice variance to collide.
+			// WalkSublayout compiles with a choice, but its model never feeds these memos.
 			// If descent is ever changed to thread choiceGuid through, change this key to
 			// (ClassId, LayoutName, choiceGuid) in the SAME change, or this becomes a wrong-menu bug.
 			private readonly Dictionary<(int ClassId, string LayoutName), (string MenuId, string HotlinksId)> _itemMenuBindings
@@ -396,6 +400,7 @@ private void AddField(DetailField field)
 				StampHelpTopic(field);
 
 				Fields.Add(field);
+				_itemSummaryHvo = 0;
 			}
 
 			// The node whose walk is adding rows right now (see Walk).
@@ -674,6 +679,33 @@ private void WalkByKind(ViewNode node, ICmObject obj, int depth)
 						// Legacy <choice> -- first passing <where> (or the <otherwise>) only.
 						WalkChoiceGroup(node, obj, depth);
 						break;
+					case ViewNodeKind.Sublayout:
+						WalkSublayout(node, obj, depth);
+						break;
+				}
+			}
+
+			// A <sublayout>: the named layout of this same object, picked by its choice field as
+			// the shown record's is, laid out in place at this depth.
+			private void WalkSublayout(ViewNode node, ICmObject obj, int depth)
+			{
+				var layoutName = node.TargetLayout;
+				if (string.IsNullOrEmpty(layoutName) || !_visited.Add((obj.Hvo, layoutName)))
+					return;
+				try
+				{
+					var choiceGuid = ResolveLayoutChoiceGuid(_cache, obj, node.Field);
+					var compiled = CompileForObject(_cache, obj, layoutName, choiceGuid, _overrides);
+					if (compiled == null)
+						return;
+					EnterModel(compiled);
+					foreach (var child in compiled.Roots)
+						Walk(child, obj, depth);
+					ExitModel();
+				}
+				finally
+				{
+					_visited.Remove((obj.Hvo, layoutName));
 				}
 			}
 
@@ -1107,7 +1139,16 @@ private void WalkField(ViewNode node, ICmObject obj, int depth)
 						WalkMsaChooser(node, obj, depth);
 						break;
 					case DetailEditorCategory.Summary:
-						// Summary slices are section header rows in legacy too.
+						// Summary slices are section header rows, as in the WinForms
+						// DataTree, except an item's opening summary row: its sequence
+						// item header stands for it.
+						if (obj.Hvo == _itemSummaryHvo)
+						{
+							// What the row would hold sits under that item header instead.
+							foreach (var child in node.Children)
+								Walk(child, obj, depth);
+							return;
+						}
 						AddHeader(node, obj, depth, Localize(node.Label) ?? node.Field);
 						break;
 					case DetailEditorCategory.Literal:
@@ -3263,8 +3304,13 @@ private void AddGhostPrompt(ViewNode node, ICmObject obj, int depth)
 				var prompt = string.Format(
 					SIL.FieldWorks.Common.FwAvalonia.FwAvaloniaStrings.GhostAddPromptFormat, label);
 
-				var stableId = StableId(node, obj) + DetailField.GhostStableIdSuffix;
 				var ghost = ResolveGhostCreation(node, obj);
+				// The WinForms DataTree shows a prompt only where the layout names a ghost
+				// field; a prompt that names none and can create nothing is left out.
+				if (ghost == null && string.IsNullOrEmpty(node.GhostField))
+					return;
+
+				var stableId = StableId(node, obj) + DetailField.GhostStableIdSuffix;
 				AddField(new DetailField(stableId, label, node.Field,
 					node.WritingSystem, DetailFieldKind.Text, node.EditorClassification,
 					node.AutomationId, node.LocalizationKey, node.Routing,
@@ -3437,9 +3483,9 @@ private void WalkSequence(ViewNode node, ICmObject obj, int depth)
 				var sectionLabel = Localize(node.Label) ?? node.Field;
 				var firstItem = _cache.ServiceLocator.ObjectRepository.GetObject(_sda.get_VecItem(obj.Hvo, flid, 0));
 
-				// Items with a summary line pair (senses, etymologies, extended notes) are
-				// sections of their own: each item's header carries its summary lines, and no
-				// banner sits over them.
+				// Items with a summary line pair (senses, etymologies, extended notes, notebook
+				// records) are sections of their own: each item's header stands for the summary
+				// row its layout opens with, and no banner sits over them.
 				var summarized = DetailSummaryLines.Covers(firstItem);
 
 				// A sequence sitting directly under a section header that already names it (the
@@ -3494,7 +3540,10 @@ private void WalkSequence(ViewNode node, ICmObject obj, int depth)
 						CollapsedLabel = collapsedLabel,
 						IsNormalWeight = summarized && DetailSummaryLines.IsNormalWeight(item)
 					});
+					if (summarized)
+						_itemSummaryHvo = item.Hvo;
 					DescendInto(node, item, itemDepth);
+					_itemSummaryHvo = 0;
 				}
 			}
 

Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs

@@ -7,15 +7,16 @@
 using SIL.FieldWorks.Common.FwUtils;
 using SIL.LCModel;
 using SIL.LCModel.Core.WritingSystems;
+using SIL.LCModel.Infrastructure;
 
 namespace SIL.FieldWorks.XWorks
 {
 	/// <summary>
 	/// The header lines of the objects whose detail layout opens with a summary row carrying a
-	/// collapsed variant: a sense, an etymology and an extended note. Each pair follows that
-	/// class's Summary and SummaryCollapsed parts in the shipped configuration
-	/// (LexSenseParts.xml, LexEntryParts.xml), with their literal text localized through the
-	/// string table.
+	/// collapsed variant: a sense, an etymology, an extended note and a notebook record. Each
+	/// pair follows that class's Summary and SummaryCollapsed parts in the shipped
+	/// configuration (LexSenseParts.xml, LexEntryParts.xml, NotebookParts.xml), with their
+	/// literal text localized through the string table.
 	/// </summary>
 	internal sealed class DetailSummaryLines
 	{
@@ -29,7 +30,7 @@ public DetailSummaryLines(LcmCache cache)
 
 		/// <summary>True when <paramref name="obj"/>'s class has a summary line pair.</summary>
 		public static bool Covers(ICmObject obj)
-			=> obj is ILexSense || obj is ILexEtymology || obj is ILexExtendedNote;
+			=> obj is ILexSense || obj is ILexEtymology || obj is ILexExtendedNote || obj is IRnGenericRec;
 
 		/// <summary>
 		/// True when <paramref name="obj"/>'s layout opens with a NormalSummary row rather than
@@ -61,6 +62,10 @@ public bool TryGet(ICmObject obj, out string expanded, out string collapsed)
 					collapsed = expanded + Lit(" - ") + Abbreviation(note.ExtendedNoteTypeRA) + Lit(" - ")
 						+ BestAnalysis(note.Discussion);
 					return true;
+				case IRnGenericRec record:
+					expanded = IndexInOwner(record) + Lit(" ") + Name(record.TypeRA);
+					collapsed = expanded + Lit(" - ") + record.Title?.Text;
+					return true;
 				default:
 					expanded = collapsed = null;
 					return false;
@@ -102,12 +107,32 @@ private string GrammaticalInfo(IMoMorphSynAnalysis msa)
 			}
 		}
 
+		// A record's IndexInOwnerTSS (its number among its owner's subrecords).
+		private string IndexInOwner(IRnGenericRec record)
+		{
+			var mdc = (IFwMetaDataCacheManaged)_cache.MetaDataCacheAccessor;
+			if (mdc.FieldExists(RnGenericRecTags.kClassName, "IndexInOwnerTSS", true))
+			{
+				var flid = mdc.GetFieldId(RnGenericRecTags.kClassName, "IndexInOwnerTSS", true);
+				return _cache.DomainDataByFlid.get_StringProp(record.Hvo, flid)?.Text ?? string.Empty;
+			}
+			return (record.IndexInOwner + 1).ToString();
+		}
+
 		private string Abbreviation(ICmPossibility possibility)
 			=> possibility == null ? string.Empty : BestAnalysis(possibility.Abbreviation);
 
+		private string Name(ICmPossibility possibility)
+			=> possibility == null
+				? string.Empty
+				: Best(possibility.Name, AnalysisWritingSystems.Concat(VernacularWritingSystems));
+
 		private IEnumerable<CoreWritingSystemDefinition> AnalysisWritingSystems
 			=> _cache.LangProject.CurrentAnalysisWritingSystems;
 
+		private IEnumerable<CoreWritingSystemDefinition> VernacularWritingSystems
+			=> _cache.LangProject.CurrentVernacularWritingSystems;
+
 		// The first non-empty alternative in the analysis writing systems, in project order.
 		private string BestAnalysis(IMultiAccessorBase value) => Best(value, AnalysisWritingSystems);
 

Src/xWorks/xWorksTests/Avalonia/Composer/AllomorphSectionCompositionTests.cs

@@ -212,19 +212,21 @@ public void Compose_AffixProcess_RendersUnsupportedRow_ForRuleFormula()
 		}
 
 		/// <summary>
-		/// An entry with no allomorphs offers the watermarked add row rather than nothing.
+		/// An entry with no allomorphs shows only the Allomorphs header, as the WinForms view
+		/// does: the list names no ghost field, and the header's menu inserts the first one.
 		/// </summary>
 		[Test]
-		public void Compose_EmptyAllomorphSection_ShowsGhostAddRow()
+		public void Compose_EmptyAllomorphSection_ShowsOnlyItsHeader_WithInsertOnItsMenu()
 		{
 			NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor,
 				() => m_entry.AlternateFormsOS.Clear());
 
 			var subtree = ComposeSectionSubtree();
 
-			Assert.That(subtree.Any(f => !string.IsNullOrEmpty(f.GhostPrompt)), Is.True,
-				"an empty allomorph sequence composes the ghost add row. Composed:\n"
-				+ Describe(subtree));
+			Assert.That(subtree.Any(f => !string.IsNullOrEmpty(f.GhostPrompt)), Is.False,
+				"no prompt row for an empty allomorph sequence. Composed:\n" + Describe(subtree));
+			Assert.That(subtree.Single(f => f.Kind == DetailFieldKind.Header).MenuId,
+				Is.EqualTo("mnuDataTree-AlternateForms"), "the header keeps Insert Allomorph");
 		}
 
 		/// <summary>

Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs

@@ -1562,27 +1562,19 @@ public void Compose_GhostTranslation_CreatesACmTranslation_TypedFreeByGhostInitM
 				"the new translation is typed Free Translation, like legacy");
 		}
 
-		// A ghost prompt whose layout authors NO ghost field (and no init method)
-		// must compose NON-editable -- an editable one would call MakeNewObject on typing and
-		// silently DISCARD the typed text (nothing exists to receive the string). The shipped
-		// AlternateForms seq (LexEntryParts.xml:119, no ghost= attribute) is exactly that case.
+		// LT-22844: AlternateForms names no ghost field and can create nothing, so it has no
+		// prompt row; SubSenses names none either but types a gloss into a new sense.
 		[Test]
-		public void Compose_GhostWithoutGhostField_IsNotEditable_SoTypingCannotCreateAndDiscard()
+		public void Compose_AnEmptyListWithNoGhostField_HasNoPromptRow_UnlessTypingCanCreate()
 		{
 			var composed = DetailComposer.Compose(m_entry, Cache);
-			var ghost = composed.Model.Fields.FirstOrDefault(f =>
-				f.Field == "AlternateForms" && f.StableId.EndsWith("/ghost"));
-			Assert.That(ghost, Is.Not.Null,
-				"the empty always-visible alternate-forms sequence still renders its prompt row");
-			Assert.That(ghost.IsEditable, Is.False,
-				"no ghost field anywhere to route the text: the prompt must not accept typing");
-
-			var before = m_entry.AlternateFormsOS.Count;
-			Assert.That(composed.EditContext.TrySetText(ghost, ghost.Values[0].WsAbbrev, "lost"),
-				Is.False, "no setter is registered for the non-editable prompt");
-			Assert.That(composed.EditContext.IsOpen, Is.False, "nothing staged, no session opened");
-			Assert.That(m_entry.AlternateFormsOS.Count, Is.EqualTo(before),
-				"no bare object was created behind the user's back");
+
+			Assert.That(m_entry.AlternateFormsOS, Is.Empty, "precondition: no allomorphs");
+			Assert.That(composed.Model.Fields.Any(f =>
+				f.Field == "AlternateForms" && f.StableId.EndsWith(DetailField.GhostStableIdSuffix)), Is.False,
+				"no prompt row that could only discard what is typed");
+			var subsense = SubsenseGhost(composed, m_entry.SensesOS[0]);
+			Assert.That(subsense?.IsEditable, Is.True, "the subsense prompt still creates a sense");
 		}
 
 		private static DetailField ExampleGhost(ComposedDetail composed, ILexSense sense)