From f3fa2449995e3922a843a347659cf7fbecad978c Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:29:38 -0400 Subject: [PATCH 1/4] LT-22691: Share the writing-system option rule across detail views A multi-string field's Writing Systems menu offers more writing systems than the Avalonia composer would resolve, so choosing one the project had not checked did nothing, and then clearing the others showed every writing system instead of the one chosen. Each detail view also worked out the menu for itself, four rules apiece, free to drift. FieldWritingSystemOptions now answers what a field may offer, what it shows and what may be switched off. Both views call it: the WinForms slice for its menu and its visible set, the Avalonia composer for what a multi-string row renders. The Avalonia menu still runs on the old path; that part follows. Two visible changes, both in the Avalonia detail view. Choosing a writing system the project has not checked now shows it in the row, where before nothing happened. Show all right now reveals every option rather than only the checked ones, as the WinForms slice does. Co-Authored-By: Claude Opus 5 --- .../MultiStringSliceWritingSystemsTests.cs | 62 +++++ .../DetailControls/MultiStringSlice.cs | 60 +++-- Src/Common/FwAvalonia/Detail/DetailModel.cs | 21 ++ .../FwAvaloniaTests/ViewDefinitionTests.cs | 35 ++- .../ViewDefinitionJsonSerializer.cs | 12 +- .../ViewDefinition/ViewDefinitionModel.cs | 20 +- .../ViewDefinitionOverrideApplier.cs | 8 +- .../ViewDefinition/XmlLayoutImporter.cs | 16 +- .../DetailRules/FieldWritingSystemOptions.cs | 241 ++++++++++++++++++ .../FieldWritingSystemOptionsTests.cs | 203 +++++++++++++++ .../Avalonia/Composer/DetailComposer.cs | 54 +++- .../Composer/DetailEditContextEditingTests.cs | 20 ++ .../Composer/FieldTypeComposerTests.cs | 63 +++++ 13 files changed, 772 insertions(+), 43 deletions(-) create mode 100644 Src/Common/Controls/DetailControls/DetailControlsTests/MultiStringSliceWritingSystemsTests.cs create mode 100644 Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs create mode 100644 Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs diff --git a/Src/Common/Controls/DetailControls/DetailControlsTests/MultiStringSliceWritingSystemsTests.cs b/Src/Common/Controls/DetailControls/DetailControlsTests/MultiStringSliceWritingSystemsTests.cs new file mode 100644 index 0000000000..a69879c253 --- /dev/null +++ b/Src/Common/Controls/DetailControls/DetailControlsTests/MultiStringSliceWritingSystemsTests.cs @@ -0,0 +1,62 @@ +// 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 System.Windows.Forms; +using NUnit.Framework; +using SIL.FieldWorks.Common.DetailRules; +using SIL.FieldWorks.Common.Widgets; +using SIL.LCModel; +using SIL.LCModel.DomainServices; + +namespace SIL.FieldWorks.Common.Framework.DetailControls +{ + /// + /// The WinForms Writing Systems menu is answered by the rule the Avalonia detail view also + /// uses. These pin that rule against the view's own writing-system query, so the two detail + /// views cannot offer different writing systems. + /// + [TestFixture] + public class MultiStringSliceWritingSystemsTests : MemoryOnlyBackendProviderTestBase + { + private const int MoFormFormFlid = 5035001; + + [TestCase(WritingSystemServices.kwsAnals, 0)] + [TestCase(WritingSystemServices.kwsVerns, 0)] + [TestCase(WritingSystemServices.kwsAnalVerns, 0)] + // The Pronunciation form, the one shipped part that offers a second set on request. + [TestCase(WritingSystemServices.kwsPronunciations, WritingSystemServices.kwsVerns)] + public void SharedRule_OffersExactlyWhatTheViewOffered(int wsMagic, int optionalMagic) + { + var hvo = Cache.LangProject.Hvo; + using (var host = new Form()) + using (var view = new LabeledMultiStringView(hvo, MoFormFormFlid, wsMagic, optionalMagic, + false, true, true)) + { + host.Controls.Add(view); + view.InnerView.Cache = Cache; + var spec = WritingSystemFieldSpec.FromMagicIds(hvo, wsMagic, optionalMagic, false); + + Assert.That(FieldWritingSystemOptions.Options(Cache, spec).Select(ws => ws.Id), + Is.EqualTo(view.WritingSystemOptions.Select(ws => ws.Id)), + "the menu must offer the writing systems the view offered, in the same order"); + Assert.That(FieldWritingSystemOptions.DefaultShown(Cache, spec).Select(ws => ws.Id), + Is.EqualTo(view.GetWritingSystemOptions(false).Select(ws => ws.Id)), + "and default to the same subset when the field stores no selection"); + } + } + + [Test] + public void SharedRule_OffersMoreThanItShows_WhenTheProjectHasAnUncheckedWritingSystem() + { + var hvo = Cache.LangProject.Hvo; + var spec = WritingSystemFieldSpec.FromMagicIds(hvo, WritingSystemServices.kwsAnals, 0, false); + var offered = FieldWritingSystemOptions.Options(Cache, spec).Select(ws => ws.Id).ToList(); + var shown = FieldWritingSystemOptions.DefaultShown(Cache, spec).Select(ws => ws.Id).ToList(); + + Assert.That(shown, Is.SubsetOf(offered), + "everything shown must be offerable; the menu may offer more than the field shows"); + } + } +} diff --git a/Src/Common/Controls/DetailControls/MultiStringSlice.cs b/Src/Common/Controls/DetailControls/MultiStringSlice.cs index 788ded83b7..3efbb56b05 100644 --- a/Src/Common/Controls/DetailControls/MultiStringSlice.cs +++ b/Src/Common/Controls/DetailControls/MultiStringSlice.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2015-2017 SIL International +// Copyright (c) 2015-2017 SIL International // This software is licensed under the LGPL, version 2.1 or later // (http://www.gnu.org/licenses/lgpl-2.1.html) @@ -10,6 +10,7 @@ using System.Xml; using SIL.LCModel.Core.WritingSystems; using SIL.FieldWorks.Common.Controls; +using SIL.FieldWorks.Common.DetailRules; using SIL.FieldWorks.Common.Framework.DetailControls.Resources; using SIL.LCModel.Core.KernelInterfaces; using SIL.FieldWorks.Common.FwUtils; @@ -28,8 +29,17 @@ namespace SIL.FieldWorks.Common.Framework.DetailControls /// public class MultiStringSlice : ViewPropertySlice, IWritingSystemChooser { + // The writing-system sets this slice was built for, kept so the Writing Systems menu can + // be answered by the rule both detail views share. + private int m_wsMagic; + private int m_wsMagicOptional; + private bool m_forceIncludeEnglish; + public MultiStringSlice(ICmObject obj, int flid, int ws, int wsOptional, bool forceIncludeEnglish, bool editable, bool spellCheck) { + m_wsMagic = ws; + m_wsMagicOptional = wsOptional; + m_forceIncludeEnglish = forceIncludeEnglish; var view = new LabeledMultiStringView(obj.Hvo, flid, ws, wsOptional, forceIncludeEnglish, editable, spellCheck); Control = view; #if _DEBUG @@ -63,6 +73,9 @@ public override RootSite RootSite /// public void Reuse(ICmObject obj, int flid, int ws, int wsOptional, bool forceIncludeEnglish, bool editable, bool spellCheck) { + m_wsMagic = ws; + m_wsMagicOptional = wsOptional; + m_forceIncludeEnglish = forceIncludeEnglish; var view = (LabeledMultiStringView)Control; Label = null; // new slice normally has this SetupWssToDisplay(); @@ -109,10 +122,19 @@ void HandleRightMouseClickedEvent(SimpleRootSite sender, FwRightMouseClickEventA /// /// public IEnumerable GetVisibleWritingSystems() + => FieldWritingSystemOptions.Shown(m_cache, WritingSystemSpec(), StoredWritingSystems()); + + // The sets this field draws on, as the shared rule takes them. + private WritingSystemFieldSpec WritingSystemSpec() + => WritingSystemFieldSpec.FromMagicIds(Object?.Hvo ?? 0, m_wsMagic, + m_wsMagicOptional, m_forceIncludeEnglish); + + // The selection persisted on this field's part ref, or null when it has none and the + // field shows its default set. + private IReadOnlyList StoredWritingSystems() { - var singlePropertySequenceValue = StringSliceUtils.GetVisibleWSSPropertyValue(PartRef(), - ((LabeledMultiStringView) Control).GetWritingSystemOptions(false)); - return StringSliceUtils.GetVisibleWritingSystems(singlePropertySequenceValue, WritingSystemOptionsForDisplay); + var stored = XmlUtils.GetOptionalAttributeValue(PartRef(), "visibleWritingSystems", null); + return stored == null ? null : ChoiceGroup.DecodeSinglePropertySequenceValue(stored); } public override void Install(DataTree parent) @@ -251,30 +273,18 @@ public bool OnDisplayWritingSystemOptionsForSlice(object parameter, ref UIListDi { CheckDisposed(); display.List.Clear(); - m_propertyTable.SetProperty(display.PropertyName, StringSliceUtils.GetVisibleWSSPropertyValue(PartRef(), - ((LabeledMultiStringView) Control).GetWritingSystemOptions(false)), false); - AddWritingSystemListWithIcuLocales(display, WritingSystemOptionsForDisplay); + // Options, checkmarks and the cannot-empty-the-set rule all come from the shared + // rule, so this menu and the Avalonia one offer the same list. + var options = FieldWritingSystemOptions.Menu(m_cache, WritingSystemSpec(), + StoredWritingSystems()); + m_propertyTable.SetProperty(display.PropertyName, + ChoiceGroup.EncodeSinglePropertySequenceValue( + options.Where(o => o.IsChecked).Select(o => o.Id).ToArray()), false); + foreach (var option in options) + display.List.Add(option.Label, option.Id, null, null, option.CanUncheck); return true;//we handled this, no need to ask anyone else. } - /// - /// stores the list values in terms of icu locale - /// - /// - /// - private void AddWritingSystemListWithIcuLocales(UIListDisplayProperties display, IEnumerable list) - { - var active = StringSliceUtils.GetVisibleWSSPropertyValue(PartRef(), - ((LabeledMultiStringView) Control).GetWritingSystemOptions(false)).Split(','); - foreach (var ws in list) - { - // generally enable all items, but if only one is checked that one is disabled; - // it can't be turned off. - bool enabled = (active.Length != 1 || ws.Id != active[0]); - display.List.Add(ws.DisplayLabel, ws.Id, null, null, enabled); - } - } - /// ------------------------------------------------------------------------------------ /// /// Called when property changed. diff --git a/Src/Common/FwAvalonia/Detail/DetailModel.cs b/Src/Common/FwAvalonia/Detail/DetailModel.cs index f6a02160aa..d64d064bc1 100644 --- a/Src/Common/FwAvalonia/Detail/DetailModel.cs +++ b/Src/Common/FwAvalonia/Detail/DetailModel.cs @@ -1707,6 +1707,27 @@ public DetailField( /// public bool IsMultiStringRow { get; set; } + /// + /// The layout's optionalWs spec: writing systems this row can be asked to show + /// beyond its set, but never shows by default. Null on every + /// shipped row but the Pronunciation form. Only the Writing Systems menu reads it. + /// + public string OptionalWritingSystem { get; set; } + + /// + /// The layout's forceIncludeEnglish flag: English joins this row's writing-system + /// options even when the project has not checked it. Only the Writing Systems menu + /// reads it. + /// + public bool ForceIncludeEnglish { get; set; } + + /// + /// The row's stored per-writing-system selection (the layout's + /// visibleWritingSystems, as the project override holds it), or null when the row + /// stores none and shows its default set. The Writing Systems menu checks these. + /// + public IReadOnlyList VisibleWritingSystems { get; set; } + /// /// How this row's domain decides two option names are the same name, for rows that mint /// items from typed text. Environments compare with spaces stripped, so the picker must diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs index 357351af03..99de2d24b7 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2026 SIL International +// 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) @@ -49,6 +49,10 @@ public class XmlLayoutImporterTests + + + "; private static ViewDefinitionModel Import(string layoutXml) @@ -195,6 +199,35 @@ public void Import_NoVisibleWritingSystems_LeavesOverrideNull() "a field with no override shows the full configured set (null = no restriction)"); } + [Test] + public void Import_OptionalWsAndForceIncludeEnglish_RideTheNode() + { + // Both widen what the field's Writing Systems menu may OFFER without changing what + // the field shows. Only the Pronunciation form uses optionalWs in the shipped parts. + var model = Import(@" + + +"); + + var field = model.Roots[0]; + Assert.That(field.WritingSystem, Is.EqualTo("all pronunciation")); + Assert.That(field.OptionalWritingSystem, Is.EqualTo("all vernacular")); + Assert.That(field.ForceIncludeEnglish, Is.True); + } + + [Test] + public void Import_NoOptionalWs_LeavesTheWideningFactsUnset() + { + var model = Import(@" + + +"); + + Assert.That(model.Roots[0].OptionalWritingSystem, Is.Null, + "a part with no optionalWs offers only its own writing systems"); + Assert.That(model.Roots[0].ForceIncludeEnglish, Is.False); + } + [Test] public void Import_DynamicEditor_RaisesInfoDiagnostic() { diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs index 2698e3ea5b..dbf6380723 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionJsonSerializer.cs @@ -91,6 +91,13 @@ private static JObject WriteNode(ViewNode node) AddIfPresent(o, "hotlinks", node.HotlinksId); AddIfPresent(o, "helpTopicID", node.HelpTopicId); AddIfPresent(o, "sliceId", node.SliceId); + AddIfPresent(o, "optionalWs", node.OptionalWritingSystem); + if (node.ForceIncludeEnglish) + o["forceIncludeEnglish"] = true; + // 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) + o["visibleWritingSystems"] = new JArray(node.VisibleWritingSystems); AddIfPresent(o, "ghost", node.GhostField); AddIfPresent(o, "ghostWs", node.GhostWs); AddIfPresent(o, "ghostClass", node.GhostClass); @@ -216,7 +223,10 @@ private static ViewNode ReadNode(JToken token) condition: ReadCondition((JObject)o["condition"]), chooserLinks: ((JArray)o["chooserLinks"])?.Select(ReadChooserLink).ToList(), helpTopicId: (string)o["helpTopicID"], - sliceId: (string)o["sliceId"]); + sliceId: (string)o["sliceId"], + optionalWritingSystem: (string)o["optionalWs"], + forceIncludeEnglish: (bool?)o["forceIncludeEnglish"] ?? false, + visibleWritingSystems: ((JArray)o["visibleWritingSystems"])?.Select(t => (string)t).ToList()); } private static T ParseEnum(JObject o, string name, T fallback) where T : struct diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs index 920b54228d..207393e3ce 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionModel.cs @@ -388,8 +388,12 @@ public ViewNode( bool toggleValue = false, bool reorder = false, string helpTopicId = null, - string sliceId = null) + string sliceId = null, + string optionalWritingSystem = null, + bool forceIncludeEnglish = false) { + OptionalWritingSystem = optionalWritingSystem; + ForceIncludeEnglish = forceIncludeEnglish; SliceId = sliceId; HelpTopicId = helpTopicId; Reorder = reorder; @@ -473,6 +477,20 @@ public ViewNode( /// public IReadOnlyList VisibleWritingSystems { get; } + /// + /// The part's optionalWs attribute: writing systems a multistring field can be + /// asked to show beyond its set, but never shows by default. + /// Null when the part has none, which is every shipped part but the Pronunciation form. + /// + public string OptionalWritingSystem { get; } + + /// + /// The part's forceIncludeEnglish attribute: English joins this field's + /// writing-system options even when the project has not checked it. Set on the reversal + /// name and abbreviation parts. + /// + public bool ForceIncludeEnglish { get; } + public IReadOnlyList Children { get; } /// diff --git a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs index 54c968eaed..829854b740 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/ViewDefinitionOverrideApplier.cs @@ -311,7 +311,9 @@ private static ViewNode CloneWith(ViewNode n, ViewVisibility visibility, string n.ContextMenuId, n.HotlinksId, n.GhostField, n.GhostWs, n.GhostClass, n.GhostLabel, n.ForVariant, n.CustomEditorClass, n.CustomEditorAssembly, n.GhostInitMethod, n.Condition, n.ChooserLinks, n.EnumStringList, visibleWritingSystems, n.ToggleValue, n.Reorder, - helpTopicId: n.HelpTopicId, sliceId: n.SliceId); + helpTopicId: n.HelpTopicId, sliceId: n.SliceId, + optionalWritingSystem: n.OptionalWritingSystem, + forceIncludeEnglish: n.ForceIncludeEnglish); // 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). @@ -323,6 +325,8 @@ private static ViewNode CloneWithId(ViewNode n, string newId) n.ContextMenuId, n.HotlinksId, n.GhostField, n.GhostWs, n.GhostClass, n.GhostLabel, n.ForVariant, n.CustomEditorClass, n.CustomEditorAssembly, n.GhostInitMethod, n.Condition, n.ChooserLinks, n.EnumStringList, n.VisibleWritingSystems, n.ToggleValue, n.Reorder, - helpTopicId: n.HelpTopicId, sliceId: n.SliceId); + helpTopicId: n.HelpTopicId, sliceId: n.SliceId, + optionalWritingSystem: n.OptionalWritingSystem, + forceIncludeEnglish: n.ForceIncludeEnglish); } } diff --git a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs index ba3b06c352..5a6877e1cd 100644 --- a/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs +++ b/Src/Common/FwAvalonia/ViewDefinition/XmlLayoutImporter.cs @@ -36,7 +36,8 @@ public sealed class XmlLayoutImporter : IViewDefinitionImporter { "id", "label", "abbr", "field", "ws", "editor", "visibility", "expansion", "localizationKey", "labelId", "automationId", "routing", "menu", "contextMenu", "hotlinks", - "forVariant", "visibleWritingSystems", "reorder", "helpTopicID" + "forVariant", "visibleWritingSystems", "reorder", "helpTopicID", + "optionalWs", "forceIncludeEnglish" }; public static readonly HashSet HandledObjSeqAttributes = @@ -362,6 +363,11 @@ private ViewNode CreateNode( var visibleWss = ParseWsList(Attr(callerEl, "visibleWritingSystems") ?? Attr(contentEl, "visibleWritingSystems")); + // optionalWs and forceIncludeEnglish widen what the field's Writing Systems + // menu may offer, never what the field shows. + var optionalWs = Attr(contentEl, "optionalWs"); + var forceEnglish = ParseOptionalBool(Attr(contentEl, "forceIncludeEnglish")) ?? false; + // Caller children under a slice-content part (/ wrappers on a section // part, e.g. AsLexemeForm's MorphTypeBasic) become child nodes, mirroring how // DataTree.ProcessPartRefNode realizes them as indented child slices. Other caller @@ -396,7 +402,9 @@ private ViewNode CreateNode( chooserLinks: chooserLinks.Count > 0 ? chooserLinks : null, visibleWritingSystems: visibleWss, sliceId: Attr(contentEl, "id"), - helpTopicId: Attr(contentEl, "helpTopicID")); + helpTopicId: Attr(contentEl, "helpTopicID"), + optionalWritingSystem: optionalWs, + forceIncludeEnglish: forceEnglish); } // Dynamic custom slices keep their legacy class/assembly identity so the host can @@ -421,7 +429,9 @@ private ViewNode CreateNode( // The slice's reorder= attribute: its items may be reordered // even when the property is virtual. reorder: ParseOptionalBool(Attr(contentEl, "reorder")) ?? false, - helpTopicId: Attr(contentEl, "helpTopicID")); + helpTopicId: Attr(contentEl, "helpTopicID"), + optionalWritingSystem: optionalWs, + forceIncludeEnglish: forceEnglish); } case "obj": case "seq": diff --git a/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs b/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs new file mode 100644 index 0000000000..813e3f1986 --- /dev/null +++ b/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs @@ -0,0 +1,241 @@ +// 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; +using System.Collections.Generic; +using System.Linq; +using SIL.LCModel; +using SIL.LCModel.Core.WritingSystems; +using SIL.LCModel.DomainServices; + +// Rules both detail views share. Nothing here may depend on WinForms, Avalonia, or a slice +// or control type, so the rules outlive WinForms. DetailRulesBoundaryTests enforces this. +namespace SIL.FieldWorks.Common.DetailRules +{ + /// + /// Which writing-system sets a multi-writing-system field draws on: the set it shows, the + /// set it offers on request only, and whether English joins them regardless. Build one + /// with or , whichever the caller holds. + /// + public sealed class WritingSystemFieldSpec + { + private WritingSystemFieldSpec(int objectHvo, int writingSystems, int optionalWritingSystems, + bool forceIncludeEnglish) + { + ObjectHvo = objectHvo; + WritingSystems = writingSystems; + OptionalWritingSystems = optionalWritingSystems; + ForceIncludeEnglish = forceIncludeEnglish; + } + + /// The field's object; some magic sets resolve against it. + public int ObjectHvo { get; } + + /// The magic id of the field's own writing-system set, or 0 for none. + public int WritingSystems { get; } + + /// + /// The magic id of the set the field offers on request only, or 0 for none. The + /// Pronunciation form is the only shipped part that carries one. + /// + public int OptionalWritingSystems { get; } + + /// + /// Whether English joins the options even when the project has not checked it. + /// + public bool ForceIncludeEnglish { get; } + + /// + /// The spec a layout describes: its ws and optionalWs names and its + /// forceIncludeEnglish flag. A name no magic set answers to resolves to none, so + /// the field offers nothing rather than the analysis writing systems the writing-system + /// query falls back to. + /// + public static WritingSystemFieldSpec FromLayout(int objectHvo, string writingSystems, + string optionalWritingSystems, bool forceIncludeEnglish) + => new WritingSystemFieldSpec(objectHvo, + Resolve(WritingSystemServices.GetMagicWsIdFromName(writingSystems)), + Resolve(WritingSystemServices.GetMagicWsIdFromName(optionalWritingSystems)), + forceIncludeEnglish); + + /// The same spec, for a caller that already holds its magic ids. + public static WritingSystemFieldSpec FromMagicIds(int objectHvo, int writingSystems, + int optionalWritingSystems, bool forceIncludeEnglish) + => new WritingSystemFieldSpec(objectHvo, Resolve(writingSystems), + Resolve(optionalWritingSystems), forceIncludeEnglish); + + // Pronunciation sets resolve through the plural id. Seeding the project's list is the + // caller's business: these rules read the model and never write to it. + private static int Resolve(int magicId) + { + switch (magicId) + { + case WritingSystemServices.kwsPronunciation: + case WritingSystemServices.kwsFirstPronunciation: + case WritingSystemServices.kwsPronunciations: + return WritingSystemServices.kwsPronunciations; + } + return magicId; + } + } + + /// + /// One entry of a field's Writing Systems menu: the writing system, whether the field + /// shows it now, and whether the user may switch it off. + /// + public sealed class WritingSystemMenuOption + { + /// The writing system this entry offers. + /// Whether the field shows it now. + /// False for the last shown entry. + public WritingSystemMenuOption(CoreWritingSystemDefinition writingSystem, bool isChecked, + bool canUncheck) + { + WritingSystem = writingSystem ?? throw new ArgumentNullException(nameof(writingSystem)); + IsChecked = isChecked; + CanUncheck = canUncheck; + } + + /// The writing system this entry offers. + public CoreWritingSystemDefinition WritingSystem { get; } + + /// The id the stored selection holds. + public string Id => WritingSystem.Id; + + /// The menu text. + public string Label => WritingSystem.DisplayLabel; + + /// Whether the field shows this writing system now. + public bool IsChecked { get; } + + /// False for the last shown entry, so the set can never be emptied. + public bool CanUncheck { get; } + } + + /// + /// Which writing systems a multi-writing-system field offers, which of them it shows, and + /// which may be switched off. Both detail views answer their Writing Systems menu from + /// here, so neither can drift from the other. + /// + public static class FieldWritingSystemOptions + { + /// + /// Every writing system the field can be told to show: its own set, including ones the + /// project has not checked, then the set it offers on request. + /// + public static IReadOnlyList Options(LcmCache cache, + WritingSystemFieldSpec spec) + { + Require(cache, spec); + // Deduplicated by ID, not by instance: one writing system must never become two + // menu entries, whichever query produced it. + var options = new List(); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var ws in ListFor(cache, spec.WritingSystems, spec, includeUnchecked: true)) + { + if (seen.Add(ws.Id)) + options.Add(ws); + } + foreach (var ws in ListFor(cache, spec.OptionalWritingSystems, spec, includeUnchecked: true)) + { + if (seen.Add(ws.Id)) + options.Add(ws); + } + return options; + } + + /// + /// What the field shows when nothing usable is stored: its own set, restricted to the + /// writing systems the project has checked. The set offered on request is absent, since + /// it is never shown unasked. + /// + public static IReadOnlyList DefaultShown(LcmCache cache, + WritingSystemFieldSpec spec) + { + Require(cache, spec); + return ListFor(cache, spec.WritingSystems, spec, includeUnchecked: false); + } + + /// + /// The writing systems the field shows: the stored selection, in its own order, + /// restricted to the ids that are still options; when + /// none of them is. + /// + /// The field's persisted selection, or null for + /// none. + public static IReadOnlyList Shown(LcmCache cache, + WritingSystemFieldSpec spec, IReadOnlyList storedSelection) + { + var stored = Select(Options(cache, spec), storedSelection); + return stored.Count > 0 ? stored : DefaultShown(cache, spec); + } + + /// + /// The field's Writing Systems menu: every option in order, each marked with whether + /// the field shows it and whether it may be switched off. + /// + /// The field's persisted selection, or null for + /// none. + public static IReadOnlyList Menu(LcmCache cache, + WritingSystemFieldSpec spec, IReadOnlyList storedSelection) + { + // Options once, then the shown set from it: recomputing would query the writing + // systems a second time. + var options = Options(cache, spec); + var stored = Select(options, storedSelection); + var shown = new HashSet( + (stored.Count > 0 ? stored : DefaultShown(cache, spec)).Select(ws => ws.Id), + StringComparer.OrdinalIgnoreCase); + // The last shown writing system cannot be switched off, so a field is never blank. + var last = shown.Count == 1; + return options + .Select(ws => + { + var isChecked = shown.Contains(ws.Id); + return new WritingSystemMenuOption(ws, isChecked, !(isChecked && last)); + }) + .ToList(); + } + + // The stored ids that are still options, in the STORED order and deduplicated: that is + // the order the field renders them in. + private static IReadOnlyList Select( + IReadOnlyList options, IReadOnlyList stored) + { + if (stored == null || stored.Count == 0) + return Array.Empty(); + var byId = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var ws in options) + { + if (!byId.ContainsKey(ws.Id)) + byId.Add(ws.Id, ws); + } + var result = new List(); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var id in stored) + { + if (!string.IsNullOrEmpty(id) && byId.TryGetValue(id, out var ws) && seen.Add(ws.Id)) + result.Add(ws); + } + return result; + } + + private static IReadOnlyList ListFor(LcmCache cache, int magicId, + WritingSystemFieldSpec spec, bool includeUnchecked) + { + if (magicId == 0) + return Array.Empty(); + return WritingSystemServices.GetWritingSystemList(cache, magicId, spec.ObjectHvo, + spec.ForceIncludeEnglish, includeUnchecked); + } + + private static void Require(LcmCache cache, WritingSystemFieldSpec spec) + { + if (cache == null) + throw new ArgumentNullException(nameof(cache)); + if (spec == null) + throw new ArgumentNullException(nameof(spec)); + } + } +} diff --git a/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs b/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs new file mode 100644 index 0000000000..eca2244414 --- /dev/null +++ b/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs @@ -0,0 +1,203 @@ +// 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; +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using SIL.FieldWorks.Common.DetailRules; +using SIL.LCModel; +using SIL.LCModel.Core.WritingSystems; + +namespace SIL.FieldWorks.FdoUi +{ + /// + /// The writing systems a multi-writing-system field offers, shows, and lets the user + /// switch off. The distinction that matters is offered-versus-shown: a writing system the + /// project has not checked is offered by the menu but not shown until it is chosen. + /// + [TestFixture] + public class FieldWritingSystemOptionsTests : MemoryOnlyBackendProviderRestoredForEachTestTestBase + { + private const string AllAnalysis = "all analysis"; + private const string AllVernacular = "all vernacular"; + + private WritingSystemFieldSpec Analysis(string optionalWs = null, + bool forceIncludeEnglish = false) + => WritingSystemFieldSpec.FromLayout(0, AllAnalysis, optionalWs, forceIncludeEnglish); + + private static IReadOnlyList Ids(IEnumerable systems) + => systems.Select(ws => ws.Id).ToList(); + + // An analysis writing system the project has NOT checked: active, so the menu offers it, + // but absent from the shown set until the user picks it. + private CoreWritingSystemDefinition AddUncheckedAnalysisWs(string id) + { + // The fixture already runs each test inside a unit of work; a nested one throws. + Cache.ServiceLocator.WritingSystemManager.GetOrSet(id, out var ws); + // Active but NOT current: the project knows it, the user has not checked it. + Cache.ServiceLocator.WritingSystems.AnalysisWritingSystems.Add(ws); + return ws; + } + + [Test] + public void Options_OfferAnUncheckedWritingSystem_ThatDefaultShownDoesNot() + { + var offered = AddUncheckedAnalysisWs("de"); + + var options = Ids(FieldWritingSystemOptions.Options(Cache, Analysis())); + var shown = Ids(FieldWritingSystemOptions.DefaultShown(Cache, Analysis())); + + Assert.That(options, Does.Contain(offered.Id), + "the menu offers an active writing system the project has not checked"); + Assert.That(shown, Does.Not.Contain(offered.Id), + "but the field does not show it until the user chooses it"); + Assert.That(shown, Is.SubsetOf(options), "everything shown must be offerable"); + } + + [Test] + public void Options_AddTheOptionalSpecsWritingSystems_AfterItsOwn() + { + var withoutOptional = Ids(FieldWritingSystemOptions.Options(Cache, Analysis())); + + var withOptional = Ids(FieldWritingSystemOptions.Options(Cache, Analysis(AllVernacular))); + + Assert.That(withOptional.Take(withoutOptional.Count), Is.EqualTo(withoutOptional), + "the field's own writing systems come first, in their own order"); + Assert.That(withOptional, Is.SupersetOf(withoutOptional)); + Assert.That(withOptional, Does.Contain(Cache.ServiceLocator.WritingSystems.DefaultVernacularWritingSystem.Id), + "the optional spec's writing systems are offered too"); + } + + [Test] + public void Options_OfTheOptionalSpec_AreNotShownByDefault() + { + var vernacular = Cache.ServiceLocator.WritingSystems.DefaultVernacularWritingSystem.Id; + + var shown = Ids(FieldWritingSystemOptions.DefaultShown(Cache, Analysis(AllVernacular))); + + Assert.That(shown, Does.Not.Contain(vernacular), + "the optional spec widens what can be chosen, never what is shown unasked"); + } + + [Test] + public void Options_OfAPartWithoutAnOptionalSpec_AddNothing() + { + // The no-magic-set guard earns its keep here: the query treats an unrecognized + // set as the analysis one, so without it every part lacking optionalWs would + // offer the analysis writing systems. + var vernacularField = WritingSystemFieldSpec.FromLayout(0, AllVernacular, null, false); + var analysisWs = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem.Id; + + var options = Ids(FieldWritingSystemOptions.Options(Cache, vernacularField)); + + Assert.That(options, Does.Not.Contain(analysisWs), + "a null optional spec must widen the options by nothing at all"); + Assert.That(options, + Is.EqualTo(Ids(FieldWritingSystemOptions.Options(Cache, + WritingSystemFieldSpec.FromLayout(0, AllVernacular, null, false))))); + } + + [Test] + public void Options_OfASpecNamingNoMagicSet_AreEmpty() + { + // Deliberately unlike the composer's render-side resolver, which falls back to the + // analysis set: a field whose spec names no set has no writing systems to offer. + var spec = WritingSystemFieldSpec.FromLayout(0, "not a magic writing system set", + null, false); + + Assert.That(FieldWritingSystemOptions.Options(Cache, spec), Is.Empty); + Assert.That(FieldWritingSystemOptions.DefaultShown(Cache, spec), Is.Empty); + Assert.That(FieldWritingSystemOptions.Menu(Cache, spec, null), Is.Empty); + } + + [Test] + public void Shown_IsTheStoredSelection_InItsOwnOrder() + { + var second = AddUncheckedAnalysisWs("de"); + var first = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem; + + var shown = Ids(FieldWritingSystemOptions.Shown(Cache, Analysis(), + new[] { second.Id, first.Id })); + + Assert.That(shown, Is.EqualTo(new[] { second.Id, first.Id }), + "the stored order is the order the field renders them in"); + } + + [Test] + public void Shown_DropsAStoredIdThatIsNoLongerAnOption() + { + var real = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem; + + var shown = Ids(FieldWritingSystemOptions.Shown(Cache, Analysis(), + new[] { "zzz-no-such-ws", real.Id })); + + Assert.That(shown, Is.EqualTo(new[] { real.Id }), "a stale id must not survive"); + } + + [Test] + public void Shown_FallsBackToTheDefault_WhenNothingStoredSurvives() + { + var expected = Ids(FieldWritingSystemOptions.DefaultShown(Cache, Analysis())); + + Assert.That(Ids(FieldWritingSystemOptions.Shown(Cache, Analysis(), null)), + Is.EqualTo(expected), "no stored selection shows the default set"); + Assert.That(Ids(FieldWritingSystemOptions.Shown(Cache, Analysis(), new string[0])), + Is.EqualTo(expected)); + Assert.That(Ids(FieldWritingSystemOptions.Shown(Cache, Analysis(), new[] { "zzz-no-such-ws" })), + Is.EqualTo(expected), "an entirely stale selection must not blank the field"); + } + + [Test] + public void Menu_ChecksWhatIsShown_AndOffersWhatIsNot() + { + var offeredOnly = AddUncheckedAnalysisWs("de"); + var analysis = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem; + + var menu = FieldWritingSystemOptions.Menu(Cache, Analysis(), new[] { analysis.Id }); + + Assert.That(Ids(menu.Select(m => m.WritingSystem)), + Is.EqualTo(Ids(FieldWritingSystemOptions.Options(Cache, Analysis()))), + "the menu lists every option, in option order"); + Assert.That(menu.Single(m => m.Id == analysis.Id).IsChecked, Is.True); + Assert.That(menu.Single(m => m.Id == offeredOnly.Id).IsChecked, Is.False); + Assert.That(menu.Single(m => m.Id == analysis.Id).Label, + Is.EqualTo(analysis.DisplayLabel), "the menu text is the writing system's label"); + } + + [Test] + public void Menu_CannotUncheckTheLastShownWritingSystem() + { + var second = AddUncheckedAnalysisWs("de"); + var first = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem; + + var one = FieldWritingSystemOptions.Menu(Cache, Analysis(), new[] { first.Id }); + Assert.That(one.Single(m => m.Id == first.Id).CanUncheck, Is.False, + "the only shown writing system cannot be switched off, so the field is never blank"); + Assert.That(one.Single(m => m.Id == second.Id).CanUncheck, Is.True, + "an unshown one is always available to switch on"); + + var two = FieldWritingSystemOptions.Menu(Cache, Analysis(), new[] { first.Id, second.Id }); + Assert.That(two.Where(m => m.IsChecked).Select(m => m.CanUncheck), Is.All.True, + "with two shown, either may be switched off"); + } + + [Test] + public void EveryEntryPoint_RejectsAMissingCacheOrSpec() + { + Assert.That(() => FieldWritingSystemOptions.Options(null, Analysis()), + Throws.ArgumentNullException); + Assert.That(() => FieldWritingSystemOptions.Options(Cache, null), + Throws.ArgumentNullException); + Assert.That(() => FieldWritingSystemOptions.DefaultShown(null, Analysis()), + Throws.ArgumentNullException); + Assert.That(() => FieldWritingSystemOptions.Shown(Cache, null, null), + Throws.ArgumentNullException); + Assert.That(() => FieldWritingSystemOptions.Menu(null, Analysis(), null), + Throws.ArgumentNullException); + Assert.That(() => new WritingSystemMenuOption(null, false, false), + Throws.ArgumentNullException); + } + } +} diff --git a/Src/xWorks/Avalonia/Composer/DetailComposer.cs b/Src/xWorks/Avalonia/Composer/DetailComposer.cs index 0ae621e60a..d28f2c6c26 100644 --- a/Src/xWorks/Avalonia/Composer/DetailComposer.cs +++ b/Src/xWorks/Avalonia/Composer/DetailComposer.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Threading; using System.Xml.Linq; +using SIL.FieldWorks.Common.DetailRules; using SIL.FieldWorks.Common.FwAvalonia; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; @@ -1188,8 +1189,12 @@ private void WalkTextField(ViewNode node, ICmObject obj, int depth) // A multistring editor is the legacy MultiStringSlice; its in-string context menu adds the // shared mnuDataTree-MultiStringSlice group (Writing Systems submenu), a single-ws string // editor does not. Carry that so the menu composition mirrors the legacy slice test. - textField.IsMultiStringRow = string.Equals(node.RawEditor, - EditorKindMap.MultiStringEditor, StringComparison.OrdinalIgnoreCase); + textField.IsMultiStringRow = IsMultiStringEditor(node); + // The Writing Systems menu offers more than the row renders, so carry the three + // layout facts that decide its option list (FieldWritingSystemOptions). + textField.OptionalWritingSystem = node.OptionalWritingSystem; + textField.ForceIncludeEnglish = node.ForceIncludeEnglish; + textField.VisibleWritingSystems = node.VisibleWritingSystems; if (editable) { // An editable text row over a run-bearing TsString property (String/MultiString) @@ -1211,20 +1216,49 @@ private void WalkTextField(ViewNode node, ICmObject obj, int depth) RegisterTextRowEditHandler(stableId, hvo, flid, type, systems); } + // The multistring editor is the one whose rows carry a Writing Systems menu. + private static bool IsMultiStringEditor(ViewNode node) + => string.Equals(node.RawEditor, EditorKindMap.MultiStringEditor, + StringComparison.OrdinalIgnoreCase); + + // The layout facts that decide a multistring row's writing systems, as the shared + // rule takes them. + private WritingSystemFieldSpec WritingSystemSpecOf(ViewNode node, int hvo) + { + var spec = WritingSystemFieldSpec.FromLayout(hvo, node.WritingSystem, + node.OptionalWritingSystem, node.ForceIncludeEnglish); + // Seeding the project's pronunciation list belongs to the caller: the shared + // rule only reads the model. This keeps what ResolveWritingSystems does. + if (spec.WritingSystems == WritingSystemServices.kwsPronunciations) + WritingSystemServices.InitializePronunciationWritingSystems(_cache); + return spec; + } + // A text row's writing systems: the layout set, restricted by visibleWritingSystems // unless the row's part is under the Show-all reveal, then collapsed to one ws for // String/Unicode props. private IReadOnlyList ResolveTextRowWritingSystems(int hvo, int flid, CellarPropertyType type, ViewNode node) { - IReadOnlyList systems = ResolveWritingSystems(_cache, node.WritingSystem); - // A per-field writing-system visibility override (legacy visibleWritingSystems) restricts - // the resolved set to the authored subset (in the override's order), intersected with the - // field's valid writing systems. An empty intersection keeps the full set rather than hiding - // the field entirely (defensive -- a stale override must never blank a real - // field). - if (_showAllWsFields == null || !_showAllWsFields.Contains(node.StableId)) - systems = ApplyVisibleWritingSystems(systems, node.VisibleWritingSystems); + var revealed = _showAllWsFields != null && _showAllWsFields.Contains(node.StableId); + IReadOnlyList systems; + if (IsMultiStringEditor(node)) + { + // The Writing Systems menu offers writing systems the project has not + // checked. Render from the SAME rule, or a chosen one cannot appear. + var spec = WritingSystemSpecOf(node, hvo); + systems = revealed + ? FieldWritingSystemOptions.Options(_cache, spec) + : FieldWritingSystemOptions.Shown(_cache, spec, node.VisibleWritingSystems); + } + else + { + systems = ResolveWritingSystems(_cache, node.WritingSystem); + // A stored selection restricts the resolved set, in its own order. An empty + // intersection keeps the full set rather than blanking a real field. + if (!revealed) + systems = ApplyVisibleWritingSystems(systems, node.VisibleWritingSystems); + } if ((type == CellarPropertyType.String || type == CellarPropertyType.Unicode) && systems.Count > 0) { diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs index 5d471d48e2..674bdbe642 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/DetailEditContextEditingTests.cs @@ -2182,6 +2182,26 @@ public void Compose_CustomMultiStringWithSingularWsSelector_ComposesAPlainString "a plural WsSelector keeps the multistring row and its Writing Systems menu"); } + [Test] + public void Compose_MultiStringRow_CarriesTheLayoutFactsItsWritingSystemMenuNeeds() + { + var fields = Compose(showHidden: true).ToList(); + + // The Pronunciation form is the ONE shipped part with optionalWs: its menu offers the + // vernacular writing systems as well, though the row shows only pronunciation ones. + var pronunciation = fields.First(f => f.Field == "Form" && f.ObjectHvo == m_pronunciation.Hvo); + Assert.That(pronunciation.IsMultiStringRow, Is.True, "precondition: a multistring row"); + Assert.That(pronunciation.WritingSystem, Is.EqualTo("all pronunciation")); + Assert.That(pronunciation.OptionalWritingSystem, Is.EqualTo("all vernacular"), + "the widening spec must reach the row, or its menu offers too few writing systems"); + + var citation = fields.First(f => f.Field == "CitationForm" && f.ObjectHvo == m_entry.Hvo); + Assert.That(citation.IsMultiStringRow, Is.True, "precondition: a multistring row"); + Assert.That(citation.OptionalWritingSystem, Is.Null, + "an ordinary multistring row offers only its own writing systems"); + Assert.That(citation.ForceIncludeEnglish, Is.False); + } + [Test] public void Compose_CustomFields_SitAtTheLegacyPlaceholderPosition() { diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs index 037fd6ab18..d57ec168ef 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs @@ -84,6 +84,69 @@ public void PerFieldWs_LimitsDisplayedWritingSystems_OneVsMany() Is.SameAs(all), "a stale override that matches nothing keeps the full set, never blanks the field"); } + [Test] + public void PerFieldWs_ShowsAWritingSystemTheProjectHasNotChecked() + { + // The menu offers active-but-unchecked writing systems, so a row must render one. + // Before the shared rule, choosing German alone matched nothing and the row fell + // back to showing them ALL. + CoreWritingSystemDefinition offered = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + Cache.ServiceLocator.WritingSystemManager.GetOrSet("de", out offered); + Cache.ServiceLocator.WritingSystems.VernacularWritingSystems.Add(offered); + }); + + try + { + var form = ComposedFormRow(null, null); + Assume.That(form.Values.Select(v => v.WsTag), Does.Not.Contain(offered.Id), + "precondition: an unchecked writing system is not shown unasked"); + + var restricted = ComposedFormRow(RestrictFormRowTo(form, offered.Id), null); + + Assert.That(restricted.Values.Select(v => v.WsTag), Is.EqualTo(new[] { offered.Id }), + "the row shows exactly the writing system chosen, not every writing system"); + + // Show all right now reveals every OPTION, unchecked ones included, as the + // WinForms slice does. + var templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(form.StableId); + var revealed = ComposedFormRow(null, new HashSet { templateId }); + Assert.That(revealed.Values.Select(v => v.WsTag), Does.Contain(offered.Id), + "the reveal shows what the menu offers, not just what the project has checked"); + } + finally + { + // The fixture is shared across tests: an extra writing system would change what + // every later row composes. + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + Cache.ServiceLocator.WritingSystems.VernacularWritingSystems.Remove(offered)); + } + } + + [Test] + public void PerFieldWs_WithNothingStored_ShowsWhatItAlwaysShowed() + { + var expected = DetailComposer.ResolveWritingSystems(Cache, "all vernacular").Select(w => w.Id); + + Assert.That(ComposedFormRow(null, null).Values.Select(v => v.WsTag), Is.EqualTo(expected), + "a row with no stored selection composes the set it composed before the shared rule"); + } + + // An override restricting the Form row to one writing system. + private static ViewDefinitionOverrideResolver RestrictFormRowTo(DetailField form, string wsTag) + { + var restriction = new ViewDefinitionOverride(form.ClassName, form.LayoutName, "detail", + new[] + { + new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibleWritingSystems, + ViewDefinitionOverrideEditor.StripRuntimeSuffix(form.StableId), + writingSystems: new[] { wsTag }) + }, null); + return (cls, layout) => + cls == form.ClassName && layout == form.LayoutName ? restriction : null; + } + // ---- Transient "Show all right now" reveal ---- // The reveal bypasses the ws restriction for EXACTLY the revealed part: Compose From 114ed0a7f202577734eb9d86f639d3aff500f604 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:49:48 -0400 Subject: [PATCH 2/4] LT-22691: Answer the Writing Systems menu natively on multi-string rows The multi-string label menu was the last shared menu group still routed through the hidden WinForms tree. Its Writing Systems submenu was populated by asking the mediator, which only the hidden adapter's slice could answer, and each toggle dispatched there and copied the result back into the view override afterwards. A new authority owns that menu on multi-string rows. It delegates the Field Visibility, Move Field and Help leaves to the per-object authority and answers the writing-system list from the shared rule the previous commit introduced. Toggles, Show all right now and Configure now write the view override directly, and a Pronunciation row also keeps the project's current pronunciation writing systems in step, through a rule the WinForms slice shares. The bridge learned to take a list-populated submenu's items from an authority instead of populating it through the mediator. Counted from the shipped configuration, 111 of the 118 multi-string parts bind either no menu or the empty Help menu, so their label menus now build without the hidden adapter at all. The other seven keep it only for their own menu id. Rules worth knowing. The authority claims the id only on a genuine multi-string row; any other row that binds it keeps the mediator path. The interceptor's writing-system items are gone, since no leaf reaches them any more. Shared rules read the model; the one write, the pronunciation list, joins a unit of work that is already open rather than failing inside it. Configure treats the same set in any order as no change. A row renders its selection in option order, whatever order it was stored in, so re-checking a writing system never reorders the rows. Co-Authored-By: Claude Fable 5.1 --- .../ConfigureWritingSystemsDlg.cs | 5 + .../DetailControls/MultiStringSlice.cs | 15 +- .../DetailRules/FieldWritingSystemOptions.cs | 20 +- .../PronunciationWritingSystems.cs | 50 +++++ .../FieldWritingSystemOptionsTests.cs | 7 +- .../Hosting/CompositeMenuAuthority.cs | 15 +- .../Avalonia/Hosting/IDetailMenuAuthority.cs | 12 ++ .../Hosting/MultiStringMenuAuthority.cs | 102 ++++++++++ .../Avalonia/Hosting/ObjectMenuAuthority.cs | 7 + .../Hosting/RecordEditView.Avalonia.cs | 192 ++++++++---------- .../Hosting/ReorderVectorMenuAuthority.cs | 7 + .../Avalonia/Hosting/XCoreMenuBridge.cs | 28 ++- .../DetailObjectCommandExecutionTests.cs | 150 +++++++++++++- 13 files changed, 445 insertions(+), 165 deletions(-) create mode 100644 Src/FdoUi/DetailRules/PronunciationWritingSystems.cs create mode 100644 Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs diff --git a/Src/Common/Controls/DetailControls/ConfigureWritingSystemsDlg.cs b/Src/Common/Controls/DetailControls/ConfigureWritingSystemsDlg.cs index fa80ac56c0..a1ba53a70c 100644 --- a/Src/Common/Controls/DetailControls/ConfigureWritingSystemsDlg.cs +++ b/Src/Common/Controls/DetailControls/ConfigureWritingSystemsDlg.cs @@ -10,6 +10,7 @@ using SIL.LCModel.Core.WritingSystems; using SIL.FieldWorks.Common.FwUtils; using XCore; +using SIL.FieldWorks.Common.Framework.DetailControls.Resources; namespace SIL.FieldWorks.Common.Framework.DetailControls { @@ -66,6 +67,10 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + /// The dialog title for the field named . + public static string TitleFor(string fieldLabel) + => string.Format(DetailControlsStrings.ksSliceConfigureWssDlgTitle, fieldLabel); + /// /// Gets the selected writing system. /// diff --git a/Src/Common/Controls/DetailControls/MultiStringSlice.cs b/Src/Common/Controls/DetailControls/MultiStringSlice.cs index 3efbb56b05..027e172974 100644 --- a/Src/Common/Controls/DetailControls/MultiStringSlice.cs +++ b/Src/Common/Controls/DetailControls/MultiStringSlice.cs @@ -227,7 +227,7 @@ public bool OnDataTreeWritingSystemsConfigureDlg(object args) using (var dlg = new ConfigureWritingSystemsDlg(WritingSystemOptionsForDisplay, WritingSystemsSelectedForDisplay, m_propertyTable.GetValue("HelpTopicProvider"))) { - dlg.Text = String.Format(DetailControlsStrings.ksSliceConfigureWssDlgTitle, Label); + dlg.Text = ConfigureWritingSystemsDlg.TitleFor(Label); if (dlg.ShowDialog() == DialogResult.OK) PersistAndRedisplayWssToDisplayForPart(dlg.SelectedWritingSystems); } @@ -330,18 +330,7 @@ private void PersistAndRedisplayWssToDisplayForPart(string singlePropertySequenc /// Get the language project's list of pronunciation writing systems into sync with the supplied list. /// private void UpdatePronunciationWritingSystems(IEnumerable newValues) - { - if (newValues.Count() != m_cache.ServiceLocator.WritingSystems.CurrentPronunciationWritingSystems.Count - || !m_cache.ServiceLocator.WritingSystems.CurrentPronunciationWritingSystems.SequenceEqual(newValues)) - { - NonUndoableUnitOfWorkHelper.Do(m_cache.ServiceLocator.GetInstance(), () => - { - m_cache.ServiceLocator.WritingSystems.CurrentPronunciationWritingSystems.Clear(); - foreach (CoreWritingSystemDefinition ws in newValues) - m_cache.ServiceLocator.WritingSystems.CurrentPronunciationWritingSystems.Add(ws); - }); - } - } + => PronunciationWritingSystems.Sync(m_cache, newValues); /// /// go through all the data tree slices, finding the slices that refer to the same part as this slice diff --git a/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs b/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs index 813e3f1986..2172a9ac0f 100644 --- a/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs +++ b/Src/FdoUi/DetailRules/FieldWritingSystemOptions.cs @@ -158,8 +158,8 @@ public static IReadOnlyList DefaultShown(LcmCache c } /// - /// The writing systems the field shows: the stored selection, in its own order, - /// restricted to the ids that are still options; when + /// The writing systems the field shows: the stored selection, restricted to the ids + /// that are still options and in the options' order; when /// none of them is. /// /// The field's persisted selection, or null for @@ -198,24 +198,20 @@ public static IReadOnlyList Menu(LcmCache cache, .ToList(); } - // The stored ids that are still options, in the STORED order and deduplicated: that is - // the order the field renders them in. + // The options the stored selection names, in OPTION order: a re-checked writing system + // lands at the end of the stored list, and the rows must not reorder on a rebuild. private static IReadOnlyList Select( IReadOnlyList options, IReadOnlyList stored) { if (stored == null || stored.Count == 0) return Array.Empty(); - var byId = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var ws in options) - { - if (!byId.ContainsKey(ws.Id)) - byId.Add(ws.Id, ws); - } + var wanted = new HashSet(stored.Where(id => !string.IsNullOrEmpty(id)), + StringComparer.OrdinalIgnoreCase); var result = new List(); var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var id in stored) + foreach (var ws in options) { - if (!string.IsNullOrEmpty(id) && byId.TryGetValue(id, out var ws) && seen.Add(ws.Id)) + if (wanted.Contains(ws.Id) && seen.Add(ws.Id)) result.Add(ws); } return result; diff --git a/Src/FdoUi/DetailRules/PronunciationWritingSystems.cs b/Src/FdoUi/DetailRules/PronunciationWritingSystems.cs new file mode 100644 index 0000000000..24b8465005 --- /dev/null +++ b/Src/FdoUi/DetailRules/PronunciationWritingSystems.cs @@ -0,0 +1,50 @@ +// 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; +using System.Collections.Generic; +using System.Linq; +using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Core.WritingSystems; +using SIL.LCModel.Infrastructure; + +// Rules both detail views share. Nothing here may depend on WinForms, Avalonia, or a slice +// or control type, so the rules outlive WinForms. DetailRulesBoundaryTests enforces this. +namespace SIL.FieldWorks.Common.DetailRules +{ + /// + /// The project's current pronunciation writing systems, which a Pronunciation field's + /// writing-system choice also sets (LT-9620). Changing which writing systems that field + /// shows is a data change, not only a view preference, so both detail views must make it. + /// + public static class PronunciationWritingSystems + { + /// + /// Brings the project's current pronunciation writing systems into line with + /// . Does nothing when they already match. The write is not + /// undoable, and joins a unit of work already open rather than failing inside one. + /// + public static void Sync(LcmCache cache, IEnumerable shown) + { + if (cache == null) + throw new ArgumentNullException(nameof(cache)); + if (shown == null) + throw new ArgumentNullException(nameof(shown)); + + var wanted = shown.ToList(); + var current = cache.ServiceLocator.WritingSystems.CurrentPronunciationWritingSystems; + if (wanted.Count == current.Count && current.SequenceEqual(wanted)) + return; + + NonUndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW( + cache.ServiceLocator.GetInstance(), () => + { + current.Clear(); + foreach (var ws in wanted) + current.Add(ws); + }); + } + } +} diff --git a/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs b/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs index eca2244414..941a851c70 100644 --- a/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs +++ b/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs @@ -113,16 +113,17 @@ public void Options_OfASpecNamingNoMagicSet_AreEmpty() } [Test] - public void Shown_IsTheStoredSelection_InItsOwnOrder() + public void Shown_IsTheStoredSelection_InOptionOrder() { var second = AddUncheckedAnalysisWs("de"); var first = Cache.ServiceLocator.WritingSystems.DefaultAnalysisWritingSystem; + // Stored with the re-checked one appended, as a menu toggle leaves it. var shown = Ids(FieldWritingSystemOptions.Shown(Cache, Analysis(), new[] { second.Id, first.Id })); - Assert.That(shown, Is.EqualTo(new[] { second.Id, first.Id }), - "the stored order is the order the field renders them in"); + Assert.That(shown, Is.EqualTo(new[] { first.Id, second.Id }), + "the field renders its selection in option order, whatever order it was stored in"); } [Test] diff --git a/Src/xWorks/Avalonia/Hosting/CompositeMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/CompositeMenuAuthority.cs index bd5ff2ac69..d8246fab2d 100644 --- a/Src/xWorks/Avalonia/Hosting/CompositeMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/CompositeMenuAuthority.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; using SIL.FieldWorks.Common.FwAvalonia.Detail; using XCore; @@ -25,12 +26,14 @@ public CompositeMenuAuthority(params IDetailMenuAuthority[] members) public bool Owns(string menuId) => Owner(menuId) != null; public DetailMenuItem Build(string menuId, ChoiceBase leaf) - { - var owner = Owner(menuId); - if (owner == null) - throw new InvalidOperationException(string.Format("No authority owns menu '{0}'.", menuId)); - return owner.Build(menuId, leaf); - } + => Owning(menuId).Build(menuId, leaf); + + public IReadOnlyList BuildList(string menuId, string listId) + => Owning(menuId).BuildList(menuId, listId); + + private IDetailMenuAuthority Owning(string menuId) + => Owner(menuId) ?? throw new InvalidOperationException( + string.Format("No authority owns menu '{0}'.", menuId)); private IDetailMenuAuthority Owner(string menuId) { diff --git a/Src/xWorks/Avalonia/Hosting/IDetailMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/IDetailMenuAuthority.cs index 219a3b44c1..253c48b6b1 100644 --- a/Src/xWorks/Avalonia/Hosting/IDetailMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/IDetailMenuAuthority.cs @@ -2,6 +2,7 @@ // This software is licensed under the LGPL, version 2.1 or later // (http://www.gnu.org/licenses/lgpl-2.1.html) +using System.Collections.Generic; using SIL.FieldWorks.Common.FwAvalonia.Detail; using XCore; @@ -33,5 +34,16 @@ public interface IDetailMenuAuthority /// authority answers: an owned id must be answered in full, or a leaf would leak as /// visible-but-disabled. DetailMenuItem Build(string menuId, ChoiceBase leaf); + + /// + /// The items of a list-populated submenu under an owned menu id, in display order, each + /// already carrying its label, state and execute action. Empty hides the submenu. + /// + /// The owned menu id the submenu belongs to. + /// The submenu's list attribute: the id xCore would + /// otherwise have asked the mediator to populate. + /// The list is not one this authority + /// answers: an owned id must be answered in full. + IReadOnlyList BuildList(string menuId, string listId); } } diff --git a/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs new file mode 100644 index 0000000000..4efebac7a8 --- /dev/null +++ b/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs @@ -0,0 +1,102 @@ +// 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; +using System.Collections.Generic; +using System.Linq; +using SIL.FieldWorks.Common.DetailRules; +using SIL.FieldWorks.Common.FwAvalonia.Detail; +using XCore; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// The native authority for a multi-writing-system row's label menu: the Writing Systems + /// submenu answered from the row and the shared writing-system rule, and the Field + /// Visibility, Move Field and Help leaves delegated to the per-object authority. Owns its + /// id only on a genuine multi-string row; any other row that binds it keeps the mediator. + /// + internal sealed class MultiStringMenuAuthority : IDetailMenuAuthority + { + internal const string MenuId = RecordEditView.MultiStringSliceMenuId; + internal const string WritingSystemListId = "WritingSystemOptionsForSlice"; + internal const string ShowAllCommandId = "CmdDataTree-WritingSystemMenu-ShowAllRightNow"; + internal const string ConfigureCommandId = "CmdDataTree-WritingSystemMenu-Configure"; + + private readonly DetailField _field; + private readonly IDetailMenuAuthority _sharedLeaves; + private readonly Func> _menu; + private readonly Action> _show; + private readonly Func _showAll; + private readonly Func _configure; + + /// Creates the authority for one row's menu. + /// The row. + /// Answers the Field Visibility, Move Field and Help leaves + /// this menu re-declares. + /// The row's writing-system options, checked and enabled. + /// Makes the row show exactly the given writing systems. + /// Builds the Show all right now item from (label, row). + /// Builds the Configure item from (label, row). + public MultiStringMenuAuthority(DetailField field, IDetailMenuAuthority sharedLeaves, + Func> menu, + Action> show, + Func showAll, + Func configure) + { + _field = field ?? throw new ArgumentNullException(nameof(field)); + _sharedLeaves = sharedLeaves ?? throw new ArgumentNullException(nameof(sharedLeaves)); + _menu = menu ?? throw new ArgumentNullException(nameof(menu)); + _show = show ?? throw new ArgumentNullException(nameof(show)); + _showAll = showAll ?? throw new ArgumentNullException(nameof(showAll)); + _configure = configure ?? throw new ArgumentNullException(nameof(configure)); + } + + public bool Owns(string menuId) + => _field.IsMultiStringRow && string.Equals(menuId, MenuId, StringComparison.Ordinal); + + public DetailMenuItem Build(string menuId, ChoiceBase leaf) + { + if (leaf == null) + throw new ArgumentNullException(nameof(leaf)); + var label = XCoreMenuBridge.StripAccelerator(leaf.Label); + switch (leaf.HelpId) + { + case ShowAllCommandId: + return _showAll(label, _field); + case ConfigureCommandId: + return _configure(label, _field); + default: + // Everything else is the per-object menu's leaf set, re-declared here. + return _sharedLeaves.Build(menuId, leaf); + } + } + + public IReadOnlyList BuildList(string menuId, string listId) + { + if (!string.Equals(listId, WritingSystemListId, StringComparison.Ordinal)) + { + throw new InvalidOperationException(string.Format( + "Menu '{0}' has a list submenu '{1}' this authority does not answer.", menuId, listId)); + } + var options = _menu(_field); + return options.Select(option => ToggleItem(option, options)).ToList(); + } + + // A toggle flips its own writing system in or out of the shown set and leaves the rest + // alone. The last shown one is offered disabled, so the set can never be emptied. + private DetailMenuItem ToggleItem(WritingSystemMenuOption option, + IReadOnlyList all) + { + var toggled = all + .Where(o => string.Equals(o.Id, option.Id, StringComparison.OrdinalIgnoreCase) + ? !o.IsChecked + : o.IsChecked) + .Select(o => o.Id) + .ToList(); + return new DetailMenuItem(option.Label, option.CanUncheck, option.IsChecked, children: null, + execute: option.CanUncheck ? (Action)(() => _show(_field, toggled)) : null); + } + } +} diff --git a/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs index 52806d1a9f..420366be2e 100644 --- a/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; using XCore; @@ -89,6 +90,12 @@ public DetailMenuItem Build(string menuId, ChoiceBase leaf) } } + // Neither owned id carries a list-populated submenu. + public IReadOnlyList BuildList(string menuId, string listId) + => throw new InvalidOperationException(string.Format( + "Menu '{0}' has a list submenu '{1}' this authority does not answer.", + menuId, listId)); + // Offered on every row; without a located target it is disabled rather than guessed. private DetailMenuItem VisibilityItem(string label, ViewVisibility visibility) => _target.Value == null ? Disabled(label) : _fieldVisibility(label, _target.Value, visibility); diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index ab87bd433b..3e81a4fa74 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -509,6 +509,12 @@ internal void OnDetailWritingSystemFocused(string wsTag) /// internal const string MultiStringSliceMenuId = "mnuDataTree-MultiStringSlice"; + /// + /// The Pronunciation form's menu: a row bound to it also sets the project's current + /// pronunciation writing systems when its shown set changes (LT-9620). + /// + internal const string PronunciationMenuId = "mnuDataTree-Pronunciation"; + /// /// Composes the ordered menu-id list for a row's SLICE menu (label right-click and the /// field-options button): the row's own menu= binding, then exactly ONE shared @@ -687,8 +693,8 @@ private ViewDefinitionOverride ResolveViewOverride(string className, string layo + "'; using the shipped definition.", error)); /// - /// Registers the per-field Field Visibility, Move Field, and writing-system commands that - /// retarget to the project override layer for the Avalonia detail view. Registers nothing + /// Registers the per-field Field Visibility and Move Field commands that retarget to + /// the project override layer for the Avalonia detail view. Registers nothing /// (every command keeps its normal mediator dispatch) when the clicked row carries no /// (class, layout) context, e.g. the first-slice fallback rows, so ordinary dispatch /// stays in force when the override layer cannot be addressed. @@ -701,15 +707,6 @@ internal void AddOverrideCommands(OverrideCommandRegistry registry, DetailField return; } - // Writing-system items dispatch normally; the resulting selection is then copied - // into the override. They need no located template node, so they stay registered - // even when locating fails. - registry.Add(IsWritingSystemVisibilityChoice, (c, d) => WritingSystemItem(c, d, field)); - // Show all right now never dispatches or persists: it only marks the row for the - // host's transient reveal. - registry.Add("CmdDataTree-WritingSystemMenu-ShowAllRightNow", - (c, d) => ShowAllWritingSystemsItem(LabelOf(d), field)); - // Unknown/stale target: leave the field commands on mediator dispatch rather than // guess. if (!TryLocateOverrideTarget(field, out var templateId, out var location)) @@ -735,8 +732,25 @@ private static string LabelOf(UIItemDisplayProperties display) /// the hidden command adapter. /// internal IDetailMenuAuthority CreateMenuAuthority(DetailMenuRequest request) - => new CompositeMenuAuthority(CreateReorderVectorAuthority(request), - CreateObjectMenuAuthority(request.Field)); + { + // One per-object authority serves both, so the row's override target is located + // once per menu, not once per authority that asks. + var objectMenu = CreateObjectMenuAuthority(request.Field); + return new CompositeMenuAuthority(CreateReorderVectorAuthority(request), + CreateMultiStringMenuAuthority(request.Field, objectMenu), objectMenu); + } + + /// The native authority for a multi-writing-system row's label menu. + internal IDetailMenuAuthority CreateMultiStringMenuAuthority(DetailField field) + => CreateMultiStringMenuAuthority(field, CreateObjectMenuAuthority(field)); + + private IDetailMenuAuthority CreateMultiStringMenuAuthority(DetailField field, + IDetailMenuAuthority sharedLeaves) + => new MultiStringMenuAuthority(field, sharedLeaves, + menu: WritingSystemMenuOf, + show: ShowWritingSystems, + showAll: ShowAllWritingSystemsItem, + configure: ConfigureWritingSystemsItem); /// The native authority for the row's per-object and Help menus. internal IDetailMenuAuthority CreateObjectMenuAuthority(DetailField field) @@ -882,122 +896,82 @@ private DetailMenuItem ShowAllWritingSystemsItem(string label, DetailField field RefreshAvaloniaDetail(); }); - /// - /// Whether this menu item makes a persistent change to which writing systems a - /// multi-writing-system field shows: a per-writing-system toggle (recognized by the - /// property its group drives -- the toggles carry no command id) or the Configure - /// dialog. Show all right now is not one of these: it is the transient reveal - /// (), not a configuration change to persist. - /// - private static bool IsWritingSystemVisibilityChoice(ChoiceBase choice) - { - if (choice is ListPropertyChoice list) - { - return string.Equals(list.ParentProperty, - PropertyConstants.CurrentContextMenuSelectedWsIds, StringComparison.Ordinal); - } + // The row's Writing Systems menu, from the shared rule and the row's own layout facts. + private IReadOnlyList WritingSystemMenuOf(DetailField field) + => FieldWritingSystemOptions.Menu(Cache, WritingSystemSpecOf(field), field.VisibleWritingSystems); - return string.Equals(choice.HelpId, "CmdDataTree-WritingSystemMenu-Configure", - StringComparison.Ordinal); - } + private static WritingSystemFieldSpec WritingSystemSpecOf(DetailField field) + => WritingSystemFieldSpec.FromLayout(field.ObjectHvo, field.WritingSystem, + field.OptionalWritingSystem, field.ForceIncludeEnglish); /// - /// A writing-system item that dispatches normally and then copies the resulting - /// selection into the override: the hidden adapter slice owns the picker and the - /// Configure dialog, while the Avalonia detail view composes from its own override - /// store. + /// Makes the row show exactly the given writing systems: stores the selection in the + /// row's view override, keeps the project's pronunciation writing systems in step on a + /// Pronunciation row, and recomposes. Nothing here reaches the mediator. /// - private DetailMenuItem WritingSystemItem(ChoiceBase choice, UIItemDisplayProperties display, - DetailField field) - { - var isListToggle = choice is ListPropertyChoice; - // The bridge strips execute from disabled items, so the last checked toggle - // (disabled) can never be invoked to EMPTY the set. - return new DetailMenuItem(XCoreMenuBridge.StripAccelerator(display.Text), display.Enabled, - display.Checked, children: null, execute: () => - { - // Snapshot first, so a dialog that changes nothing (e.g. Cancel) copies - // nothing. - var before = isListToggle ? null : CurrentSliceSelectedWritingSystems(); - choice.OnClick(null, EventArgs.Empty); - CopyWritingSystemSelectionToOverride(field, isListToggle, before); - }); - } - - // Copies the click's selection into the row's override and recomposes. A toggle - // updates its property BEFORE the slice: read the property, in option order; - // Configure reads the slice. - private void CopyWritingSystemSelectionToOverride(DetailField field, bool fromListToggle, - IReadOnlyList sliceSetBeforeClick) + private void ShowWritingSystems(DetailField field, IReadOnlyList writingSystems) { try { - var slice = m_dataEntryForm?.CurrentSlice as MultiStringSlice; - if (slice == null) - { - // The command dispatched, but the result is unreadable: say so, or the - // symptom is "the menu did nothing" with no trail. - Logger.WriteEvent("Writing-system selection was not copied: the adapter " - + "slice is unreadable; the view override was not updated."); + // The menu never offers an empty set, so empty means nothing to store -- and an + // empty op would CLEAR the restriction rather than narrow it. + if (writingSystems == null || writingSystems.Count == 0) return; - } - - // A stale adapter target would store another row's set under this row's id. - if (slice.Object == null || slice.Object.Hvo != field.ObjectHvo) - { - Logger.WriteEvent("Writing-system selection was not copied: the adapter " - + "slice is not the clicked row's; the view override was not updated."); - return; - } - - List selected; - if (fromListToggle) - { - var ids = m_propertyTable.GetStringProperty( - PropertyConstants.CurrentContextMenuSelectedWsIds, null); - // Canonicalize: option order, junk tokens dropped -- the stored order is the - // render order. - selected = string.IsNullOrEmpty(ids) - ? null - : StringSliceUtils.GetVisibleWritingSystems(ids, - slice.WritingSystemOptionsForDisplay).Select(ws => ws.Id).ToList(); - } - else - { - selected = slice.WritingSystemsSelectedForDisplay?.Select(ws => ws.Id).ToList(); - if (selected != null && sliceSetBeforeClick != null - && selected.SequenceEqual(sliceSetBeforeClick, StringComparer.Ordinal)) - { - return; // the dialog changed nothing (e.g. Cancel): no override write. - } - } - - // The menu disables the last checked toggle, so an empty set only means "nothing - // to copy" -- and an empty op would CLEAR the restriction, so bail instead. - if (selected == null || selected.Count == 0) - return; - var templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId); var op = new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibleWritingSystems, - templateId, writingSystems: selected); + templateId, writingSystems: writingSystems.ToList()); if (!TryMutateOverride(field, op)) return; - - // A successful configuration write replaces any transient reveal on the part: - // a newly persisted display set supersedes the reveal on every row sharing it. + if (string.Equals(field.MenuId, PronunciationMenuId, StringComparison.Ordinal)) + { + // Exactly the chosen writing systems, never a fallback set: a stale id + // must not rewrite the project's list to the defaults. + var chosen = new HashSet(writingSystems, StringComparer.OrdinalIgnoreCase); + var resolved = FieldWritingSystemOptions.Options(Cache, WritingSystemSpecOf(field)) + .Where(ws => chosen.Contains(ws.Id)).ToList(); + if (resolved.Count == chosen.Count) + PronunciationWritingSystems.Sync(Cache, resolved); + } + // A persisted set supersedes any transient reveal on the part. m_showAllWsFields.Remove(templateId); RefreshAvaloniaDetail(); } catch (Exception e) { - Logger.WriteError("Copying the writing-system selection into the view override failed.", e); + Logger.WriteError("Storing the row's writing-system selection failed.", e); } } - // The adapter slice's current selection, or null when it cannot be read. - private IReadOnlyList CurrentSliceSelectedWritingSystems() - => (m_dataEntryForm?.CurrentSlice as MultiStringSlice) - ?.WritingSystemsSelectedForDisplay?.Select(ws => ws.Id).ToList(); + // Opens the per-field Configure dialog on the row's own options and shown set, and + // applies the result. + private DetailMenuItem ConfigureWritingSystemsItem(string label, DetailField field) + => new DetailMenuItem(label, isEnabled: true, isChecked: false, children: null, execute: () => + { + try + { + var spec = WritingSystemSpecOf(field); + var shown = FieldWritingSystemOptions.Shown(Cache, spec, field.VisibleWritingSystems); + using (var dlg = new ConfigureWritingSystemsDlg( + FieldWritingSystemOptions.Options(Cache, spec), shown, + m_propertyTable.GetValue("HelpTopicProvider"))) + { + dlg.Text = ConfigureWritingSystemsDlg.TitleFor(field.Label); + if (dlg.ShowDialog() != System.Windows.Forms.DialogResult.OK) + return; + var chosen = dlg.SelectedWritingSystems.Select(ws => ws.Id).ToList(); + // The dialog answers in option order, the stored set may not: + // same set means nothing changed. + if (new HashSet(chosen, StringComparer.Ordinal) + .SetEquals(shown.Select(ws => ws.Id))) + return; // nothing changed: no override write. + ShowWritingSystems(field, chosen); + } + } + catch (Exception e) + { + Logger.WriteError("Configuring the row's writing systems failed.", e); + } + }); // 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) diff --git a/Src/xWorks/Avalonia/Hosting/ReorderVectorMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/ReorderVectorMenuAuthority.cs index 77eb516941..6e7917fb3f 100644 --- a/Src/xWorks/Avalonia/Hosting/ReorderVectorMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/ReorderVectorMenuAuthority.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; using SIL.FieldWorks.Common.FwAvalonia.Detail; using XCore; @@ -59,6 +60,12 @@ public DetailMenuItem Build(string menuId, ChoiceBase leaf) } } + // Neither owned id carries a list-populated submenu. + public IReadOnlyList BuildList(string menuId, string listId) + => throw new InvalidOperationException(string.Format( + "Menu '{0}' has a list submenu '{1}' this authority does not answer.", + menuId, listId)); + /// /// Whether the row's items may be reordered: an editable reference-vector row whose /// property is a real reference sequence or is marked reorderable. Move Left and Move diff --git a/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs b/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs index 0104400aaf..3baea28557 100644 --- a/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs +++ b/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs @@ -61,9 +61,9 @@ public static IReadOnlyList CreateMenuItems(XWindow window, stri /// owns is populated without any mediator display query and every leaf under it, /// submenus included, is answered by the authority, so nothing on the mediator (the /// hidden DataTree adapter included) takes part in it. Other ids keep the mediator path. + /// A list-populated submenu is answered too: the authority supplies its items, so the + /// group is never populated through the mediator. /// - /// An owned id contains a list-populated - /// submenu, which no authority can answer yet. public static IReadOnlyList CreateMenuItems(XWindow window, string[] menuIds, Func interceptor, IxCoreColleague temporaryColleague, IDetailMenuAuthority authority) @@ -203,18 +203,13 @@ private static List ConvertChildren(ChoiceGroup submenu, } // An owned submenu takes its label from the configuration and its children from the - // authority. Omitted when no child is visible, spliced when inline. A list submenu is - // refused, not left to the mediator. + // authority, a list-populated one included. Omitted when empty, spliced when inline. private static IEnumerable ConvertOwnedSubmenu(ChoiceGroup submenu, IDetailMenuAuthority authority, string ownedId) { - if (!string.IsNullOrEmpty(submenu.ListId)) - { - throw new NotSupportedException(string.Format( - "Menu '{0}' has a list-populated submenu '{1}' that no native authority can answer yet.", - ownedId, submenu.ListId)); - } - var children = ConvertChildren(submenu, null, authority, ownedId); + var children = string.IsNullOrEmpty(submenu.ListId) + ? ConvertChildren(submenu, null, authority, ownedId) + : Normalized(authority.BuildList(ownedId, submenu.ListId)); if (children.Count == 0 || submenu.IsInlineChoiceList) return children; return new[] @@ -223,6 +218,17 @@ private static IEnumerable ConvertOwnedSubmenu(ChoiceGroup subme }; } + // The authority's list items as the renderer needs them: disabled items lose their + // execute action like any other leaf, and stranded separators go. + private static List Normalized(IReadOnlyList items) + { + var result = new List(); + foreach (var item in items ?? Array.Empty()) + result.Add(item.IsSeparator ? item : WithoutExecuteWhenDisabled(item)); + TrimSeparators(result); + return result; + } + // A disabled leaf carries no execute action, so "Execute != null" means invokable for // every consumer -- programmatic invokers included, not just the pointer UI. private static DetailMenuItem WithoutExecuteWhenDisabled(DetailMenuItem item) diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs index 5f52eb2905..c9b4c6bf1e 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs @@ -738,7 +738,7 @@ public void ReorderVectorAuthority_RejectsALeafItDoesNotAnswer() } [Test] - public void LabelMenu_IsFullyOwned_UnlessItCarriesTheWritingSystemsList() + public void LabelMenu_IsFullyOwned_OnEveryRowKind() { MakeTwoSubentries(); var subentries = SubentriesField(); @@ -750,8 +750,18 @@ public void LabelMenu_IsFullyOwned_UnlessItCarriesTheWritingSystemsList() .First(f => f.MenuId == ObjectMenuAuthority.HelpMenuId); Assert.That(XCoreMenuBridge.OwnsAll(authority, LabelMenuIds(helpBound)), Is.True, "a row bound to the empty Help menu merges only owned ids"); - Assert.That(XCoreMenuBridge.OwnsAll(authority, LabelMenuIds(LexemeFormField())), Is.False, - "the multi-string menu's writing-system list still needs the mediator"); + var multiString = OwnedMultiStringField(); + Assert.That(XCoreMenuBridge.OwnsAll(m_view.CreateMenuAuthority(LabelMenuRequest(multiString, NoItem)), + LabelMenuIds(multiString)), Is.True, + "a multi-string row's menu, writing-system list included, is answered natively"); + var lexemeForm = LexemeFormField(); + Assert.That(m_view.CreateMultiStringMenuAuthority(lexemeForm).Owns(MultiStringMenuAuthority.MenuId), + Is.True, "the shared multi-string group is owned even on a row whose own menu id is not yet"); + Assert.That(XCoreMenuBridge.OwnsAll(m_view.CreateMenuAuthority(LabelMenuRequest(lexemeForm, NoItem)), + LabelMenuIds(lexemeForm)), Is.False, + "the Lexeme Form row still needs the adapter for mnuDataTree-LexemeForm, not for its writing systems"); + Assert.That(m_view.CreateMultiStringMenuAuthority(subentries).Owns(MultiStringMenuAuthority.MenuId), + Is.False, "a row that is not multi-string never claims the multi-string menu"); Assert.That(XCoreMenuBridge.OwnsAll(null, new[] { ReorderVectorMenuAuthority.MenuId }), Is.False); } @@ -795,6 +805,10 @@ public EchoAuthority(IEnumerable owned, params string[] hidden) public DetailMenuItem Build(string menuId, ChoiceBase leaf) => _hidden.Contains(leaf.HelpId) ? null : new DetailMenuItem(XCoreMenuBridge.StripAccelerator(leaf.Label), isEnabled: true); + + // Echoes the list's id as one item, so a test can see where it spliced in. + public IReadOnlyList BuildList(string menuId, string listId) + => new[] { new DetailMenuItem("list:" + listId, isEnabled: true) }; } // Records whether the mediator asked anyone to display the Always-visible command. @@ -893,12 +907,105 @@ public void TryLocateOverrideTarget_SkipsASequenceItemHeader_AndMapsAGhostRowToI } [Test] - public void OwnedMenu_WithAListSubmenu_IsRefused() + public void OwnedMenu_WithAListSubmenu_TakesItsItemsFromTheAuthority() { + // The writing-system list is the one list-populated submenu under an owned id. + // Populating it through the mediator would need the hidden adapter's slice. var ids = new[] { RecordEditView.MultiStringSliceMenuId }; - Assert.That(() => BuildWithSpy(ids, new EchoAuthority(ids), out _), - Throws.TypeOf().With.Message.Contains("WritingSystemOptionsForSlice"), - "a list-populated submenu has no configured leaves an authority could answer"); + var items = BuildWithSpy(ids, new EchoAuthority(ids), out var askedMediator); + + var writingSystems = FindItem(items, "Writing Systems"); + Assert.That(writingSystems, Is.Not.Null, "the owned submenu survives"); + Assert.That(writingSystems.Children.Select(c => c.Label), + Does.Contain("list:WritingSystemOptionsForSlice"), + "the authority's list items splice in where the list submenu sat"); + Assert.That(askedMediator, Is.False, + "an owned id asks the mediator nothing, the list submenu included"); + } + + // The multi-string menu's native authority: the Writing Systems submenu answered from + // the row and the shared rule, the rest delegated to the per-object authority. + + [Test] + public void MultiStringMenuAuthority_AnswersEveryLeafOfItsMenu() + { + var authority = m_view.CreateMultiStringMenuAuthority(LexemeFormField()); + var window = m_propertyTable.GetValue("window"); + var menu = window.GetContextMenuChoiceGroup(new[] { MultiStringMenuAuthority.MenuId }); + menu.PopulateNow(querySubmenuVisibility: false); + + var leaves = Leaves(menu).ToList(); + Assert.That(leaves.Select(l => l.HelpId), Is.EquivalentTo(new[] + { + MultiStringMenuAuthority.ShowAllCommandId, MultiStringMenuAuthority.ConfigureCommandId, + ObjectMenuAuthority.AlwaysVisibleCommandId, ObjectMenuAuthority.IfDataCommandId, + ObjectMenuAuthority.NormallyHiddenCommandId, ObjectMenuAuthority.MoveFieldUpCommandId, + ObjectMenuAuthority.MoveFieldDownCommandId, ObjectMenuAuthority.HelpCommandId + }), "the shipped menu defines exactly the leaves the authority knows"); + foreach (var leaf in leaves) + { + Assert.That(() => authority.Build(MultiStringMenuAuthority.MenuId, leaf), Throws.Nothing, + "the authority does not answer leaf '{0}'", leaf.HelpId); + } + // The list submenu has no configured leaves; the authority supplies its items. + Assert.That(authority.BuildList(MultiStringMenuAuthority.MenuId, + MultiStringMenuAuthority.WritingSystemListId), Is.Not.Empty, + "the writing-system list offers the row's options"); + } + + [Test] + public void MultiStringMenuAuthority_RejectsWhatItDoesNotAnswer() + { + var authority = m_view.CreateMultiStringMenuAuthority(LexemeFormField()); + var window = m_propertyTable.GetValue("window"); + var reorderMenu = window.GetContextMenuChoiceGroup(new[] { ReorderVectorMenuAuthority.MenuId }); + reorderMenu.PopulateNow(); + var foreignLeaf = reorderMenu.OfType() + .First(c => c.HelpId == ReorderVectorMenuAuthority.AlphabeticalOrderCommandId); + + Assert.That(() => authority.Build(MultiStringMenuAuthority.MenuId, foreignLeaf), + Throws.InvalidOperationException, "a foreign leaf is refused, never left to the mediator"); + Assert.That(() => authority.BuildList(MultiStringMenuAuthority.MenuId, "SomeOtherList"), + Throws.InvalidOperationException, "a foreign list is refused too"); + } + + [Test] + public void MultiStringMenu_NativeAuthority_RendersWhatTheInterceptorPathRendered() + { + UseHelpProvider(KnowsEveryTopic); + var field = LexemeFormField(); + var ids = LabelMenuIds(field); + EnsureAdapter(field.ObjectHvo, field.Field); + Assume.That(AdapterTargets(field), "precondition: the adapter twin is the Lexeme Form row"); + + var before = Describe(BuildItemsWithOverrideInterceptor(ids, field)); + var after = Describe(BuildAsTheHostDoes(ids, field)); + + Assert.That(after, Is.EqualTo(before), + "the native menu renders what the mediator path rendered, writing systems included"); + } + + [Test] + public void MultiStringMenu_IsBuiltWithoutTheAdapterOrTheMediator() + { + UseHelpProvider(KnowsEveryTopic); + var field = OwnedMultiStringField(); + // No EnsureAdapter: the hidden tree never exists. + + var items = BuildWithSpy(LabelMenuIds(field), + m_view.CreateMenuAuthority(LabelMenuRequest(field, NoItem)), out var asked); + + Assert.That(asked, Is.False, "nothing in a multi-string row's label menu reaches the mediator"); + var writingSystems = FindItem(items, "Writing Systems"); + Assert.That(writingSystems, Is.Not.Null); + var toggles = writingSystems.Children + .Where(c => c.Label != "Show all right now" && c.Label != "Configure...").ToList(); + Assert.That(toggles, Is.Not.Empty, "the row's writing systems are offered"); + Assert.That(toggles.Count(c => c.IsChecked), Is.GreaterThan(0), "the shown ones are checked"); + Assert.That(toggles.Where(c => c.IsEnabled).Select(c => c.Execute), Is.All.Not.Null, + "an enabled toggle executes"); + Assert.That(FindItem(items, "Show all right now")?.Execute, Is.Not.Null); + Assert.That(FindItem(items, "Configure...")?.Execute, Is.Not.Null); } // The per-object menu's native authority: Field Visibility, Move Field and Help answered @@ -1417,6 +1524,9 @@ public void WritingSystemToggle_TwoVernaculars_StoresTheReducedThenRestoredSet() "the stored op holds the set AFTER the uncheck, not the pre-click set"); // Re-check the same writing system on a freshly built menu: the ADD direction. + // The menu reads the row's stored selection, so take the row as the host's + // refresh recomposed it, override applied. + field = LexemeFormFieldWithOverrides(); writingSystems = BuildWritingSystemsSubmenu(field); var reAdd = writingSystems.Children.Single(c => !c.IsSeparator && c.Label == toggled); Assert.That(reAdd.IsChecked, Is.False, "the unchecked toggle stays unchecked"); @@ -1426,8 +1536,8 @@ public void WritingSystemToggle_TwoVernaculars_StoresTheReducedThenRestoredSet() var restored = StoredWritingSystems(field); TestContext.WriteLine("restored: " + string.Join(",", restored)); - // Exact order matters: the property appends the re-checked writing system at the - // end, but the copy canonicalizes to option order so rows never reorder. + // Exact order matters: a toggle stores the set in option order, so rows never + // reorder when a writing system is re-checked. Assert.That(restored, Is.EqualTo(new[] { "fr", "es" }), "re-checking stores the restored set in option order"); } @@ -1597,7 +1707,7 @@ private void InvokeShowAllRightNow(DetailField field) { EnsureAdapter(field.ObjectHvo, field.Field); var showAll = FindItem(BuildWritingSystemsSubmenu(field).Children, "Show all right now"); - Assert.That(showAll, Is.Not.Null, "the intercepted reveal item must materialize"); + Assert.That(showAll, Is.Not.Null, "the reveal item must materialize"); Assert.That(showAll.IsEnabled, Is.True); Assert.That(showAll.Execute, Is.Not.Null); showAll.Execute(); @@ -1767,6 +1877,22 @@ private IReadOnlyList ComposedFormWsTags(HashSet reveal) private DetailField LexemeFormField() => FindLexemeFormRow(DetailComposer.Compose(m_entry, Cache).Model); + // The same row as the host composes it after a refresh: the project override applied. + private DetailField LexemeFormFieldWithOverrides() + => FindLexemeFormRow(DetailComposer.Compose(m_entry, Cache, + overrides: (cls, layout) => GetOverrideStore().TryGet(cls, layout)).Model); + + // A multi-string row bound to no menu of its own, or to the empty Help menu, so every id + // its label menu merges is owned once the multi-string group is. + private DetailField OwnedMultiStringField() + { + var field = DetailComposer.Compose(m_entry, Cache).Model.Fields.FirstOrDefault(f => + f.IsMultiStringRow && f.ObjectHvo != 0 + && (string.IsNullOrEmpty(f.MenuId) || f.MenuId == ObjectMenuAuthority.HelpMenuId)); + Assume.That(field, Is.Not.Null, "the fixture composes a multi-string row with an owned binding"); + return field; + } + // The one locator for the Lexeme Form row (the MoForm's own Form field) in a composed // model. private DetailField FindLexemeFormRow(DetailModel model) @@ -1797,9 +1923,11 @@ private IReadOnlyList BuildItemsWithOverrideInterceptor(string[] private ViewDefinitionOverride ReadOverrideFor(DetailField field) => GetOverrideStore().TryGet(field.ClassName, field.LayoutName); + // The Writing Systems submenu as the host builds it: the multi-string group comes from + // its native authority, whatever the row's own menu id still needs. private DetailMenuItem BuildWritingSystemsSubmenu(DetailField field) { - var items = BuildItemsWithOverrideInterceptor( + var items = BuildAsTheHostDoes( new[] { "mnuDataTree-LexemeForm", "mnuDataTree-MultiStringSlice" }, field); var writingSystems = FindItem(items, "Writing Systems"); Assert.That(writingSystems, Is.Not.Null, From b898658c798ba7ed1321cc4f94932c5487731ae1 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Mon, 5 Oct 2026 08:43:01 -0400 Subject: [PATCH 3/4] LT-22691: Address review wording on the writing-system rule and menu Assert messages now state the expectation rather than the outcome, and one that described the code's history now describes the behaviour. The importer test for a part without optionalWs is renamed for what it asserts, and the comments say "expand" rather than "widen" and drop a mention of the WinForms slice. The menu authority computes a toggle's resulting set when the item is clicked rather than for every option on each menu open. The transition test that compares the native menu with the mediator path says that it is deleted with that path. Co-Authored-By: Claude Fable 5.1 --- .../FwAvaloniaTests/ViewDefinitionTests.cs | 4 ++-- .../DetailRules/FieldWritingSystemOptionsTests.cs | 8 ++++---- .../Avalonia/Hosting/MultiStringMenuAuthority.cs | 12 ++++++++---- .../Avalonia/Composer/FieldTypeComposerTests.cs | 5 ++--- .../Hosting/DetailObjectCommandExecutionTests.cs | 2 ++ 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs index 99de2d24b7..52a0ffbee4 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/ViewDefinitionTests.cs @@ -202,7 +202,7 @@ public void Import_NoVisibleWritingSystems_LeavesOverrideNull() [Test] public void Import_OptionalWsAndForceIncludeEnglish_RideTheNode() { - // Both widen what the field's Writing Systems menu may OFFER without changing what + // Both expand what the field's Writing Systems menu may OFFER without changing what // the field shows. Only the Pronunciation form uses optionalWs in the shipped parts. var model = Import(@" @@ -216,7 +216,7 @@ public void Import_OptionalWsAndForceIncludeEnglish_RideTheNode() } [Test] - public void Import_NoOptionalWs_LeavesTheWideningFactsUnset() + public void Import_WithoutOptionalWs_LeavesBothPropertiesUnset() { var model = Import(@" diff --git a/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs b/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs index 941a851c70..876a3b11e5 100644 --- a/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs +++ b/Src/FdoUi/FdoUiTests/DetailRules/FieldWritingSystemOptionsTests.cs @@ -50,9 +50,9 @@ public void Options_OfferAnUncheckedWritingSystem_ThatDefaultShownDoesNot() var shown = Ids(FieldWritingSystemOptions.DefaultShown(Cache, Analysis())); Assert.That(options, Does.Contain(offered.Id), - "the menu offers an active writing system the project has not checked"); + "the menu should offer an active writing system the project has not checked"); Assert.That(shown, Does.Not.Contain(offered.Id), - "but the field does not show it until the user chooses it"); + "but the field should not show it until the user chooses it"); Assert.That(shown, Is.SubsetOf(options), "everything shown must be offerable"); } @@ -78,7 +78,7 @@ public void Options_OfTheOptionalSpec_AreNotShownByDefault() var shown = Ids(FieldWritingSystemOptions.DefaultShown(Cache, Analysis(AllVernacular))); Assert.That(shown, Does.Not.Contain(vernacular), - "the optional spec widens what can be chosen, never what is shown unasked"); + "the optional spec expands what can be chosen, never what is shown unasked"); } [Test] @@ -93,7 +93,7 @@ public void Options_OfAPartWithoutAnOptionalSpec_AddNothing() var options = Ids(FieldWritingSystemOptions.Options(Cache, vernacularField)); Assert.That(options, Does.Not.Contain(analysisWs), - "a null optional spec must widen the options by nothing at all"); + "a part with no optionalWs should not offer the analysis writing systems"); Assert.That(options, Is.EqualTo(Ids(FieldWritingSystemOptions.Options(Cache, WritingSystemFieldSpec.FromLayout(0, AllVernacular, null, false))))); diff --git a/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs index 4efebac7a8..24172201d4 100644 --- a/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/MultiStringMenuAuthority.cs @@ -89,14 +89,18 @@ public IReadOnlyList BuildList(string menuId, string listId) private DetailMenuItem ToggleItem(WritingSystemMenuOption option, IReadOnlyList all) { - var toggled = all + return new DetailMenuItem(option.Label, option.CanUncheck, option.IsChecked, children: null, + execute: option.CanUncheck ? (Action)(() => _show(_field, Toggled(option, all))) : null); + } + + // The shown set with one writing system flipped, computed only when its item is clicked. + private static IReadOnlyList Toggled(WritingSystemMenuOption option, + IReadOnlyList all) + => all .Where(o => string.Equals(o.Id, option.Id, StringComparison.OrdinalIgnoreCase) ? !o.IsChecked : o.IsChecked) .Select(o => o.Id) .ToList(); - return new DetailMenuItem(option.Label, option.CanUncheck, option.IsChecked, children: null, - execute: option.CanUncheck ? (Action)(() => _show(_field, toggled)) : null); - } } } diff --git a/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs b/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs index d57ec168ef..19886bc7cc 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Composer/FieldTypeComposerTests.cs @@ -108,8 +108,7 @@ public void PerFieldWs_ShowsAWritingSystemTheProjectHasNotChecked() Assert.That(restricted.Values.Select(v => v.WsTag), Is.EqualTo(new[] { offered.Id }), "the row shows exactly the writing system chosen, not every writing system"); - // Show all right now reveals every OPTION, unchecked ones included, as the - // WinForms slice does. + // Show all right now reveals every OPTION, unchecked ones included. var templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(form.StableId); var revealed = ComposedFormRow(null, new HashSet { templateId }); Assert.That(revealed.Values.Select(v => v.WsTag), Does.Contain(offered.Id), @@ -130,7 +129,7 @@ public void PerFieldWs_WithNothingStored_ShowsWhatItAlwaysShowed() var expected = DetailComposer.ResolveWritingSystems(Cache, "all vernacular").Select(w => w.Id); Assert.That(ComposedFormRow(null, null).Values.Select(v => v.WsTag), Is.EqualTo(expected), - "a row with no stored selection composes the set it composed before the shared rule"); + "a row with no stored selection should compose its full configured set"); } // An override restricting the Form row to one writing system. diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs index c9b4c6bf1e..e989d1081f 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs @@ -969,6 +969,8 @@ public void MultiStringMenuAuthority_RejectsWhatItDoesNotAnswer() Throws.InvalidOperationException, "a foreign list is refused too"); } + // A transition contract: it compares the native menu with the mediator path and is + // deleted with that path. The standalone contracts above carry the behaviour after. [Test] public void MultiStringMenu_NativeAuthority_RendersWhatTheInterceptorPathRendered() { From 16784be837a3fc7ea2e5da59774b068b0702be61 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Tue, 6 Oct 2026 13:51:39 -0400 Subject: [PATCH 4/4] LT-22691: Refuse list submenus on the reference-choices item menu ReferenceItemMenuAuthority arrived from main without the BuildList member this branch adds to IDetailMenuAuthority, so the merged tree did not compile. mnuReferenceChoices has no list-populated submenu, so the authority refuses every list, as ObjectMenuAuthority and ReorderVectorMenuAuthority do. Co-Authored-By: Claude Opus 5.5 --- Src/xWorks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Src/xWorks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs b/Src/xWorks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs index 5654dd738f..5522e400fd 100644 --- a/Src/xWorks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs +++ b/Src/xWorks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs @@ -3,6 +3,7 @@ // (http://www.gnu.org/licenses/lgpl-2.1.html) using System; +using System.Collections.Generic; using SIL.FieldWorks.Common.DetailRules; using SIL.FieldWorks.Common.FwAvalonia.Detail; using SIL.FieldWorks.FdoUi; @@ -126,6 +127,12 @@ public DetailMenuItem Build(string menuId, ChoiceBase leaf) } } + // The owned id carries no list-populated submenu. + public IReadOnlyList BuildList(string menuId, string listId) + => throw new InvalidOperationException(string.Format( + "Menu '{0}' has a list submenu '{1}' this authority does not answer.", + menuId, listId)); + // The tool a command's parameters name, or null. private static string ToolOf(CommandChoice command) => command == null ? null : XmlUtils.GetOptionalAttributeValue(command.CommandObject.Parameters[0], "tool");