From eb01ddb0edb2c0350d6f47a8f844dfa0e2978e77 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Fri, 23 Oct 2026 12:42:18 -0400 Subject: [PATCH 1/5] LT-22858: Bring back the notebook subrecord header lines A subrecord header reads "1. Observation" while expanded and "1. Observation - title" while collapsed, as the record's Summary and SummaryCollapsed parts define, and stands for the summary row the subrecord's layout opens with, so no second SubRecords header follows. This reverts commit 01de10d48. Co-Authored-By: Claude Opus 5.5 --- .../Avalonia/Composer/DetailComposer.cs | 19 +++++-- .../Avalonia/Composer/DetailSummaryLines.cs | 36 +++++++++++-- .../Composer/DetailSummaryLinesTests.cs | 51 +++++++++++++++++++ 3 files changed, 97 insertions(+), 9 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index f4b1736146..5854b874b9 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/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 Fields = new List(); // One edit handler per composed field, keyed by StableId. A field's text/rich-text/option/ @@ -396,6 +399,7 @@ private void AddField(DetailField field) StampHelpTopic(field); Fields.Add(field); + _itemSummaryHvo = 0; } // The node whose walk is adding rows right now (see Walk). @@ -1107,7 +1111,11 @@ 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) + break; AddHeader(node, obj, depth, Localize(node.Label) ?? node.Field); break; case DetailEditorCategory.Literal: @@ -3437,9 +3445,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 +3502,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; } } diff --git a/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs b/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs index bee5d9f1d1..39d687c82b 100644 --- a/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs +++ b/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs @@ -6,16 +6,18 @@ using System.Linq; using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; using SIL.LCModel.Core.WritingSystems; +using SIL.LCModel.Infrastructure; namespace SIL.FieldWorks.XWorks { /// /// 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. /// internal sealed class DetailSummaryLines { @@ -29,7 +31,7 @@ public DetailSummaryLines(LcmCache cache) /// True when 's class has a summary line pair. 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; /// /// True when 's layout opens with a NormalSummary row rather than @@ -61,6 +63,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 +108,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 AnalysisWritingSystems => _cache.LangProject.CurrentAnalysisWritingSystems; + private IEnumerable 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); diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs index 689018f41f..206d7401a0 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs @@ -195,6 +195,57 @@ public void ExtendedNote_WhileCollapsed_ShowsItsTypeAndDiscussion() Assert.That(Lines(note), Is.EqualTo(("Extended Note", "Extended Note - use - formal speech"))); } + private IRnGenericRec RecordWithSubrecord(out IRnGenericRec subrecord) + { + IRnGenericRec record = null, sub = null; + Do(() => + { + if (Cache.LangProject.ResearchNotebookOA == null) + Cache.LangProject.ResearchNotebookOA = Cache.ServiceLocator.GetInstance().Create(); + var notebook = Cache.LangProject.ResearchNotebookOA; + if (notebook.RecTypesOA == null) + notebook.RecTypesOA = Cache.ServiceLocator.GetInstance().Create(); + var type = Cache.ServiceLocator.GetInstance().Create(); + notebook.RecTypesOA.PossibilitiesOS.Add(type); + type.Name.set_String(Cache.DefaultAnalWs, Analysis("Observation")); + record = Cache.ServiceLocator.GetInstance().Create(); + notebook.RecordsOC.Add(record); + sub = Cache.ServiceLocator.GetInstance().Create(); + record.SubRecordsOS.Add(sub); + sub.TypeRA = type; + sub.Title = Analysis("Market day"); + }); + subrecord = sub; + return record; + } + + [Test] + public void Subrecord_ShowsItsNumberAndType_AndWhileCollapsed_ItsTitle() + { + RecordWithSubrecord(out var subrecord); + + var lines = Lines(subrecord); + + Assert.That(lines.Expanded, Does.StartWith("1").And.EndWith(" Observation")); + Assert.That(lines.Collapsed, Is.EqualTo(lines.Expanded + " - Market day")); + } + + [Test] + public void Compose_ASubrecord_HasOneHeaderOfItsOwn_UnderTheSubrecordsSection() + { + var record = RecordWithSubrecord(out var subrecord); + + var headers = DetailComposer.Compose(record, Cache).Model.Fields + .Where(f => f.Kind == DetailFieldKind.Header).ToList(); + + var subrecordHeaders = headers.Where(h => h.ObjectHvo == subrecord.Hvo).ToList(); + Assert.That(subrecordHeaders.Select(h => h.Label), Is.EqualTo(new[] { Lines(subrecord).Expanded }), + "one header for the subrecord, carrying its summary line"); + var section = headers.Single(h => h.Label == "Subrecords"); + Assert.That(subrecordHeaders[0].Indent, Is.EqualTo(section.Indent + 1), + "the subrecord sits directly under the Subrecords section, with no banner between"); + } + [Test] public void Compose_EtymologiesAndExtendedNotes_HaveNoBanner_AndCarryTheirSummaryLines() { From a333eb5c13d2af9b71bcbc96d4848a36517ed3eb Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Fri, 23 Oct 2026 12:49:03 -0400 Subject: [PATCH 2/5] LT-22858: Compose a notebook subrecord's fields under its header The SubRecord layout brings a subrecord's fields in with , which the layout importer dropped as a publishing construct, so each subrecord showed only its header. The importer now keeps it as a Sublayout node, and the composer lays out that layout for the subrecord's Type in place, the way the shown record's own layout is chosen. The fields sit one level under the subrecord's header, and nested subrecords repeat the pattern. A subrecord header with fields under it can now collapse, which is what shows its collapsed line. Co-Authored-By: Claude Opus 5.5 --- .../LayoutImportCoverageTests.cs | 25 +++++- .../ViewDefinition/ViewDefinitionModel.cs | 9 +- .../ViewDefinition/XmlLayoutImporter.cs | 18 ++-- .../Avalonia/Composer/DetailComposer.cs | 35 +++++++- .../Composer/DetailSummaryLinesTests.cs | 87 +++++++++++++++++++ 5 files changed, 163 insertions(+), 11 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs index d618409d69..43d13058e3 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs +++ b/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(@" - - + + "); + 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(@" + + +"); + + 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] diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs index 207393e3ce..f741f591a1 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs @@ -46,7 +46,14 @@ public enum ViewNodeKind /// (<where>/<otherwise>); only the FIRST branch whose condition /// passes renders, matching DataTree.ProcessSubpartNode's choice handling. /// - ChoiceGroup + ChoiceGroup, + + /// + /// Another layout of the same object laid out in place (<sublayout>): + /// names it, and , + /// when set, is the field whose value picks its choiceGuid variant. + /// + Sublayout } /// Field visibility, mirroring the legacy visibility attribute. diff --git a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs index 5a6877e1cd..9d3eb2d671 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs @@ -137,12 +137,20 @@ private void ProcessContainer( break; } case "sublayout": - diagnostics.Add(new ViewDiagnostic( - ViewDiagnosticSeverity.Info, - "sublayout-dropped", - $" 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 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, diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 5854b874b9..0dd020a338 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -345,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 @@ -678,6 +679,33 @@ private void WalkByKind(ViewNode node, ICmObject obj, int depth) // Legacy -- first passing (or the ) only. WalkChoiceGroup(node, obj, depth); break; + case ViewNodeKind.Sublayout: + WalkSublayout(node, obj, depth); + break; + } + } + + // A : 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)); } } @@ -1115,7 +1143,12 @@ private void WalkField(ViewNode node, ICmObject obj, int depth) // DataTree, except an item's opening summary row: its sequence // item header stands for it. if (obj.Hvo == _itemSummaryHvo) - break; + { + // 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: diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs index 206d7401a0..da0a99ded2 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs @@ -2,6 +2,8 @@ // This software is licensed under the LGPL, version 2.1 or later // (http://www.gnu.org/licenses/lgpl-2.1.html) +using System; +using System.Collections.Generic; using System.Linq; using NUnit.Framework; using SIL.FieldWorks.Common.FwAvalonia.Detail; @@ -246,6 +248,91 @@ public void Compose_ASubrecord_HasOneHeaderOfItsOwn_UnderTheSubrecordsSection() "the subrecord sits directly under the Subrecords section, with no banner between"); } + // The Event record Type: its Normal layout shows Hypothesis and hides an empty Sources, + // and the layout with no choiceGuid does the reverse. + private static readonly Guid EventTypeGuid = new Guid("82290763-1633-4998-8317-0EC3F5027FBD"); + + private ICmPossibility EventType() + { + var types = Cache.LangProject.ResearchNotebookOA.RecTypesOA; + return types.PossibilitiesOS.FirstOrDefault(p => p.Guid == EventTypeGuid) + ?? Cache.ServiceLocator.GetInstance().Create(EventTypeGuid, types); + } + + private IRnGenericRec AddSubrecord(IRnGenericRec owner, string title) + { + var sub = Cache.ServiceLocator.GetInstance().Create(); + owner.SubRecordsOS.Add(sub); + sub.Title = Analysis(title); + return sub; + } + + // The non-header rows composed for the object. + private static List RowsOf(IReadOnlyList fields, ICmObject obj) + => fields.Where(f => f.ObjectHvo == obj.Hvo && f.Kind != DetailFieldKind.Header).ToList(); + + // LT-22858 + [Test] + public void Compose_ASubrecord_ShowsItsFields_UnderItsHeader_ChosenByItsType() + { + var record = RecordWithSubrecord(out var untyped); + IRnGenericRec eventSub = null; + Do(() => + { + untyped.TypeRA = null; + eventSub = AddSubrecord(record, "Harvest"); + eventSub.TypeRA = EventType(); + }); + + var fields = DetailComposer.Compose(record, Cache).Model.Fields; + + foreach (var sub in new[] { untyped, eventSub }) + { + var header = fields.Single(f => f.Kind == DetailFieldKind.Header && f.ObjectHvo == sub.Hvo); + var title = RowsOf(fields, sub).FirstOrDefault(f => f.Field == "Title"); + Assert.That(title, Is.Not.Null, "the subrecord's own fields compose under its header"); + Assert.That(title.Indent, Is.EqualTo(header.Indent + 1), "one level under the header"); + } + var eventFields = RowsOf(fields, eventSub).Select(f => f.Field).ToList(); + var untypedFields = RowsOf(fields, untyped).Select(f => f.Field).ToList(); + Assert.That(eventFields, Does.Contain("Hypothesis").And.Not.Contain("Sources"), + "an Event subrecord gets the Event layout"); + Assert.That(untypedFields, Does.Contain("Sources").And.Not.Contain("Hypothesis"), + "a subrecord with no Type gets the layout with no choice"); + } + + [Test] + public void Edit_ASubrecordsTitle_UnderItsHeader_SavesToTheSubrecord() + { + var record = RecordWithSubrecord(out var sub); + Do(() => record.Title = Analysis("Field trip")); + var composed = DetailComposer.Compose(record, Cache); + var title = RowsOf(composed.Model.Fields, sub).Single(f => f.Field == "Title"); + + Assert.That(composed.EditContext.TrySetText(title, title.Values[0].WsAbbrev, "Fish market"), Is.True); + composed.EditContext.Commit(); + + Assert.That(sub.Title.Text, Is.EqualTo("Fish market")); + Assert.That(record.Title.Text, Is.EqualTo("Field trip"), "only the subrecord changed"); + } + + [Test] + public void Compose_ASubrecordOfASubrecord_ShowsItsFieldsOneLevelFurtherIn() + { + var record = RecordWithSubrecord(out var sub); + IRnGenericRec subSub = null; + Do(() => subSub = AddSubrecord(sub, "Stall prices")); + + var fields = DetailComposer.Compose(record, Cache).Model.Fields; + + var subTitle = RowsOf(fields, sub).Single(f => f.Field == "Title"); + var subSubHeader = fields.Single(f => f.Kind == DetailFieldKind.Header && f.ObjectHvo == subSub.Hvo); + var subSubTitle = RowsOf(fields, subSub).SingleOrDefault(f => f.Field == "Title"); + Assert.That(subSubTitle, Is.Not.Null, "the nested subrecord's fields compose too"); + Assert.That(subSubHeader.Indent, Is.GreaterThan(subTitle.Indent), "its header nests inside the subrecord"); + Assert.That(subSubTitle.Indent, Is.EqualTo(subSubHeader.Indent + 1)); + } + [Test] public void Compose_EtymologiesAndExtendedNotes_HaveNoBanner_AndCarryTheirSummaryLines() { From 8ac0893596ce3fc0f20bf14181cb02851fe54280 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Fri, 23 Oct 2026 19:01:16 -0400 Subject: [PATCH 3/5] LT-22844: Leave out prompt rows that can create nothing An empty list whose layout names no ghost field showed a "Type to add ..." row that could not be typed in, labelled with the raw field name: the SubRecords list on a new notebook record, and now on every subrecord with no subrecords. The WinForms DataTree shows a prompt only where the layout names a ghost field, so those rows are left out. Each affected list keeps its section header and that header's Insert command, and the sense and subsense gloss prompts, which can create a sense, stay. Co-Authored-By: Claude Opus 5.5 --- .../Avalonia/Composer/DetailComposer.cs | 7 ++++- .../AllomorphSectionCompositionTests.cs | 12 ++++---- .../Composer/DetailEditContextEditingTests.cs | 28 +++++++------------ .../Composer/DetailSummaryLinesTests.cs | 24 ++++++++++++++++ 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 0dd020a338..0f79200fba 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -3304,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, diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/AllomorphSectionCompositionTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/AllomorphSectionCompositionTests.cs index 4f66381005..79a537cbd7 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/AllomorphSectionCompositionTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/AllomorphSectionCompositionTests.cs @@ -212,19 +212,21 @@ public void Compose_AffixProcess_RendersUnsupportedRow_ForRuleFormula() } /// - /// 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. /// [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"); } /// diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index 1a2d3f3585..810eb2b15b 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/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) diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs index da0a99ded2..ba710b5de7 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs @@ -316,6 +316,30 @@ public void Edit_ASubrecordsTitle_UnderItsHeader_SavesToTheSubrecord() Assert.That(record.Title.Text, Is.EqualTo("Field trip"), "only the subrecord changed"); } + // LT-22844: the SubRecords list names no ghost field, so it has no prompt row. + [Test] + public void Compose_ARecordWithNoSubrecords_KeepsTheSubrecordsHeader_WithNoPromptRow() + { + var record = RecordWithSubrecord(out var sub); + IRnGenericRec empty = null; + Do(() => + { + empty = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.ResearchNotebookOA.RecordsOC.Add(empty); + }); + + var fields = DetailComposer.Compose(empty, Cache).Model.Fields; + + var section = fields.Single(f => f.Kind == DetailFieldKind.Header && f.Label == "Subrecords"); + Assert.That(section.HotlinksId, Is.EqualTo("mnuDataTree-SubRecords-Hotlinks"), + "Insert Subrecord stays on the header's hotlinks"); + Assert.That(fields.Any(f => f.Field == "SubRecords" && f.GhostPrompt != null), Is.False, + "no prompt row for an empty Subrecords list"); + var nested = DetailComposer.Compose(record, Cache).Model.Fields; + Assert.That(nested.Any(f => f.ObjectHvo == sub.Hvo && f.Field == "SubRecords" && f.GhostPrompt != null), Is.False, + "no prompt row inside a subrecord that has no subrecords"); + } + [Test] public void Compose_ASubrecordOfASubrecord_ShowsItsFieldsOneLevelFurtherIn() { From 62e63cfcfde255409f65506c8f451a6305a3ca17 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Fri, 23 Oct 2026 19:01:17 -0400 Subject: [PATCH 4/5] LT-22858: Drop an unused using from DetailSummaryLines Co-Authored-By: Claude Opus 5.5 --- Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs b/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs index 39d687c82b..015b7f27c9 100644 --- a/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs +++ b/Src/xWorks/Avalonia/Composer/DetailSummaryLines.cs @@ -6,7 +6,6 @@ using System.Linq; using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; -using SIL.LCModel.Core.KernelInterfaces; using SIL.LCModel.Core.WritingSystems; using SIL.LCModel.Infrastructure; From e437db3ad26278e7dc400d393da0fbc5d98e3e19 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Mon, 26 Oct 2026 12:34:57 -0400 Subject: [PATCH 5/5] LT-22858: Raise the composer depth cap for nested subrecords Each level of subrecord nesting takes three composer levels, so the cap of 12 stopped at a fourth-level subrecord: its own Subrecords section was dropped, and Insert Subrecord, Insert Subrecord of Subrecord and Demote there made records seem to vanish. The WinForms view has no limit. The (object, layout) visited set already stops recursion, and the cap is only a backstop for malformed layouts, so it now allows 48 levels, about 16 levels of subrecords. Co-Authored-By: Claude Opus 5.5 --- .../Avalonia/Composer/DetailComposer.cs | 7 +++---- .../Composer/DetailSummaryLinesTests.cs | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 0f79200fba..aada0f21b0 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -63,10 +63,9 @@ public ComposedDetail(DetailModel model, IDetailEditContext editContext) /// public static class DetailComposer { - // The visited (hvo, layout) guard is the real recursion stop. This depth cap is only a - // backstop for malformed layouts, so it must still allow the deepest shipped lexeme-edit - // detail paths (e.g. sense -> extended note -> example -> nested custom fields). - private const int MaxDepth = 12; + // The visited (hvo, layout) guard stops recursion; this cap is a backstop for malformed + // layouts. Nested subrecords take three levels each and WinForms has no limit. + private const int MaxDepth = 48; private static readonly ViewDefinitionCompiler Compiler = new ViewDefinitionCompiler(); // Deliberately NOT a Lazy -- a failed load must not be cached as null for diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs index ba710b5de7..1dd92d6b72 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailSummaryLinesTests.cs @@ -357,6 +357,26 @@ public void Compose_ASubrecordOfASubrecord_ShowsItsFieldsOneLevelFurtherIn() Assert.That(subSubTitle.Indent, Is.EqualTo(subSubHeader.Indent + 1)); } + // Each nesting level takes three composer levels; the WinForms view has no limit, so a + // subrecord several levels down still shows, and a record inserted under it appears. + [Test] + public void Compose_DeeplyNestedSubrecords_AllShowTheirHeadersAndFields() + { + var record = RecordWithSubrecord(out var level); + Do(() => + { + for (var i = 2; i <= 8; i++) + level = AddSubrecord(level, "Level " + i); + }); + + var fields = DetailComposer.Compose(record, Cache).Model.Fields; + + Assert.That(fields.Any(f => f.Kind == DetailFieldKind.Header && f.ObjectHvo == level.Hvo), Is.True, + "the eighth-level subrecord has its header"); + Assert.That(RowsOf(fields, level).Any(f => f.Field == "Title"), Is.True, + "and its fields"); + } + [Test] public void Compose_EtymologiesAndExtendedNotes_HaveNoBanner_AndCarryTheirSummaryLines() {