From 66a7dc422b11eeb2a2ec1b566b28153f9dc8a3b2 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Thu, 12 Nov 2026 06:50:04 -0500 Subject: [PATCH 1/3] LT-22691: Apply a layout's part overrides through a conditional A row whose part wraps its slice in a conditional now shows the label and binds the menu its layout asks for. Until now the Avalonia detail view dropped a part ref's overrides when the part's content sat inside an , or , so the row kept the part's own values. In Grammar > Category Edit, an affix template's name row now reads "Template Name" and offers the template's menu. It read "Name" and offered only Help. The part ref's abbreviation, hotlinks, visibility, expansion, target layout, shown writing systems and child rows reach such a row the same way. Co-Authored-By: Claude Fable 5.2 --- .../LayoutImportCoverageTests.cs | 206 ++++++++++++++++++ .../ViewDefinition/XmlLayoutImporter.cs | 54 +++-- .../Composer/AdhocCoProhibComposeTests.cs | 45 ++++ .../Composer/DetailConditionalPartRefTests.cs | 85 ++++++++ ...xiconFirstSliceEditContextEdgeCaseTests.cs | 2 + 5 files changed, 375 insertions(+), 17 deletions(-) create mode 100644 Src/xWorks/xWorksTests/Avalonia/Composer/DetailConditionalPartRefTests.cs diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs index 43d13058e3..8be4b6ae96 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs @@ -333,6 +333,40 @@ public class ConditionalImportTests + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + "; private static ViewDefinitionModel Import(string className, string refName) @@ -342,6 +376,156 @@ private static ViewDefinitionModel Import(string className, string refName) $""), parts); } + private static ViewDefinitionModel ImportLayout(string className, string layoutContent) + { + var parts = new DictionaryPartResolver(XElement.Parse(PartsXml)); + return new XmlLayoutImporter().Import(XElement.Parse( + $"{layoutContent}"), parts); + } + + [Test] + public void PartRefOverrides_ReachASliceInsideEachIf() + { + var model = ImportLayout("MoInflAffixTemplate", + ""); + + Assert.That(model.Roots.Count, Is.EqualTo(2), "one conditional per "); + foreach (var conditional in model.Roots) + { + var slice = conditional.Children.Single(); + Assert.That(slice.Label, Is.EqualTo("Template Name")); + Assert.That(slice.Abbreviation, Is.EqualTo("TN")); + Assert.That(slice.MenuId, Is.EqualTo("mnuDataTree-POS-AffixTemplate")); + Assert.That(slice.HotlinksId, Is.EqualTo("mnuTemplate-Hotlinks")); + Assert.That(slice.Visibility, Is.EqualTo(ViewVisibility.IfData)); + } + } + + [Test] + public void PartRefOverrides_ReachSlicesInsideWhereAndOtherwise() + { + var model = ImportLayout("MoAffixAllomorph", + ""); + + var choice = model.Roots.Single(); + foreach (var branch in choice.Children) + { + var slice = branch.Children.Single(); + Assert.That(slice.Label, Is.EqualTo("Position")); + Assert.That(slice.MenuId, Is.EqualTo("mnuPosition")); + } + } + + [Test] + public void PartRefOverrides_ReachASliceInsideANestedConditional() + { + var model = ImportLayout("MoInflAffixTemplate", + ""); + + var slice = model.Roots.Single().Children.Single().Children.Single(); + Assert.That(slice.Kind, Is.EqualTo(ViewNodeKind.Field)); + Assert.That(slice.Label, Is.EqualTo("Template Name")); + } + + // A sequence item's heading takes its menu from the item layout's root nodes. + [Test] + public void PartRefLabelAndMenu_StayOnTheConditionalNode() + { + var model = ImportLayout("MoInflAffixTemplate", + ""); + + Assert.That(model.Roots.Select(r => r.Label), Is.All.EqualTo("Template Name")); + Assert.That(model.Roots.Select(r => r.MenuId), + Is.All.EqualTo("mnuDataTree-POS-AffixTemplate")); + } + + // Field Visibility reads and sets the row's own node, so that node holds the value. + [Test] + public void PartRefVisibility_IsHeldByTheContent_NotTheConditional() + { + var template = ImportLayout("MoInflAffixTemplate", + ""); + foreach (var conditional in template.Roots) + { + Assert.That(conditional.Visibility, Is.EqualTo(ViewVisibility.Always)); + Assert.That(conditional.Children.Single().Visibility, + Is.EqualTo(ViewVisibility.Never)); + } + + var choice = ImportLayout("PartOfSpeech", + "").Roots.Single(); + Assert.That(choice.Visibility, Is.EqualTo(ViewVisibility.Always)); + Assert.That(choice.Children[1].Children.Single().Visibility, + Is.EqualTo(ViewVisibility.Never)); + } + + [Test] + public void PartRefChildren_LandUnderTheSliceInsideTheConditional() + { + var model = ImportLayout("MoInflAffixTemplate", + ""); + + foreach (var conditional in model.Roots) + { + var child = conditional.Children.Single().Children.Single(); + Assert.That(child.Field, Is.EqualTo("Disabled")); + Assert.That(child.Indented, Is.True); + } + } + + [Test] + public void SequenceInsideAChoice_TakesTheTargetLayout_ButKeepsItsOwnLabel() + { + var model = ImportLayout("PartOfSpeech", + ""); + + var sequence = model.Roots.Single().Children[1].Children.Single(); + Assert.That(sequence.Kind, Is.EqualTo(ViewNodeKind.Sequence)); + Assert.That(sequence.TargetLayout, Is.EqualTo("nested")); + Assert.That(sequence.MenuId, Is.EqualTo("mnuSub")); + Assert.That(sequence.Label, Is.EqualTo("Own Label")); + Assert.That(sequence.Abbreviation, Is.Null); + } + + [Test] + public void ObjectInsideAnIf_TakesTheRefsLayoutAndChildren_ButNotItsLabel() + { + var model = ImportLayout("PartOfSpeech", + "" + + ""); + + var atom = model.Roots.Single().Children.Single(); + Assert.That(atom.Kind, Is.EqualTo(ViewNodeKind.ObjectAtom)); + Assert.That(atom.TargetLayout, Is.EqualTo("Brief")); + Assert.That(atom.Visibility, Is.EqualTo(ViewVisibility.IfData)); + Assert.That(atom.Label, Is.Null); + Assert.That(atom.Children.Single().Field, Is.EqualTo("Name")); + } + + [Test] + public void ConditionalWrittenInALayout_LeavesItsPartRefsAlone() + { + var model = ImportLayout("MoInflAffixTemplate", + ""); + + var conditional = model.Roots.Single(); + Assert.That(conditional.Label, Is.Null); + Assert.That(conditional.Children.Single().Label, Is.EqualTo("Active")); + Assert.That(model.Diagnostics, Is.Empty); + } + + [Test] + public void PartRefDiagnostics_AreReportedOncePerConditional_NotAgainPerSlice() + { + var model = ImportLayout("MoInflAffixTemplate", + ""); + + Assert.That(model.Diagnostics.Count(d => d.Code == "unhandled-attribute"), + Is.EqualTo(model.Roots.Count)); + } + [Test] public void If_ImportsConditionalNode_WithStructuredIntEqualsCondition() { @@ -787,6 +971,28 @@ public void ShippedLexEntryRefNormal_ImportsTheRefTypeConditionals() "every condition form LexEntryRef/Normal uses is in the supported set"); } + /// + /// The affix template's name part wraps its slice in a Disabled test, and its layout + /// renames the row and binds the template menu. + /// + [Test] + public void ShippedMoInflAffixTemplateDefault_NameRow_TakesTheLayoutLabelAndMenu() + { + var repoRoot = FindRepoRoot(); + var (layouts, parts) = LoadShippedFiles(repoRoot); + var model = ImportShippedLayout(repoRoot, layouts, parts, "MoInflAffixTemplate", "default"); + + var nameRows = model.Roots + .Where(r => r.Kind == ViewNodeKind.Conditional) + .SelectMany(r => r.Children) + .Where(n => n.Field == "Name") + .ToList(); + Assert.That(nameRows.Count, Is.EqualTo(2), "one per Disabled state"); + Assert.That(nameRows.Select(n => n.Label), Is.All.EqualTo("Template Name")); + Assert.That(nameRows.Select(n => n.MenuId), + Is.All.EqualTo("mnuDataTree-POS-AffixTemplate")); + } + /// /// The shipped lexeme-form ghost configuration must arrive complete on the typed /// node -- ghost/ghostWs/ghostLabel, the explicit ghostClass (MoStemAllomorph, differing diff --git a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs index cca2809a77..a0c72dd301 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs @@ -258,8 +258,13 @@ private ViewNode CreateNode( string layoutType, string stableId, bool indented, - List diagnostics) + List diagnostics, + bool insideConditional = false) { + // Content inside a part's conditional takes the part ref as its caller; the + // conditional's own node has already reported that part ref. + var hasPartRef = !ReferenceEquals(callerEl, contentEl); + var reportPartRef = hasPartRef && !insideConditional; var label = Attr(callerEl, "label") ?? Attr(contentEl, "label"); var abbreviation = Attr(callerEl, "abbr") ?? Attr(contentEl, "abbr"); var visibility = ParseVisibility(Attr(callerEl, "visibility") ?? Attr(contentEl, "visibility")); @@ -270,7 +275,7 @@ private ViewNode CreateNode( // Report attributes the importer drops instead of dropping them silently. The caller // element is reported only when distinct from the content element (inline children pass the // same element for both). - if (!ReferenceEquals(callerEl, contentEl)) + if (reportPartRef) { ReportUnhandledAttributes(callerEl, HandledCallerPartAttributes, "part ref", stableId, diagnostics); ReportSubstitutionValues(callerEl, HandledCallerPartAttributes, stableId, diagnostics); @@ -380,7 +385,7 @@ private ViewNode CreateNode( // part, e.g. AsLexemeForm's MorphTypeBasic) become child nodes, mirroring how // DataTree.ProcessPartRefNode realizes them as indented child slices. Other caller // child kinds are reported, not silently dropped. - if (!ReferenceEquals(callerEl, contentEl)) + if (hasPartRef) { var structuralCallerChildren = new List(); foreach (var callerChild in callerEl.Elements()) @@ -446,6 +451,15 @@ private ViewNode CreateNode( case "seq": { var kind = contentEl.Name.LocalName == "obj" ? ViewNodeKind.ObjectAtom : ViewNodeKind.Sequence; + // An object or sequence inside a conditional keeps its own label: the part + // ref's label names the section above it and would repeat as a second + // heading. + if (insideConditional) + { + label = Attr(contentEl, "label"); + abbreviation = Attr(contentEl, "abbr"); + localizationKey = Attr(contentEl, "localizationKey") ?? Attr(contentEl, "labelId"); + } var targetLayout = Attr(callerEl, "param") ?? Attr(contentEl, "layout"); ReportUnhandledAttributes(contentEl, HandledObjSeqAttributes, contentEl.Name.LocalName, stableId, diagnostics); ReportSubstitutionValues(contentEl, HandledObjSeqAttributes, stableId, diagnostics); @@ -477,13 +491,14 @@ private ViewNode CreateNode( return null; // unsupported condition form; diagnostic already raised var children = new List(); - AddConditionalChildren(contentEl, parts, className, layoutType, stableId, indented, - children, diagnostics); + AddConditionalChildren(contentEl, hasPartRef ? callerEl : null, parts, className, + layoutType, stableId, indented, children, diagnostics); + // The part ref's visibility is held by the content nodes, one per row. return new ViewNode(stableId, ViewNodeKind.Conditional, label, abbreviation, Attr(contentEl, "field"), null, EditorClassification.GroupingNone, null, - visibility, expansion, indented, null, children, localizationKey, automationId, - routing, menuId: menuId, contextMenuId: contextMenuId, hotlinksId: hotlinksId, - condition: condition); + ViewVisibility.Always, expansion, indented, null, children, localizationKey, + automationId, routing, menuId: menuId, contextMenuId: contextMenuId, + hotlinksId: hotlinksId, condition: condition); } // holds branches (first passing one renders) and an optional @@ -514,8 +529,8 @@ private ViewNode CreateNode( } var branchChildren = new List(); - AddConditionalChildren(clause, parts, className, layoutType, branchId, indented, - branchChildren, diagnostics); + AddConditionalChildren(clause, hasPartRef ? callerEl : null, parts, className, + layoutType, branchId, indented, branchChildren, diagnostics); branches.Add(new ViewNode(branchId, ViewNodeKind.Conditional, null, null, Attr(clause, "field"), null, EditorClassification.GroupingNone, null, ViewVisibility.Always, ViewExpansion.NotApplicable, indented, null, @@ -523,8 +538,8 @@ private ViewNode CreateNode( } return new ViewNode(stableId, ViewNodeKind.ChoiceGroup, label, abbreviation, null, - null, EditorClassification.GroupingNone, null, visibility, expansion, indented, - null, branches, localizationKey, automationId, routing, menuId: menuId, + null, EditorClassification.GroupingNone, null, ViewVisibility.Always, expansion, + indented, null, branches, localizationKey, automationId, routing, menuId: menuId, contextMenuId: contextMenuId, hotlinksId: hotlinksId); } @@ -606,11 +621,15 @@ private static ViewStringList ImportStringList( return new ViewStringList(idList, Attr(stringListEl, "group")); } - // A conditional wrapper's children are part content (//) - // inside part definitions, or / refs at layout level, - // matching DataTree.ProcessPartChildren's dispatch. + /// + /// Adds a conditional's children: part content (slice, seq, obj) when the conditional + /// sits in a part, or part and indent refs when it sits in a layout. Part content takes + /// the label, menu and other overrides of , the part ref + /// whose part holds the conditional; null for a conditional written in a layout. + /// private void AddConditionalChildren( XElement container, + XElement partRef, IPartResolver parts, string className, string layoutType, @@ -630,8 +649,9 @@ private void AddConditionalChildren( break; default: { - var node = CreateNode(child, child, parts, className, layoutType, - $"{parentPath}/#{output.Count}", indented, diagnostics); + var node = CreateNode(child, partRef ?? child, parts, className, layoutType, + $"{parentPath}/#{output.Count}", indented, diagnostics, + insideConditional: partRef != null); if (node != null) output.Add(node); break; diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/AdhocCoProhibComposeTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/AdhocCoProhibComposeTests.cs index 1427b7f513..54d1298fa4 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/AdhocCoProhibComposeTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/AdhocCoProhibComposeTests.cs @@ -175,5 +175,50 @@ public void Compose_MoAdhocProhibGr_ComposesMemberRuleRows() Assert.That(memberRows.Min(f => f.Indent), Is.GreaterThan(groupIndent), "the member's rows nest below the group's own rows"); } + + // A nested group's other rows are the child rows of its name row's part ref, and the + // name part holds one slice per state of the group's Active box. + [TestCase(false)] + [TestCase(true)] + public void Compose_ANestedGroup_ShowsItsNameDescriptionActiveAndMembers(bool disabled) + { + IMoAdhocProhibGr group = null; + IMoAdhocProhibGr nested = null; + IMoMorphAdhocProhib rule = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + var entry = Cache.ServiceLocator.GetInstance().Create(); + var msa = Cache.ServiceLocator.GetInstance().Create(); + entry.MorphoSyntaxAnalysesOC.Add(msa); + + group = Cache.ServiceLocator.GetInstance().Create(); + Cache.LangProject.MorphologicalDataOA.AdhocCoProhibitionsOC.Add(group); + nested = Cache.ServiceLocator.GetInstance().Create(); + group.MembersOC.Add(nested); + nested.Name.SetAnalysisDefaultWritingSystem("Inner group"); + nested.Disabled = disabled; + rule = Cache.ServiceLocator.GetInstance().Create(); + nested.MembersOC.Add(rule); + rule.FirstMorphemeRA = msa; + rule.RestOfMorphsRS.Add(msa); + }); + + var fields = DetailComposer.Compose(group, Cache, layoutName: "Edit", + plugins: SlicePluginRegistry.Default).Model.Fields; + TestContext.WriteLine(string.Join(", ", + fields.Select(f => $"{f.Field}/{f.Label}:{f.Kind}@{f.Indent}#{f.ObjectHvo}"))); + + var nestedRows = fields.Where(f => f.ObjectHvo == nested.Hvo && f.Kind != DetailFieldKind.Header).ToList(); + var name = nestedRows.Single(f => f.Field == "Name"); + Assert.That(name.Label, Is.EqualTo("Nested Group Name")); + Assert.That(name.MenuId, Is.EqualTo("mnuDataTree-Delete-Adhoc-Group")); + var description = nestedRows.Single(f => f.Field == "Description"); + Assert.That(description.Label, Is.EqualTo("Nested Group Description")); + Assert.That(description.Indent, Is.GreaterThan(name.Indent), "the other rows nest below the name row"); + Assert.That(nestedRows.Count(f => f.Field == "Disabled" && f.Kind == DetailFieldKind.Boolean), Is.EqualTo(1)); + var members = fields.Single(f => f.ObjectHvo == nested.Hvo && f.Label == "Nested Group Members"); + Assert.That(members.MenuId, Is.EqualTo("mnuDataTree-Adhoc-Group-Members")); + Assert.That(fields.Any(f => f.ObjectHvo == rule.Hvo), Is.True, "the nested group's own member composes"); + } } } diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailConditionalPartRefTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailConditionalPartRefTests.cs new file mode 100644 index 0000000000..333585f224 --- /dev/null +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailConditionalPartRefTests.cs @@ -0,0 +1,85 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Linq; +using NUnit.Framework; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Infrastructure; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// Rows of a grammatical category whose parts wrap their content in a conditional, composed + /// from the shipped layouts: the row shows what the layout's part ref asks for. + /// + [TestFixture] + public class DetailConditionalPartRefTests : MemoryOnlyBackendProviderTestBase + { + private IPartOfSpeech m_noun; + private IPartOfSpeech m_subcategory; + private IMoInflAffixTemplate m_template; + + public override void TestSetup() + { + base.TestSetup(); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + var services = Cache.ServiceLocator; + if (Cache.LangProject.PartsOfSpeechOA == null) + { + Cache.LangProject.PartsOfSpeechOA = + services.GetInstance().Create(); + } + // The Subcategories part shows nothing in a list too shallow to nest. + Cache.LangProject.PartsOfSpeechOA.Depth = 127; + + m_noun = services.GetInstance().Create(); + Cache.LangProject.PartsOfSpeechOA.PossibilitiesOS.Add(m_noun); + m_noun.Name.set_String(Cache.DefaultAnalWs, Analysis("Noun")); + + m_subcategory = services.GetInstance().Create(); + m_noun.SubPossibilitiesOS.Add(m_subcategory); + m_subcategory.Name.set_String(Cache.DefaultAnalWs, Analysis("Proper noun")); + + m_template = services.GetInstance().Create(); + m_noun.AffixTemplatesOS.Add(m_template); + m_template.Name.set_String(Cache.DefaultAnalWs, Analysis("Plural")); + }); + } + + private ITsString Analysis(string text) => TsStringUtils.MakeString(text, Cache.DefaultAnalWs); + + // The name part holds one slice per state of the template's Active box. + [TestCase(false)] + [TestCase(true)] + public void Compose_AnAffixTemplatesNameRow_TakesTheLayoutsLabelAndMenu(bool disabled) + { + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => m_template.Disabled = disabled); + + var fields = DetailComposer.Compose(m_noun, Cache, "default").Model.Fields; + + var name = fields.Single(f => f.ObjectHvo == m_template.Hvo && f.Field == "Name" + && f.Kind != DetailFieldKind.Header); + Assert.That(name.Label, Is.EqualTo("Template Name")); + Assert.That(name.MenuId, Is.EqualTo("mnuDataTree-POS-AffixTemplate")); + } + + [Test] + public void Compose_Subcategories_KeepOneHeading_AndOneNameRowPerSubcategory() + { + var fields = DetailComposer.Compose(m_noun, Cache, "default").Model.Fields; + + var headings = fields.Where(f => f.Kind == DetailFieldKind.Header).ToList(); + Assert.That(headings.Count(h => h.Label == "Subcategories"), Is.EqualTo(1)); + Assert.That(headings.Any(h => h.ObjectHvo == m_subcategory.Hvo), Is.False, + "a subcategory has no numbered heading of its own"); + var name = fields.Single(f => f.ObjectHvo == m_subcategory.Hvo && f.Field == "Name"); + Assert.That(name.Label, Is.EqualTo("Subcategory")); + } + } +} diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/LexiconFirstSliceEditContextEdgeCaseTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/LexiconFirstSliceEditContextEdgeCaseTests.cs index 7465f6eb95..b37f73abb9 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/LexiconFirstSliceEditContextEdgeCaseTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/LexiconFirstSliceEditContextEdgeCaseTests.cs @@ -111,6 +111,8 @@ public void Validate_WhitespaceOnlyLexeme_IsAnError() var context = new LexiconFirstSliceEditContext(entry, Cache); Assert.That(context.TrySetText(Field("Form"), "vern", " "), Is.True, "whitespace stages"); Assert.That(context.Validate(), Is.Not.Empty, "a whitespace-only lexeme form fails validation"); + // The staged write holds an undo task open; the fixture cannot be torn down with it. + context.Cancel(); } [Test] From eb24bedeef74b7352ead319e1bbc8d153a38c821 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Thu, 12 Nov 2026 14:18:00 -0500 Subject: [PATCH 2/3] LT-22691: Keep a layout-level choice's own visibility A written directly in a layout keeps its own visibility again. Until now every imported conditional was always visible; that rule is meant only for a conditional inside a part, whose part ref's visibility moves to the rows it holds. No shipped layout has such a choice, so nothing on screen changes. Co-Authored-By: Claude Opus 5.5 --- .../FwAvaloniaTests/LayoutImportCoverageTests.cs | 16 ++++++++++++++++ .../ViewDefinition/XmlLayoutImporter.cs | 13 +++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs index 8be4b6ae96..8187de45ee 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs @@ -516,6 +516,22 @@ public void ConditionalWrittenInALayout_LeavesItsPartRefsAlone() Assert.That(model.Diagnostics, Is.Empty); } + [Test] + public void ChoiceWrittenInALayout_KeepsItsOwnVisibility() + { + var choice = ImportLayout("MoInflAffixTemplate", + "" + + "").Roots.Single(); + Assert.That(choice.Kind, Is.EqualTo(ViewNodeKind.ChoiceGroup)); + Assert.That(choice.Visibility, Is.EqualTo(ViewVisibility.Never)); + + // An has no visibility of its own: the attribute is not a condition attribute. + var ifModel = ImportLayout("MoInflAffixTemplate", + ""); + Assert.That(ifModel.Roots, Is.Empty); + Assert.That(ifModel.Diagnostics.Single().Code, Is.EqualTo("conditional-dropped")); + } + [Test] public void PartRefDiagnostics_AreReportedOncePerConditional_NotAgainPerSlice() { diff --git a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs index a0c72dd301..3ec638284b 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs @@ -493,12 +493,12 @@ private ViewNode CreateNode( var children = new List(); AddConditionalChildren(contentEl, hasPartRef ? callerEl : null, parts, className, layoutType, stableId, indented, children, diagnostics); - // The part ref's visibility is held by the content nodes, one per row. + // A part ref's visibility is held by the content nodes, one per row. return new ViewNode(stableId, ViewNodeKind.Conditional, label, abbreviation, Attr(contentEl, "field"), null, EditorClassification.GroupingNone, null, - ViewVisibility.Always, expansion, indented, null, children, localizationKey, - automationId, routing, menuId: menuId, contextMenuId: contextMenuId, - hotlinksId: hotlinksId, condition: condition); + hasPartRef ? ViewVisibility.Always : visibility, expansion, indented, null, + children, localizationKey, automationId, routing, menuId: menuId, + contextMenuId: contextMenuId, hotlinksId: hotlinksId, condition: condition); } // holds branches (first passing one renders) and an optional @@ -538,8 +538,9 @@ private ViewNode CreateNode( } return new ViewNode(stableId, ViewNodeKind.ChoiceGroup, label, abbreviation, null, - null, EditorClassification.GroupingNone, null, ViewVisibility.Always, expansion, - indented, null, branches, localizationKey, automationId, routing, menuId: menuId, + null, EditorClassification.GroupingNone, null, + hasPartRef ? ViewVisibility.Always : visibility, expansion, indented, null, + branches, localizationKey, automationId, routing, menuId: menuId, contextMenuId: contextMenuId, hotlinksId: hotlinksId); } From 6ff99ad81704a5e4375e0bdaca426c04e1df9813 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:26:57 -0500 Subject: [PATCH 3/3] LT-22691: Hide every state's row of a part with Field Visibility Field Visibility on a row whose part holds one conditional per state now sets the row in every state. Until now it set only the row on screen, so an affix template's Description hidden while Active was ticked came back once Active was unticked. An entry ref's "Components" and "Variant of" rows come from one part ref, so hiding one now hides the other, as the WinForms detail view does. Co-Authored-By: Claude Opus 5.5 --- .../LayoutImportCoverageTests.cs | 28 +++++++++ .../ViewDefinitionJsonSerializer.cs | 2 + .../ViewDefinition/ViewDefinitionModel.cs | 11 +++- .../ViewDefinitionOverrideApplier.cs | 7 ++- .../ViewDefinitionOverrideEditor.cs | 63 +++++++++++++++---- .../ViewDefinition/XmlLayoutImporter.cs | 27 +++++--- .../Avalonia/Hosting/ObjectMenuAuthority.cs | 6 +- .../Hosting/RecordEditView.Avalonia.cs | 43 +++++++------ .../Avalonia/Hosting/CategoryEditMenuTests.cs | 46 ++++++++++++++ 9 files changed, 182 insertions(+), 51 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs index 8187de45ee..ead8c806a6 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/LayoutImportCoverageTests.cs @@ -460,6 +460,34 @@ public void PartRefVisibility_IsHeldByTheContent_NotTheConditional() Is.EqualTo(ViewVisibility.Never)); } + // Field Visibility on one branch's row reaches every branch of the same part ref. + [Test] + public void PartRefBranches_ShareOneVisibilityTarget() + { + var model = ImportLayout("MoInflAffixTemplate", + "" + + ""); + var first = model.Roots.Take(2).Select(c => c.Children.Single()).ToList(); + var second = model.Roots.Skip(2).Select(c => c.Children.Single()).ToList(); + var active = first[0].Children.Single(); + Assert.That(model.Roots.Select(r => r.PartRefId), Is.All.Null); + + // The applier and the canonical JSON both keep the part ref. + var hideOne = new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibility, + first[1].StableId, visibility: ViewVisibility.Never); + var patched = ViewDefinitionOverrideApplier.Apply( + ViewDefinitionJsonSerializer.Deserialize(ViewDefinitionJsonSerializer.Serialize(model)), + new ViewDefinitionOverride("MoInflAffixTemplate", "T", "detail", new[] { hideOne }, null)); + + Assert.That(ViewDefinitionOverrideEditor.LocateTarget(patched, first[1].StableId) + .VisibilityTargets, Is.EqualTo(first.Select(s => s.StableId))); + Assert.That(ViewDefinitionOverrideEditor.LocateTarget(patched, second[0].StableId) + .VisibilityTargets, Is.EqualTo(second.Select(s => s.StableId))); + Assert.That(ViewDefinitionOverrideEditor.LocateTarget(patched, active.StableId) + .VisibilityTargets, Is.EqualTo(new[] { active.StableId }), + "the part ref's own child rows are rows of their own"); + } + [Test] public void PartRefChildren_LandUnderTheSliceInsideTheConditional() { diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs index c59990e1d4..4bb5bc14fe 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs @@ -95,6 +95,7 @@ private static JObject WriteNode(ViewNode node) if (node.ForceIncludeEnglish) o["forceIncludeEnglish"] = true; AddIfPresent(o, "visField", node.VisibilityField); + AddIfPresent(o, "partRef", node.PartRefId); // The stored selection travels with the two facts above: options without the // checked set would round-trip a field into the wrong checkmarks. if (node.VisibleWritingSystems != null && node.VisibleWritingSystems.Count > 0) @@ -228,6 +229,7 @@ private static ViewNode ReadNode(JToken token) optionalWritingSystem: (string)o["optionalWs"], forceIncludeEnglish: (bool?)o["forceIncludeEnglish"] ?? false, visibilityField: (string)o["visField"], + partRefId: (string)o["partRef"], visibleWritingSystems: ((JArray)o["visibleWritingSystems"])?.Select(t => (string)t).ToList()); } diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs index 9dd2076aac..191efe6ede 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs @@ -398,8 +398,10 @@ public ViewNode( string sliceId = null, string optionalWritingSystem = null, bool forceIncludeEnglish = false, - string visibilityField = null) + string visibilityField = null, + string partRefId = null) { + PartRefId = partRefId; VisibilityField = visibilityField; OptionalWritingSystem = optionalWritingSystem; ForceIncludeEnglish = forceIncludeEnglish; @@ -508,6 +510,13 @@ public ViewNode( /// public string VisibilityField { get; } + /// + /// The layout part ref this node came from, when that part's content sits inside a + /// conditional: every branch's node then holds the part ref's own values, visibility + /// included. Null on every other node. + /// + public string PartRefId { get; } + public IReadOnlyList Children { get; } /// diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs index 4eb761c6fa..403e8b6de0 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs @@ -314,10 +314,11 @@ private static ViewNode CloneWith(ViewNode n, ViewVisibility visibility, string helpTopicId: n.HelpTopicId, sliceId: n.SliceId, optionalWritingSystem: n.OptionalWritingSystem, forceIncludeEnglish: n.ForceIncludeEnglish, - visibilityField: n.VisibilityField); + visibilityField: n.VisibilityField, + partRefId: n.PartRefId); - // Copy a (leaf) node under a new StableId; AutomationId is dropped so the duplicate gets a fresh, - // non-colliding identity (the renderer derives one from the new StableId by convention). + // Copy a (leaf) node under a new StableId, as a row of its own: no AutomationId + // (the renderer derives one from the new id) and no PartRefId. private static ViewNode CloneWithId(ViewNode n, string newId) => new ViewNode( newId, n.Kind, n.Label, n.Abbreviation, n.Field, n.RawEditor, n.EditorClassification, diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideEditor.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideEditor.cs index 96e8ddefdc..35767db4ea 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideEditor.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideEditor.cs @@ -68,14 +68,11 @@ public static ViewNodeLocation LocateTarget(ViewDefinitionModel model, string te // Root-level scan first (parent is null). var rootIndex = IndexOf(model.Roots, templateStableId); if (rootIndex >= 0) - { - return new ViewNodeLocation(model.Roots[rootIndex].Visibility, null, - model.Roots.Select(n => n.StableId).ToList(), rootIndex); - } + return At(model.Roots, null, model.Roots, rootIndex); foreach (var root in model.Roots) { - var found = LocateUnder(root, templateStableId); + var found = LocateUnder(model.Roots, root, templateStableId); if (found != null) return found; } @@ -83,18 +80,16 @@ public static ViewNodeLocation LocateTarget(ViewDefinitionModel model, string te return null; } - private static ViewNodeLocation LocateUnder(ViewNode parent, string templateStableId) + private static ViewNodeLocation LocateUnder(IReadOnlyList roots, ViewNode parent, + string templateStableId) { var index = IndexOf(parent.Children, templateStableId); if (index >= 0) - { - return new ViewNodeLocation(parent.Children[index].Visibility, parent.StableId, - parent.Children.Select(n => n.StableId).ToList(), index); - } + return At(roots, parent.StableId, parent.Children, index); foreach (var child in parent.Children) { - var found = LocateUnder(child, templateStableId); + var found = LocateUnder(roots, child, templateStableId); if (found != null) return found; } @@ -102,6 +97,36 @@ private static ViewNodeLocation LocateUnder(ViewNode parent, string templateStab return null; } + private static ViewNodeLocation At(IReadOnlyList roots, string parentStableId, + IReadOnlyList siblings, int index) + { + var node = siblings[index]; + return new ViewNodeLocation(node.Visibility, parentStableId, + siblings.Select(n => n.StableId).ToList(), index, VisibilityTargetsOf(roots, node)); + } + + // The node alone, or every node that holds the same part ref's visibility. + private static IReadOnlyList VisibilityTargetsOf(IReadOnlyList roots, + ViewNode node) + { + if (string.IsNullOrEmpty(node.PartRefId)) + return new[] { node.StableId }; + var ids = new List(); + CollectPartRefNodes(roots, node.PartRefId, ids); + return ids; + } + + private static void CollectPartRefNodes(IReadOnlyList nodes, string partRefId, + List ids) + { + foreach (var node in nodes) + { + if (string.Equals(node.PartRefId, partRefId, StringComparison.Ordinal)) + ids.Add(node.StableId); + CollectPartRefNodes(node.Children, partRefId, ids); + } + } + private static int IndexOf(IReadOnlyList nodes, string id) { for (var i = 0; i < nodes.Count; i++) @@ -178,15 +203,29 @@ public static ViewDefinitionOverride MergeOperation(ViewDefinitionOverride patch /// public sealed class ViewNodeLocation { + /// The template StableIds a visibility change writes; + /// null means the node alone. public ViewNodeLocation(ViewVisibility visibility, string parentStableId, - IReadOnlyList siblingOrder, int index) + IReadOnlyList siblingOrder, int index, + IReadOnlyList visibilityTargets = null) { Visibility = visibility; ParentStableId = parentStableId; SiblingOrder = siblingOrder ?? Array.Empty(); Index = index; + VisibilityTargets = visibilityTargets + ?? (index >= 0 && index < SiblingOrder.Count + ? new[] { SiblingOrder[index] } + : Array.Empty()); } + /// + /// The template StableIds a Field Visibility change writes, this node's among them. + /// A part ref whose content sits in a conditional has one node per branch; all of + /// them hold its visibility, so the change reaches every branch. + /// + public IReadOnlyList VisibilityTargets { get; } + /// The node's current visibility (after any override already applied to the model). public ViewVisibility Visibility { get; } diff --git a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs index 3ec638284b..ee550ccf1a 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs @@ -242,7 +242,7 @@ private void ProcessPart( // Each sibling after the first needs its own stable id so they don't collide. var childStableId = i == 0 ? stableId : $"{parentPath}/#{output.Count}"; var node = CreateNode(contents[i], callerEl, parts, className, layoutType, childStableId, - indented, diagnostics); + indented, diagnostics, partRefId: stableId); if (node != null) { output.Add(node); @@ -259,12 +259,15 @@ private ViewNode CreateNode( string stableId, bool indented, List diagnostics, - bool insideConditional = false) + bool insideConditional = false, + string partRefId = null) { // Content inside a part's conditional takes the part ref as its caller; the // conditional's own node has already reported that part ref. var hasPartRef = !ReferenceEquals(callerEl, contentEl); var reportPartRef = hasPartRef && !insideConditional; + // Each branch's node holds the part ref's values, so each records which part ref. + var heldPartRefId = insideConditional ? partRefId : null; var label = Attr(callerEl, "label") ?? Attr(contentEl, "label"); var abbreviation = Attr(callerEl, "abbr") ?? Attr(contentEl, "abbr"); var visibility = ParseVisibility(Attr(callerEl, "visibility") ?? Attr(contentEl, "visibility")); @@ -417,7 +420,8 @@ private ViewNode CreateNode( sliceId: Attr(contentEl, "id"), helpTopicId: Attr(contentEl, "helpTopicID"), optionalWritingSystem: optionalWs, - forceIncludeEnglish: forceEnglish); + forceIncludeEnglish: forceEnglish, + partRefId: heldPartRefId); } // Dynamic custom slices keep their legacy class/assembly identity so the host can @@ -445,7 +449,8 @@ private ViewNode CreateNode( helpTopicId: Attr(contentEl, "helpTopicID"), optionalWritingSystem: optionalWs, forceIncludeEnglish: forceEnglish, - visibilityField: Attr(contentEl, "visField")); + visibilityField: Attr(contentEl, "visField"), + partRefId: heldPartRefId); } case "obj": case "seq": @@ -477,7 +482,8 @@ private ViewNode CreateNode( ghostLabel: Attr(contentEl, "ghostLabel") ?? Attr(callerEl, "ghostLabel"), // The layout's post-create hook rides the node so the composer's ghost // setter can invoke it the way GhostStringSliceView.MakeRealObject does. - ghostInitMethod: Attr(contentEl, "ghostInitMethod") ?? Attr(callerEl, "ghostInitMethod")); + ghostInitMethod: Attr(contentEl, "ghostInitMethod") ?? Attr(callerEl, "ghostInitMethod"), + partRefId: heldPartRefId); } // Conditional display: / shows content only when the condition // passes (fails, for ifnot), evaluated via XmlVc.ConditionPasses. Preserved @@ -491,8 +497,8 @@ private ViewNode CreateNode( return null; // unsupported condition form; diagnostic already raised var children = new List(); - AddConditionalChildren(contentEl, hasPartRef ? callerEl : null, parts, className, - layoutType, stableId, indented, children, diagnostics); + AddConditionalChildren(contentEl, hasPartRef ? callerEl : null, partRefId, parts, + className, layoutType, stableId, indented, children, diagnostics); // A part ref's visibility is held by the content nodes, one per row. return new ViewNode(stableId, ViewNodeKind.Conditional, label, abbreviation, Attr(contentEl, "field"), null, EditorClassification.GroupingNone, null, @@ -529,8 +535,8 @@ private ViewNode CreateNode( } var branchChildren = new List(); - AddConditionalChildren(clause, hasPartRef ? callerEl : null, parts, className, - layoutType, branchId, indented, branchChildren, diagnostics); + AddConditionalChildren(clause, hasPartRef ? callerEl : null, partRefId, parts, + className, layoutType, branchId, indented, branchChildren, diagnostics); branches.Add(new ViewNode(branchId, ViewNodeKind.Conditional, null, null, Attr(clause, "field"), null, EditorClassification.GroupingNone, null, ViewVisibility.Always, ViewExpansion.NotApplicable, indented, null, @@ -631,6 +637,7 @@ private static ViewStringList ImportStringList( private void AddConditionalChildren( XElement container, XElement partRef, + string partRefId, IPartResolver parts, string className, string layoutType, @@ -652,7 +659,7 @@ private void AddConditionalChildren( { var node = CreateNode(child, partRef ?? child, parts, className, layoutType, $"{parentPath}/#{output.Count}", indented, diagnostics, - insideConditional: partRef != null); + insideConditional: partRef != null, partRefId: partRefId); if (node != null) output.Add(node); break; diff --git a/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs index 5e444e124c..58c156342d 100644 --- a/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs @@ -121,15 +121,11 @@ private DetailMenuItem HelpItem(string label) /// internal sealed class OverrideTarget { - public OverrideTarget(string templateId, ViewNodeLocation location) + public OverrideTarget(ViewNodeLocation location) { - TemplateId = templateId ?? throw new ArgumentNullException(nameof(templateId)); Location = location ?? throw new ArgumentNullException(nameof(location)); } - /// The row's template id, without its runtime suffix. - public string TemplateId { get; } - /// The node's position and visibility among its siblings. public ViewNodeLocation Location { get; } } diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index 59158cd365..7ba614c352 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -765,14 +765,14 @@ internal void AddOverrideCommands(OverrideCommandRegistry registry, DetailField // Unknown/stale target: leave the field commands on mediator dispatch rather than // guess. - if (!TryLocateOverrideTarget(field, out var templateId, out var location)) + if (!TryLocateOverrideTarget(field, out _, out var location)) return; registry.Add("CmdAlwaysVisible", - (c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.Always)); + (c, d) => VisibilityItem(LabelOf(d), field, location, ViewVisibility.Always)); registry.Add("CmdIfData", - (c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.IfData)); + (c, d) => VisibilityItem(LabelOf(d), field, location, ViewVisibility.IfData)); registry.Add("CmdNormallyHidden", - (c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.Never)); + (c, d) => VisibilityItem(LabelOf(d), field, location, ViewVisibility.Never)); registry.Add("CmdDataTree-MoveFieldUp", (c, d) => MoveItem(LabelOf(d), field, location, up: true)); registry.Add("CmdDataTree-MoveFieldDown", @@ -817,7 +817,7 @@ private IDetailMenuAuthority CreateMultiStringMenuAuthority(DetailField field, internal IDetailMenuAuthority CreateObjectMenuAuthority(DetailField field) => new ObjectMenuAuthority(field, LocateOverrideTarget, fieldVisibility: (label, target, visibility) => - VisibilityItem(label, field, target.TemplateId, target.Location, visibility), + VisibilityItem(label, field, target.Location, visibility), moveField: (label, target, up) => MoveItem(label, field, target.Location, up), helpTopic: KnownHelpTopic, showHelp: ShowDetailHelp); @@ -828,8 +828,8 @@ private OverrideTarget LocateOverrideTarget(DetailField field) { try { - return TryLocateOverrideTarget(field, out var templateId, out var location) - ? new OverrideTarget(templateId, location) + return TryLocateOverrideTarget(field, out _, out var location) + ? new OverrideTarget(location) : null; } catch (Exception e) @@ -929,11 +929,11 @@ private static bool TryTemplateIdOf(string stableId, out string templateId) // SetVisibility override mutation (idempotent -- re-choosing the current value is a // harmless write). private DetailMenuItem VisibilityItem(string label, DetailField field, - string templateId, ViewNodeLocation location, ViewVisibility target) + ViewNodeLocation location, ViewVisibility target) { var isChecked = location.Visibility == target; return new DetailMenuItem(label, isEnabled: true, isChecked: isChecked, children: null, - execute: () => ApplyFieldVisibility(field, templateId, target)); + execute: () => ApplyFieldVisibility(field, location, target)); } // A Move Field item: disabled at the first sibling (up) / last sibling (down) / when alone. @@ -1034,12 +1034,14 @@ private DetailMenuItem ConfigureWritingSystemsItem(string label, DetailField fie } }); - // Writes a SetVisibility op for the field's template id into the project override and recomposes. - private void ApplyFieldVisibility(DetailField field, string templateId, ViewVisibility target) + // Writes a SetVisibility op for each of the location's visibility targets into the + // project override and recomposes. + private void ApplyFieldVisibility(DetailField field, ViewNodeLocation location, + ViewVisibility target) { - var op = new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibility, templateId, - visibility: target); - MutateOverrideAndRefresh(field, op); + var ops = location.VisibilityTargets.Select(id => new ViewOverrideOperation( + ViewOverrideOperationKind.SetVisibility, id, visibility: target)).ToArray(); + MutateOverrideAndRefresh(field, ops); } // Writes a ReorderChildren op on the field's PARENT into the project override, then @@ -1058,15 +1060,15 @@ private void ApplyMoveField(DetailField field, ViewNodeLocation location, bool u // Loads-or-creates the (class, layout) override, folds the op in, saves it, and // recomposes the Avalonia detail view. Only the override file is written -- not the // shipped XML or the part-ref inventory. - private void MutateOverrideAndRefresh(DetailField field, ViewOverrideOperation op) + private void MutateOverrideAndRefresh(DetailField field, params ViewOverrideOperation[] ops) { - if (TryMutateOverride(field, op)) + if (TryMutateOverride(field, ops)) RefreshAvaloniaDetail(); } - // Folds the op into the (class, layout) override and saves it, WITHOUT recomposing. + // Folds the ops into the (class, layout) override and saves it once, WITHOUT recomposing. // Returns whether the save succeeded, so a failed save can leave view state untouched. - private bool TryMutateOverride(DetailField field, ViewOverrideOperation op) + private bool TryMutateOverride(DetailField field, params ViewOverrideOperation[] ops) { try { @@ -1074,9 +1076,10 @@ private bool TryMutateOverride(DetailField field, ViewOverrideOperation op) if (store == null) return false; - var existing = store.TryGet(field.ClassName, field.LayoutName) + var merged = store.TryGet(field.ClassName, field.LayoutName) ?? new ViewDefinitionOverride(field.ClassName, field.LayoutName, "detail", null, null); - var merged = ViewDefinitionOverrideEditor.MergeOperation(existing, op); + foreach (var op in ops) + merged = ViewDefinitionOverrideEditor.MergeOperation(merged, op); store.Save(merged); return true; } diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/CategoryEditMenuTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/CategoryEditMenuTests.cs index 571fd13dc5..12a421e15c 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/CategoryEditMenuTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/CategoryEditMenuTests.cs @@ -5,9 +5,11 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using NUnit.Framework; using SIL.FieldWorks.Common.Controls; using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; using SIL.LCModel.Infrastructure; @@ -286,6 +288,42 @@ public void ExceptionFeatures_AnswersEveryRowMenuNatively_AndInsertsASubitemInAH Assert.That(m_restriction.SubPossibilitiesOS, Has.Count.EqualTo(1)); } + // The template's Description part holds one row per state of Active; hiding the row + // holds in both. + [Test] + public void FieldVisibility_OnATemplatesDescription_HoldsWhenActiveIsUnticked() + { + ShowCategory(m_verb); + var row = DescriptionRow(m_finite); + Assert.That(row, Is.Not.Null, "precondition: an active template shows its Description"); + var store = OverrideStore(); + store.Save(new ViewDefinitionOverride(row.ClassName, row.LayoutName, "detail", null, null)); + try + { + var window = m_propertyTable.GetValue("window"); + var items = XCoreMenuBridge.CreateMenuItems(window, new[] { ObjectMenuAuthority.MenuId }, + null, null, m_view.CreateObjectMenuAuthority(row)); + Run(Item(items, "Field Visibility").Children, "Normally hidden"); + DrainMediatorAndIdleQueues(); + + var ops = store.TryGet(row.ClassName, row.LayoutName).Operations; + Assert.That(ops.Select(o => o.Kind), Is.All.EqualTo(ViewOverrideOperationKind.SetVisibility)); + Assert.That(ops, Has.Count.EqualTo(2), "one write per state of Active"); + Assert.That(ops.Select(o => o.Visibility), Is.All.EqualTo(ViewVisibility.Never)); + Assert.That(DescriptionRow(m_finite), Is.Null, "hidden while active"); + + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => m_finite.Disabled = true); + m_view.RefreshDisplay(); + DrainMediatorAndIdleQueues(); + + Assert.That(DescriptionRow(m_finite), Is.Null, "still hidden once Active is unticked"); + } + finally + { + store.Save(new ViewDefinitionOverride(row.ClassName, row.LayoutName, "detail", null, null)); + } + } + // Verb (two slots, two templates, the subcategory Irregular) and Noun in a // hierarchical category list; one exception feature in its flat list. private void CreateData() @@ -341,6 +379,14 @@ private DetailField TemplateRow(IMoInflAffixTemplate template) private DetailField SubcategoriesRow() => ShownRows().First(f => f.MenuId == SubcategoriesMenuId); + private DetailField DescriptionRow(IMoInflAffixTemplate template) + => ShownRows().FirstOrDefault(f => f.ObjectHvo == template.Hvo && f.Field == "Description"); + + private ViewDefinitionOverrideStore OverrideStore() + => (ViewDefinitionOverrideStore)typeof(RecordEditView) + .GetProperty("ViewOverrideStore", BindingFlags.Instance | BindingFlags.NonPublic) + .GetValue(m_view); + // The menus of the rows that some id leaves to the hidden tree. private List UnansweredMenus(IEnumerable rows) {