Skip to content

LT-22691: Let a menu authority answer a whole menu id, submenus included #1153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mark-sil wants to merge 1 commit into
base: main
Choose a base branch
from
+277 −76
Open

LT-22691: Let a menu authority answer a whole menu id, submenus included #1153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions Src/XCore/xCoreInterfaces/ChoiceGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,11 @@ public string ListId
}
}
protected override void Populate()
{
Populate(querySubmenuVisibility: true);
}

private void Populate(bool querySubmenuVisibility)
{
Clear();
if (IsAListGroup)
Expand All @@ -437,12 +442,12 @@ protected override void Populate()
{
foreach (XmlNode n in m_configurationNodes)
{
Populate(n);
Populate(n, querySubmenuVisibility);
}
}
else
{
Populate(m_configurationNode);
Populate(m_configurationNode, querySubmenuVisibility);
}
}

Expand All @@ -455,6 +460,16 @@ public void PopulateNow()
Populate();
}

/// <summary>
/// Populates the group, keeping every nested submenu when
/// <paramref name="querySubmenuVisibility"/> is false instead of asking the colleagues
/// whether one of its items is visible; the caller then decides the submenu's fate.
/// </summary>
public void PopulateNow(bool querySubmenuVisibility)
{
Populate(querySubmenuVisibility);
}

protected void PopulateFromList()
{
/// Just before this group is displayed, allow the group's contents to be modified by colleagues
Expand Down Expand Up @@ -521,6 +536,11 @@ public bool HasSubGroups()
}

protected void Populate(XmlNode node)
{
Populate(node, querySubmenuVisibility: true);
}

private void Populate(XmlNode node, bool querySubmenuVisibility)
{
Debug.Assert( node != null);
XmlNodeList items = node.SelectNodes("item | menu | group");
Expand All @@ -534,11 +554,11 @@ protected void Populate(XmlNode node)
break;
case "menu":
ChoiceGroup group = new ChoiceGroup(m_mediator, m_propertyTable, m_adapter, childNode, this);
group.Populate(childNode);
group.Populate(childNode, querySubmenuVisibility);
//Only add the submenu if it contains a list of items what will be visible.
//We do not want an empty submenu LT-8791.
string hasList = XmlUtils.GetAttributeValue(childNode, "list");
if (hasList != null || ASubmenuItemIsVisible(group))
if (hasList != null || !querySubmenuVisibility || ASubmenuItemIsVisible(group))
this.Add(group);
break;
case "group": //for tree views in the sidebar
Expand Down
80 changes: 50 additions & 30 deletions Src/xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,12 +624,44 @@ internal void AddOverrideCommands(OverrideCommandRegistry registry, DetailField
// 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(d, field));
(c, d) => ShowAllWritingSystemsItem(XCoreMenuBridge.StripAccelerator(d.Text), 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.
ViewNodeLocation location = null;
// Unknown/stale target: leave the field commands on mediator dispatch rather than
// guess.
if (!TryLocateOverrideTarget(field, out var templateId, out var location))
return;
registry.Add("CmdAlwaysVisible",
(c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.Always));
registry.Add("CmdIfData",
(c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.IfData));
registry.Add("CmdNormallyHidden",
(c, d) => VisibilityItem(LabelOf(d), field, templateId, location, ViewVisibility.Never));
registry.Add("CmdDataTree-MoveFieldUp",
(c, d) => MoveItem(LabelOf(d), field, location, up: true));
registry.Add("CmdDataTree-MoveFieldDown",
(c, d) => MoveItem(LabelOf(d), field, location, up: false));
}

private static string LabelOf(UIItemDisplayProperties display)
=> XCoreMenuBridge.StripAccelerator(display.Text);

/// <summary>
/// Locates the row's node in its own compiled model, with the current override applied,
/// so visibility checkmarks and move enablement reflect the live state. False, with the
/// reason logged, when the row lacks class or layout context or an override store, when
/// the compile fails, or when the model has no node for the row's template id.
/// </summary>
internal bool TryLocateOverrideTarget(DetailField field, out string templateId,
out ViewNodeLocation location)
{
templateId = null;
location = null;
if (field == null || string.IsNullOrEmpty(field.ClassName) || string.IsNullOrEmpty(field.LayoutName)
|| ViewOverrideStore == null)
{
return false;
}
templateId = ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId);
try
{
if (Cache.ServiceLocator.ObjectRepository.TryGetObject(field.ObjectHvo, out var fieldObj))
Expand All @@ -642,45 +674,34 @@ internal void AddOverrideCommands(OverrideCommandRegistry registry, DetailField
}
catch (Exception e)
{
Logger.WriteError("Resolving the field's override target failed; this row's "
+ "menu-button commands fall back to ordinary command dispatch.", e);
return;
Logger.WriteError("Resolving the field's override target failed; its Field Visibility "
+ "and Move Field commands are not retargeted to the override layer.", e);
return false;
}

// Unknown/stale target: leave the field commands on the legacy path rather than
// guess.
if (location != null)
if (location == null)
{
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));
Logger.WriteEvent(string.Format("Detail row '{0}' has no node in its compiled model; its "
+ "Field Visibility and Move Field commands are not retargeted to the override layer.",
templateId));
return false;
}
return true;
}

// 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(UIItemDisplayProperties display, DetailField field,
private DetailMenuItem VisibilityItem(string label, DetailField field,
string templateId, ViewNodeLocation location, ViewVisibility target)
{
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(UIItemDisplayProperties display, DetailField field,
ViewNodeLocation location, bool up)
private DetailMenuItem MoveItem(string label, DetailField field, ViewNodeLocation location, bool up)
{
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);
Expand All @@ -692,9 +713,8 @@ private DetailMenuItem MoveItem(UIItemDisplayProperties display, DetailField fie
/// record) and recomposes. The reveal is view state, not a command, so the item
/// dispatches nothing and never writes the override.
/// </summary>
private DetailMenuItem ShowAllWritingSystemsItem(UIItemDisplayProperties display, DetailField field)
=> new DetailMenuItem(XCoreMenuBridge.StripAccelerator(display.Text), isEnabled: true,
isChecked: false, children: null, execute: () =>
private DetailMenuItem ShowAllWritingSystemsItem(string label, DetailField field)
=> new DetailMenuItem(label, isEnabled: true, isChecked: false, children: null, execute: () =>
{
m_showAllWsFields.Add(ViewDefinitionOverrideEditor.StripRuntimeSuffix(field.StableId));
RefreshAvaloniaDetail();
Expand Down
125 changes: 83 additions & 42 deletions Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
namespace SIL.FieldWorks.XWorks
{
/// <summary>
/// Converts an xCore context-menu <see cref="ChoiceGroup"/> into the neutral
/// <see cref="DetailMenuItem"/> model the Avalonia detail view renders as a native MenuFlyout.
/// Labels, enablement, checkmarks, submenus, and execution all run through the SAME xCore
/// machinery the WinForms adapter uses (GetDisplayProperties -> mediator Display* round-trip;
/// OnClick -> mediator command dispatch) -- only the rendering changes. Because this consumes
/// the
/// shared engine, it serves every DTMenuHandler-hosting tool (Grammar, Notebook, Lists,
/// Words), not just the Lexicon.
/// Converts xCore context menus into the neutral <see cref="DetailMenuItem"/> model the
/// Avalonia detail view renders as a native MenuFlyout. A menu id without a native
/// authority runs through the SAME xCore machinery the WinForms adapter uses
/// (GetDisplayProperties -> mediator Display* round-trip; OnClick -> mediator command
/// dispatch), only the rendering changes; an owned id is answered by its authority alone.
/// Because this consumes the shared engine, it serves every DTMenuHandler-hosting tool
/// (Grammar, Notebook, Lists, Words), not just the Lexicon.
/// </summary>
public static class XCoreMenuBridge
{
Expand Down Expand Up @@ -58,23 +57,47 @@ public static IReadOnlyList<DetailMenuItem> CreateMenuItems(XWindow window, stri
=> CreateMenuItems(window, menuIds, interceptor, temporaryColleague, null);

/// <summary>
/// As the interceptor overload, plus a native <paramref name="authority"/> that answers
/// every leaf under the menu ids it owns BEFORE the mediator is asked: those leaves get
/// no Display* round trip and no interceptor call, so nothing on the mediator (the
/// hidden DataTree adapter included) takes part in them. Leaves under other ids keep
/// the mediator path.
/// As the interceptor overload, plus a native <paramref name="authority"/>. A menu id it
/// 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.
/// </summary>
/// <exception cref="NotSupportedException">An owned id contains a list-populated
/// submenu, which no authority can answer yet.</exception>
public static IReadOnlyList<DetailMenuItem> CreateMenuItems(XWindow window, string[] menuIds,
Func<ChoiceBase, UIItemDisplayProperties, DetailMenuItem> interceptor,
IxCoreColleague temporaryColleague, IDetailMenuAuthority authority)
{
var group = window?.GetContextMenuChoiceGroup(menuIds);
if (group == null)
return new List<DetailMenuItem>();
var items = new List<DetailMenuItem>();
if (window == null || menuIds == null)
return items;

// One group per id keeps each id's ownership known; the source menus contribute
// their items in order, as the merged group's population did.
var groups = new List<(ChoiceGroup Group, string OwnedId)>();
foreach (var id in menuIds)
{
if (string.IsNullOrEmpty(id))
continue;
var group = window.GetContextMenuChoiceGroup(new[] { id });
if (group != null)
groups.Add((group, authority != null && authority.Owns(id) ? id : null));
}
if (groups.Count == 0)
return items;

if (temporaryColleague != null)
window.Mediator.AddTemporaryColleague(temporaryColleague);
group.PopulateNow();
return Convert(group, interceptor, authority);
foreach (var (group, ownedId) in groups)
{
// An owned group keeps its submenus regardless of what colleagues would say;
// Convert drops a submenu only when the authority hides every leaf in it.
group.PopulateNow(querySubmenuVisibility: ownedId == null);
items.AddRange(Convert(group, interceptor, authority, ownedId));
}

TrimSeparators(items);
return items;
}

/// <summary>
Expand All @@ -94,26 +117,11 @@ public static bool OwnsAll(IDetailMenuAuthority authority, IEnumerable<string> m
return true;
}

// The owned menu id a leaf belongs to, or null. A merged group flattens its source
// menus, so ownership comes from the nearest enclosing menu element the authority owns.
private static string OwnedMenuIdOf(ChoiceBase leaf, IDetailMenuAuthority authority)
{
if (authority == null)
return null;
for (var node = leaf.ConfigurationNode?.ParentNode; node != null; node = node.ParentNode)
{
if (node.Name != "menu")
continue;
var id = node.Attributes?["id"]?.Value;
if (!string.IsNullOrEmpty(id) && authority.Owns(id))
return id;
}
return null;
}

// ownedId: the menu id the authority answers for this group and its submenus, or
// null on the mediator path. Edge separators stay: they divide merged groups.
private static List<DetailMenuItem> Convert(ChoiceGroup group,
Func<ChoiceBase, UIItemDisplayProperties, DetailMenuItem> interceptor,
IDetailMenuAuthority authority)
IDetailMenuAuthority authority, string ownedId)
{
var items = new List<DetailMenuItem>();
foreach (var member in group)
Expand All @@ -125,8 +133,14 @@ private static List<DetailMenuItem> Convert(ChoiceGroup group,
}
else if (member is ChoiceGroup submenu)
{
if (ownedId != null)
{
items.AddRange(ConvertOwnedSubmenu(submenu, authority, ownedId));
continue;
}

submenu.PopulateNow();
var children = Convert(submenu, interceptor, authority);
var children = ConvertChildren(submenu, interceptor, authority, null);
if (children.Count == 0)
continue;

Expand All @@ -146,11 +160,9 @@ private static List<DetailMenuItem> Convert(ChoiceGroup group,
}
else if (member is ChoiceBase choice)
{
// A natively owned leaf is answered whole (hidden, or label/state/execute)
// with no mediator round trip.
var ownedId = OwnedMenuIdOf(choice, authority);
if (ownedId != null)
{
// The authority answers the leaf whole: hidden, or label/state/execute.
var native = authority.Build(ownedId, choice);
if (native != null)
items.Add(WithoutExecuteWhenDisabled(native));
Expand All @@ -177,11 +189,40 @@ private static List<DetailMenuItem> Convert(ChoiceGroup group,
display.Enabled ? (Action)(() => captured.OnClick(null, EventArgs.Empty)) : null));
}
}

TrimSeparators(items);
return items;
}

// A submenu's children. Hiding items can leave a separator first or last; those go.
private static List<DetailMenuItem> ConvertChildren(ChoiceGroup submenu,
Func<ChoiceBase, UIItemDisplayProperties, DetailMenuItem> interceptor,
IDetailMenuAuthority authority, string ownedId)
{
var children = Convert(submenu, interceptor, authority, ownedId);
TrimSeparators(children);
return children;
}

// 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.
private static IEnumerable<DetailMenuItem> 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);
if (children.Count == 0 || submenu.IsInlineChoiceList)
return children;
return new[]
{
new DetailMenuItem(StripAccelerator(submenu.Label), isEnabled: true, isChecked: false, children)
};
}

// 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)
Expand Down
Loading
Loading