diff --git a/Src/xWorks/Avalonia/Hosting/OverrideCommandRegistry.cs b/Src/xWorks/Avalonia/Hosting/OverrideCommandRegistry.cs new file mode 100644 index 0000000000..84ff78a1bd --- /dev/null +++ b/Src/xWorks/Avalonia/Hosting/OverrideCommandRegistry.cs @@ -0,0 +1,44 @@ +// 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 SIL.FieldWorks.Common.FwAvalonia.Detail; +using XCore; + +namespace SIL.FieldWorks.XWorks +{ + /// + /// The commands the Avalonia detail view retargets away from mediator dispatch, as ordered + /// (matcher, builder) entries. builds the replacement item from the + /// first matching entry; null leaves the command on its normal dispatch. + /// + internal sealed class OverrideCommandRegistry + { + private readonly List, + Func>> _entries + = new List, + Func>>(); + + public void Add(string helpId, Func build) + => Add(c => string.Equals(c.HelpId, helpId, StringComparison.Ordinal), build); + + /// Registers by matcher, for items that carry no command id. + public void Add(Func matches, + Func build) + => _entries.Add(new KeyValuePair, + Func>(matches, build)); + + public DetailMenuItem TryBuild(ChoiceBase choice, UIItemDisplayProperties display) + { + foreach (var entry in _entries) + { + if (entry.Key(choice)) + return entry.Value(choice, display); + } + + return null; + } + } +} diff --git a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs index 6349454c14..9595d33b8c 100644 --- a/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs +++ b/Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs @@ -563,14 +563,15 @@ private ViewDefinitionOverride ResolveViewOverride(string className, string layo + "'; using the shipped definition.", error)); /// - /// Builds the interceptor that retargets the per-field Field Visibility and - /// Move Field commands to the project override layer for the Avalonia detail view. Returns null - /// (intercept nothing -- every command keeps its normal mediator dispatch) when the - /// clicked row - /// carries no (class, layout) context, e.g. the first-slice fallback rows; that keeps the legacy - /// behavior intact when the override layer cannot be addressed. + /// Builds the interceptor that retargets the per-field Field Visibility, Move Field, and + /// writing-system commands to the project override layer for the Avalonia detail view. + /// Returns null (intercept nothing -- every command keeps its normal mediator dispatch) + /// when the clicked row carries no (class, layout) context, e.g. the first-slice fallback + /// rows; that keeps the legacy behavior intact when the override layer cannot be + /// addressed. /// - private Func BuildOverrideCommandInterceptor(DetailField field) + private Func BuildOverrideCommandInterceptor( + DetailField field) { if (field == null || string.IsNullOrEmpty(field.ClassName) || string.IsNullOrEmpty(field.LayoutName) || ViewOverrideStore == null) @@ -578,6 +579,12 @@ private Func BuildOverrideCommandInterceptor(DetailF return null; } + // 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. + var registry = new OverrideCommandRegistry(); + registry.Add(IsWritingSystemVisibilityChoice, (c, d) => WritingSystemItem(c, d, field)); + var templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId); // Locate the clicked node in the field's OWN compiled model (with any current override // already applied), so visibility checkmarks and move enablement reflect the live state. @@ -596,54 +603,161 @@ private Func BuildOverrideCommandInterceptor(DetailF { Logger.WriteError("Resolving the field's override target failed; the gear-menu field " + "commands fall back to the legacy path for this row.", e); - return null; + return registry.TryBuild; } - if (location == null) - return null; // unknown/stale target: leave commands on the legacy path rather than guess. - - return choice => + // Unknown/stale target: leave the field commands on the legacy path rather than + // guess. + if (location != null) { - switch (choice.HelpId) - { - case "CmdAlwaysVisible": - return VisibilityItem(choice, field, templateId, location, ViewVisibility.Always); - case "CmdIfData": - return VisibilityItem(choice, field, templateId, location, ViewVisibility.IfData); - case "CmdNormallyHidden": - return VisibilityItem(choice, field, templateId, location, ViewVisibility.Never); - case "CmdDataTree-MoveFieldUp": - return MoveItem(choice, field, location, up: true); - case "CmdDataTree-MoveFieldDown": - return MoveItem(choice, field, location, up: false); - default: - return null; // not a field command: keep its normal mediator dispatch. - } - }; + registry.Add("CmdAlwaysVisible", + (c, d) => VisibilityItem(d, field, templateId, location, ViewVisibility.Always)); + registry.Add("CmdIfData", + (c, d) => VisibilityItem(d, field, templateId, location, ViewVisibility.IfData)); + registry.Add("CmdNormallyHidden", + (c, d) => VisibilityItem(d, field, templateId, location, ViewVisibility.Never)); + registry.Add("CmdDataTree-MoveFieldUp", + (c, d) => MoveItem(d, field, location, up: true)); + registry.Add("CmdDataTree-MoveFieldDown", + (c, d) => MoveItem(d, field, location, up: false)); + } + + return registry.TryBuild; } // A Field Visibility menu item: checked when it is the field's current visibility, executes the // SetVisibility override mutation (idempotent -- re-choosing the current value is a // harmless write). - private DetailMenuItem VisibilityItem(ChoiceBase choice, DetailField field, + private DetailMenuItem VisibilityItem(UIItemDisplayProperties display, DetailField field, string templateId, ViewNodeLocation location, ViewVisibility target) { - var label = XCoreMenuBridge.StripAccelerator(choice.GetDisplayProperties().Text); + var label = XCoreMenuBridge.StripAccelerator(display.Text); var isChecked = location.Visibility == target; return new DetailMenuItem(label, isEnabled: true, isChecked: isChecked, children: null, execute: () => ApplyFieldVisibility(field, templateId, target)); } // A Move Field item: disabled at the first sibling (up) / last sibling (down) / when alone. - private DetailMenuItem MoveItem(ChoiceBase choice, DetailField field, + private DetailMenuItem MoveItem(UIItemDisplayProperties display, DetailField field, ViewNodeLocation location, bool up) { - var label = XCoreMenuBridge.StripAccelerator(choice.GetDisplayProperties().Text); + var label = XCoreMenuBridge.StripAccelerator(display.Text); var canMove = up ? location.CanMoveUp : location.CanMoveDown; return new DetailMenuItem(label, isEnabled: canMove, isChecked: false, children: null, execute: canMove ? (Action)(() => ApplyMoveField(field, location, up)) : null); } + /// + /// 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 excluded: it is a transient reveal on the slice, + /// not a configuration change, so persisting it would wrongly pin the full set. + /// + private static bool IsWritingSystemVisibilityChoice(ChoiceBase choice) + { + if (choice is ListPropertyChoice list) + { + return string.Equals(list.ParentProperty, + PropertyConstants.CurrentContextMenuSelectedWsIds, StringComparison.Ordinal); + } + + return string.Equals(choice.HelpId, "CmdDataTree-WritingSystemMenu-Configure", + StringComparison.Ordinal); + } + + /// + /// 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. + /// + 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) + { + 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."); + 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 op = new ViewOverrideOperation(ViewOverrideOperationKind.SetVisibleWritingSystems, + ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId), + writingSystems: selected); + MutateOverrideAndRefresh(field, op); + } + catch (Exception e) + { + Logger.WriteError("Copying the writing-system selection into the view override 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(); + // 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/XCoreMenuBridge.cs b/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs index 5efffbd95e..9e9f775742 100644 --- a/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs +++ b/Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs @@ -32,16 +32,18 @@ public static IReadOnlyList CreateMenuItems(XWindow window, stri /// /// As , but lets the host RETARGET specific leaf /// commands for the Avalonia detail view (advanced-entry-view). For each command leaf, the - /// is offered the leaf (so the host can - /// read the localized label and command id from it); if it returns a non-null + /// is offered the leaf and its + /// already-computed display properties (so the host reads the localized label and state + /// without a second Display* round trip); if it returns a non-null /// , that item (its label/checked/enabled/execute) is used INSTEAD of /// the default xCore-dispatched item. Returning null leaves the command on its normal mediator /// path. This is how the per-field Field Visibility / Move Field commands route to the project /// override layer while Help and every other item keep working unchanged. The interceptor only - /// sees leaf commands (submenus pass through). + /// sees leaf commands (submenus pass through). Every leaf, default or retargeted, is + /// normalized so a disabled item carries no execute action. /// public static IReadOnlyList CreateMenuItems(XWindow window, string[] menuIds, - Func interceptor) + Func interceptor) { var group = window?.GetContextMenuChoiceGroup(menuIds); if (group == null) @@ -50,7 +52,8 @@ public static IReadOnlyList CreateMenuItems(XWindow window, stri return Convert(group, interceptor); } - private static List Convert(ChoiceGroup group, Func interceptor) + private static List Convert(ChoiceGroup group, + Func interceptor) { var items = new List(); foreach (var member in group) @@ -90,16 +93,17 @@ private static List Convert(ChoiceGroup group, Func captured.OnClick(null, EventArgs.Empty))); + display.Checked, null, + display.Enabled ? (Action)(() => captured.OnClick(null, EventArgs.Empty)) : null)); } } @@ -107,6 +111,13 @@ private static List Convert(ChoiceGroup group, Func item.IsEnabled || item.Execute == null + ? item + : new DetailMenuItem(item.Label, isEnabled: false, item.IsChecked, item.Children, null); + // xCore marks the accelerator with a single '_' before the mnemonic; WinForms // translates it to '&'. Avalonia shows text raw, so strip only the first // marker: any later underscore is literal content. diff --git a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs index 1539a74cb5..834c867b24 100644 --- a/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs +++ b/Src/xWorks/xWorksTests/Avalonia/Hosting/DetailObjectCommandExecutionTests.cs @@ -13,9 +13,11 @@ using SIL.FieldWorks.Common.Controls; using SIL.FieldWorks.Common.FwAvalonia; using SIL.FieldWorks.Common.FwAvalonia.Detail; +using SIL.FieldWorks.Common.FwAvalonia.ViewDefinition; using SIL.FieldWorks.Common.Framework.DetailControls; using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; +using SIL.LCModel.Core.WritingSystems; using SIL.LCModel.Infrastructure; using XCore; // Both namespaces above define DataTree; the adapter tests mean the legacy WinForms one. @@ -363,10 +365,165 @@ public void LexemeFormSliceMenu_WritingSystemsSubmenu_ListsTheProjectWritingSyst "the spliced entries are the project's vernacular writing systems (the Lexeme Form's ws set)"); } + // THE decisive copy test: unchecking one of two vernaculars must store the REDUCED + // set (the slice still holds the pre-click set right after OnClick), and re-checking + // must store the restored set. + [Test] + public void WritingSystemToggle_TwoVernaculars_StoresTheReducedThenRestoredSet() + { + CoreWritingSystemDefinition second = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + { + Cache.ServiceLocator.WritingSystemManager.GetOrSet("es", out second); + Cache.ServiceLocator.WritingSystems.AddToCurrentVernacularWritingSystems(second); + }); + var field = LexemeFormField(); + try + { + EnsureAdapter(m_entry.LexemeFormOA.Hvo, "Form"); + var writingSystems = BuildWritingSystemsSubmenu(field); + var checkedEntries = writingSystems.Children + .Where(c => !c.IsSeparator && c.IsChecked && c.Execute != null).ToList(); + TestContext.WriteLine("checked: " + string.Join(", ", + checkedEntries.Select(e => e.Label))); + Assert.That(checkedEntries.Count, Is.EqualTo(2), + "precondition: both vernacular writing systems start visible"); + var toggled = checkedEntries[0].Label; + + checkedEntries[0].Execute(); + DrainMediatorAndIdleQueues(); + + var reduced = StoredWritingSystems(field); + TestContext.WriteLine("reduced: " + string.Join(",", reduced)); + Assert.That(reduced, Is.EqualTo(new[] { "es" }), + "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. + 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"); + Assert.That(reAdd.Execute, Is.Not.Null, "an unchecked toggle is clickable"); + reAdd.Execute(); + DrainMediatorAndIdleQueues(); + + 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. + Assert.That(restored, Is.EqualTo(new[] { "fr", "es" }), + "re-checking stores the restored set in option order"); + } + finally + { + DeleteOverrideFor(field); + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, () => + Cache.ServiceLocator.WritingSystems.CurrentVernacularWritingSystems.Remove(second)); + } + } + + // The Lexeme Form row is keyed by the MoForm's own descended (class, layout). Compose + // must resolve an override for it, or the row's customizations are written and never + // read. + [Test] + public void Compose_ResolvesTheOverrideForADescendedLayout() + { + var field = LexemeFormField(); + var store = GetOverrideStore(); + var templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId); + TestContext.WriteLine( + $"class={field.ClassName} layout={field.LayoutName} template={templateId}"); + try + { + store.Save(new ViewDefinitionOverride(field.ClassName, field.LayoutName, "detail", + new[] + { + new ViewOverrideOperation(ViewOverrideOperationKind.SetLabel, templateId, + label: "OverrideMarker") + }, null)); + + var asked = new List(); + var recomposed = DetailComposer.Compose(m_entry, Cache, showHiddenFields: false, + overrides: (cls, layout) => + { + asked.Add(cls + "/" + layout); + return store.TryGet(cls, layout); + }); + TestContext.WriteLine("compose asked for: " + string.Join(", ", asked)); + + var row = recomposed.Model.Fields.Single(f => f.Field == "Form" + && f.Kind == DetailFieldKind.Text && f.ObjectHvo == m_entry.LexemeFormOA.Hvo); + Assert.That(row.Label, Is.EqualTo("OverrideMarker"), + "an override keyed by the row's own (class, layout) must reach the composed row"); + } + finally + { + DeleteOverrideFor(field); + } + } + // ----------------------------------------------------------------- // Helpers -- production-path command drivers // ----------------------------------------------------------------- + // The composed Lexeme Form row, as the production host resolves it before raising a + // menu request. + private DetailField LexemeFormField() + => DetailComposer.Compose(m_entry, Cache).Model.Fields.Single(f => f.Field == "Form" + && f.Kind == DetailFieldKind.Text && f.ObjectHvo == m_entry.LexemeFormOA.Hvo); + + private ViewDefinitionOverrideStore GetOverrideStore() + { + var storeProperty = typeof(RecordEditView).GetProperty("ViewOverrideStore", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(storeProperty, Is.Not.Null, "the override store seam must exist"); + var store = (ViewDefinitionOverrideStore)storeProperty.GetValue(m_view); + Assert.That(store, Is.Not.Null, "the project must have a reachable override store"); + return store; + } + + // Materializes a menu the way OnDetailMenuRequested does, WITH the override interceptor, + // so the intercepted writing-system items are the ones under test. + private IReadOnlyList BuildItemsWithOverrideInterceptor(string[] menuIds, + DetailField field) + { + var build = typeof(RecordEditView).GetMethod("BuildOverrideCommandInterceptor", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(build, Is.Not.Null, "the override interceptor seam must exist"); + var interceptor = (Func)build.Invoke( + m_view, new object[] { field }); + TestContext.WriteLine("interceptor built: " + (interceptor != null)); + var window = m_propertyTable.GetValue("window"); + return XCoreMenuBridge.CreateMenuItems(window, menuIds, interceptor); + } + + private ViewDefinitionOverride ReadOverrideFor(DetailField field) + => GetOverrideStore().TryGet(field.ClassName, field.LayoutName); + + private DetailMenuItem BuildWritingSystemsSubmenu(DetailField field) + { + var items = BuildItemsWithOverrideInterceptor( + new[] { "mnuDataTree-LexemeForm", "mnuDataTree-MultiStringSlice" }, field); + var writingSystems = FindItem(items, "Writing Systems"); + Assert.That(writingSystems, Is.Not.Null, + "precondition: the Writing Systems submenu is present"); + return writingSystems; + } + + // The writing systems the stored override op carries for the field's row. + private IReadOnlyList StoredWritingSystems(DetailField field) + { + var stored = ReadOverrideFor(field); + Assert.That(stored, Is.Not.Null, + "toggling a writing system must write a project override"); + return stored.Operations.Single(o => + o.Kind == ViewOverrideOperationKind.SetVisibleWritingSystems).WritingSystems; + } + + // Saving an empty patch deletes the file, so overrides never leak between tests. + private void DeleteOverrideFor(DetailField field) + => GetOverrideStore().Save(new ViewDefinitionOverride( + field.ClassName, field.LayoutName, "detail", null, null)); + // Drives a SLICE-menu command exactly as OnDetailMenuRequested(Kind=SliceMenu) does: ensure the // adapter targets the object, materialize the menu (menuId + the host-appended mnuDataTree-Object) // through XCoreMenuBridge, find the item by label, invoke its Execute (mediator dispatch). Then