Repository navigation
feat: nested sub-menus in the node menu, on every client - #1255
Merged
Merged
Conversation
The node menu had grown to ~15 flat entries. Menu items can now nest: a
parent opens a sub-menu instead of acting, and the three export actions
(PDF / DOCX / email) sit behind one "Export" entry.
Children were already modelled on NodeMenuItemDefinition and already
recursed through Localized/Equals, but NO web client rendered them.
Blazor's FlattenMenuItems (and its portal-next port) DELETED the parent
and spliced its children inline behind a divider; React Native had no
`children` field at all, so a whole subtree was invisible there. With two
GitHub sync sources configured that produced two identical "Sync now /
Update to latest / Check branch" triplets with nothing to say which repo
either belonged to — the parent's label, 🐙 icon and repo tooltip were the
only thing distinguishing them, and all three were thrown away.
Contract
- GroupArea ("_group") sentinel, sibling of the existing "_separator", for
a parent that exists only to hold children. IsSubmenuParent: ANY entry
carrying children is a parent and is never activatable — neither FAST nor
Fluent v9 can express "activates AND opens", both toggle instead.
Aggregator (one place, so renderers stay dumb)
- Children sorted by Order at every depth. Only the top level was sorted
before, so a sub-menu came out in provider-append order.
- A _group parent with no surviving children is pruned, bottom-up. Items
are permission-filtered by the PROVIDER, never the renderer, so an
emptied group is a normal outcome of access control, not a bug — and
rendering it would open onto nothing. A parent with a real Area of its
own survives; it still has somewhere to go.
Renderers — all use their component library's own nesting, nothing hand-rolled
- Blazor: NodeMenuItemList.razor recurses via FluentMenuItem.MenuItems →
<fluent-menu slot="submenu">. Replaces the four verbatim-duplicated
@foreach blocks; the GitHub one had silently dropped icons entirely.
- portal-next: MenuEntries recurses via nested Fluent v9 Menu/MenuTrigger/
MenuPopover. Roles, aria-haspopup/aria-expanded and the keyboard model
(Enter/ArrowRight to open) come from the library — never hover-only.
- React Native: drill-down, not a flyout — a second panel beside the first
needs hover and width and a phone has neither. Tapping a parent replaces
the list with its children plus a back control; parents carry a ›
chevron and rows clear the 44pt touch target. Extracted to leftMenu.tsx
so it is testable without the Expo-bound connection module.
Parity across clients means equivalent CAPABILITY, not an identical
gesture; the mobile difference is deliberate and documented.
i18n: menu.exportGroup + menu.exportGroupTooltip in both catalogs.
Docs: NodeMenu.md promised an "expand-on-hover sub-menu" no renderer ever
implemented — corrected, with the per-client table.
Tests: NestedMenuTest (child ordering, empty-group pruning bottom-up,
navigable-parent survival), MarkdownExportMenuTest (grouping + ordering;
its emoji invariant now descends into children, which it never did),
headerMenus.test.tsx (9: submenu render, keyboard open, aria, ordering,
parent-not-activatable, depth), leftMenu.test.tsx (10: drill-down, back,
ordering, a11y, 44pt, depth).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements true hierarchical (nested) node-menu rendering across all clients, backed by server-side normalization (recursive child sorting + empty-group pruning) and updates the export/share entries to use an Export submenu. It also fixes “Connect Microsoft 365” / instance OAuth connect navigation by ensuring those server endpoints are reached via full browser loads and by centralizing the EA consent route definition.
Changes:
- Render
NodeMenuItemDefinition.Childrenas real nested sub-menus in Blazor and portal-next, and as a drill-down hierarchy in React Native; add shared parent classification (IsSubmenuParent/_groupsentinel). - Normalize menu trees server-side (sort children by
Orderat every depth; prune empty_groupparents bottom-up) and group export/share actions under 📦 Export. - Fix connect flows by centralizing EA consent route constants + adding a routability test, and using
forceLoad: truefor server endpoints.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Security.Test/NestedMenuTest.cs | Adds integration coverage for server-side nested menu normalization (sorting + pruning). |
| test/MeshWeaver.Security.Test/MarkdownExportMenuTest.cs | Updates tests to validate export grouping under a parent and to assert icon invariants across nested entries. |
| test/Memex.Portal.Shared.Test/EaConnectRouteTest.cs | Pins EA consent route constants to real ASP.NET routing + verifies Blazor catch-all doesn’t swallow the endpoint. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.en.json | Adds new menu/export keys and updates send-document dialog strings (EN). |
| src/MeshWeaver.Messaging.Hub/Localization/strings.de.json | Adds new menu/export keys and updates send-document dialog strings (DE). |
| src/MeshWeaver.Mesh.Contract/NodeMenuItemDefinition.cs | Introduces _group sentinel + clarifies submenu-parent semantics via HasChildren/IsSubmenuParent. |
| src/MeshWeaver.Mesh.Contract/IEmailSender.cs | Adds ConnectAsUserHref to advertise deployment-specific connect flows to UI. |
| src/MeshWeaver.Mesh.Contract/HubEmailExtensions.cs | Exposes ConnectAsUserHref() helper for UI callers. |
| src/MeshWeaver.Markdown.Export/Layout/SendDocumentLayoutArea.cs | Refactors share-by-email dialog UX and adds just-in-time identity choice + connect navigation via forceLoad. |
| src/MeshWeaver.Markdown.Export/Layout/MarkdownExportMenuProvider.cs | Groups PDF/DOCX/Email under a single Export submenu parent and adds emoji icons + label/tooltip keys. |
| src/MeshWeaver.Markdown.Export/Layout/DeckExportMenuProvider.cs | Aligns Deck export entries with Markdown export group + restores localization keys/icons for parity. |
| src/MeshWeaver.InstanceSync/InstanceSyncLayoutArea.cs | Fixes instance connect navigation to a server endpoint by using forceLoad: true. |
| src/MeshWeaver.Graph/Configuration/NodeMenuItemsExtensions.cs | Normalizes menu trees: recursive child sorting and empty-group pruning. |
| src/MeshWeaver.Graph/ApprovalExtensions.cs | Adds an emoji icon to Request Approval menu entry for consistency. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-12-the-node-menu-groups-related-actions-into-sub-menus.md | Adds release note for nested node menus + export grouping. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-12-sharing-a-document-by-email-is-quicker-to-fill-in.md | Adds release note for the streamlined share-by-email dialog. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-12-connect-microsoft-365-no-longer-leads-to-a-missing-page.md | Adds release note for fixed connect navigation + route centralization. |
| src/MeshWeaver.Documentation/Data/GUI/NodeMenu.md | Updates docs to match new submenu behavior and server normalization rules. |
| src/MeshWeaver.Blazor.Portal/Layout/PortalLayoutBase.razor.cs | Removes flattening helper and documents why it was incorrect. |
| src/MeshWeaver.Blazor.Portal/Layout/PortalLayoutBase.razor | Replaces duplicated menu rendering loops with recursive NodeMenuItemList. |
| src/MeshWeaver.Blazor.Portal/Layout/NodeMenuItemList.razor | New recursive Blazor component rendering nested submenus via Fluent/FAST. |
| src/MeshWeaver.Blazor.Portal/Layout/NodeMenuItemLabel.razor | New shared icon+label renderer to keep behavior consistent across menu contexts. |
| memex/Memex.Portal.Shared/Email/GraphEmailSender.cs | Implements ConnectAsUserHref only when delegated Graph auth is configured. |
| memex/Memex.Portal.Shared/Email/ExecutiveAssistantPlugin.cs | Uses EA controller constants to build the absolute consent link reliably. |
| memex/Memex.Portal.Shared/Authentication/EaConsentController.cs | Centralizes route constants and builds attributes/paths from them. |
| clients/react/src/i18n/strings.en.json | Mirrors new menu/export and send-document keys into the React catalog (EN). |
| clients/react/src/i18n/strings.de.json | Mirrors new menu/export and send-document keys into the React catalog (DE). |
| clients/react-native/src/Shell.tsx | Extracts left menu into a separate testable component module. |
| clients/react-native/src/leftMenu.tsx | New drill-down nested menu implementation for mobile + menu item typing incl. children. |
| clients/react-native/src/leftMenu.test.tsx | Pins drill-down semantics, ordering, and a11y/touch-target behavior for nested menus. |
| clients/react-native/docs/shell.md | Documents the drill-down nested menu model and how it maps to $Menu:* streams. |
| clients/portal-next/test/shell.test.ts | Updates tests from flattening behavior to submenu-parent classification. |
| clients/portal-next/test/headerMenus.test.tsx | New DOM/a11y/keyboard tests for Fluent v9 nested submenus. |
| clients/portal-next/src/client/HeaderMenus.tsx | Renders nested submenus recursively using Fluent v9’s native submenu pattern. |
Suppressed comments (1)
clients/react-native/src/leftMenu.tsx:148
- The Home entry label is hard-coded ("⌂ Home") instead of localized (common.home exists). This will show English UI in German.
<NavRow label="⌂ Home" active={!clientScreen && nav.address === home.address} onPress={() => onNavigate(home)} />
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+132
to
+136
| <NavRow | ||
| label="‹ Back" | ||
| active={false} | ||
| onPress={() => setDrill((d) => d.slice(0, -1))} | ||
| accessibilityLabel={`Back to ${drill.length > 1 ? (drill[drill.length - 2].label ?? "menu") : "menu"}`} |
Comment on lines
141
to
+145
| { | ||
| Label = host.Localize("ui.sendDocument.recipient"), | ||
| Placeholder = host.Localize("ui.sendDocument.searchUsers"), | ||
| Placeholder = "name@example.com", | ||
| DataContext = dataContext | ||
| }.WithQueries("nodeType:User").WithMaxResults(15).WithStyle("width: 100%; margin-bottom: 12px;")); | ||
| }.WithStyle("width: 100%; margin-bottom: 4px;")); |
Comment on lines
+219
to
222
| {items.length === 0 && ( | ||
| <div style={{ padding: "6px 12px", fontSize: "0.8rem", color: "var(--colorNeutralForeground3)" }}> | ||
| No actions available | ||
| </div> |
Contributor
Contributor
Contributor
Test Results (shard 1)1 278 tests - 11 1 278 ✅ - 11 6m 58s ⏱️ + 1m 1s Results for commit b5d44e4. ± Comparison against base commit ac72779. This pull request removes 158 and adds 147 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Contributor
Test Results (shard 2)2 115 tests +23 2 114 ✅ +23 6m 47s ⏱️ - 1m 47s Results for commit b5d44e4. ± Comparison against base commit ac72779. This pull request removes 147 and adds 170 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
…b1b26443237 # Conflicts: # clients/react/src/i18n/strings.de.json # clients/react/src/i18n/strings.en.json # src/MeshWeaver.Documentation/Data/GUI/NodeMenu.md # src/MeshWeaver.Markdown.Export/Layout/DeckExportMenuProvider.cs # src/MeshWeaver.Markdown.Export/Layout/MarkdownExportMenuProvider.cs # src/MeshWeaver.Messaging.Hub/Localization/strings.de.json # src/MeshWeaver.Messaging.Hub/Localization/strings.en.json
Sibling GUI pages use ../../Architecture/<page>; the two MenuAsData links were one level short and failed DocumentationLinkIntegrityTest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The node menu had grown to ~15 flat entries. "is there a possibility to create sub-menus? it gets very big otherwise"
What
Childrendid beforeNodeMenuItemDefinition.Childrenhas existed for a while and the contract already recursed through it (Localized,Equals). No web client rendered it:FlattenMenuItemsdeleted the parent and spliced children inline behind a dividerMenuItemhad nochildrenfield — the subtree was invisibleA live bug, not just a gap:
GitHubSyncMenuProvideremits one parent per sync source, so two repos gave two identicalSync now / Update to latest / Check branchtriplets with the disambiguating label + 🐙 + repo tooltip thrown away. The docs meanwhile promised "an expand-on-hover sub-menu" nothing implemented.Renderers now
Each uses its component library's own nesting — no bespoke dropdown, no HTML strings:
NodeMenuItemList.razorrecurses viaFluentMenuItem.MenuItems→<fluent-menu slot="submenu">. Collapses the four duplicated@foreachblocks into one component; the GitHub one had been dropping icons.MenuEntriesrecurses via nested Fluent v9Menu/MenuTrigger/MenuPopover.leftMenu.tsxso it is testable without the Expo-boundconnection.Roles,
aria-haspopup/aria-expandedand the keyboard model come from the libraries; a test asserts the submenu opens on ArrowRight — never hover-only.Mobile is deliberately different. A second panel beside its parent needs hover and width; a phone has neither. Tapping a parent replaces the list with its children plus a back control;
›chevron, 44 pt targets. Parity means equivalent capability, not an identical gesture.Grouping
📦 Export holds 📄 PDF (27) · 📤 Email (28) · 📝 DOCX (29). Decks use the same parent (PDF + Email). #1248's labels, icons, orders and tooltip keys are taken verbatim — both providers were reset to main's file and only re-wrapped.
Composing with #1252 (menu presentation as data)
#1252 landed in the same aggregation seam. A textual merge would have shipped two silent defects; all three points are covered by
MenuNestingCompositionTest:Normalizeran BEFORE the overlay. The overlay is itself a source of nesting (Parentmoves an item under another), so normalizing the pre-overlay slice sorted and pruned a tree that did not exist yet — a grouping created purely by data got neither Order-sorted children nor empty-group pruning. Normalize now runs immediately afterMenuPresentationOverlay.Apply(still beforeLocalized): one rule for both origins. A danglingParentstill leaves its entry top-level — it is not a group, so pruning cannot touch it._groupas one shared sentinel made every group the same catalog key.Areais the key the catalog matches on and the key a child names as itsParent, so an admin could not re-word / re-icon / re-order / hide a specific group, and only the first was addressable as a parent. Now a prefix:NodeMenuItemDefinition.GroupArea(name)→_group:Export.Applynow descends into provider-emittedChildrenfor patch/hide. Re-parenting stays top-level-only, soParentnesting remains exactly one level deep and a parent cycle stays structurally impossible.Also corrects
MenuPresentation's XML doc, which claimed the overlay creates a container for a parent no provider emits — it does not, and must not, since nothing here filters onRequiredPermission.MenuAsData's worked example used exactly that non-existent area (a pure no-op emitting two warnings); it now uses the real_group:Exportkey and demonstrates hiding a nested entry.Depth, grouping parents, empty groups
Parentstays one level by construction). Nothing ships deeper than two.hidden), so an emptied group is a normal outcome of access control. A parent with a realAreasurvives.Tests
MenuNestingCompositionTest(new, 9) — the composition above; all 14 of feat: menu presentation is editable data, not compiled code #1252's overlay tests pass unchangedNestedMenuTest(new, 4) — child ordering, empty-group pruning, bottom-up, navigable-parent survivalMarkdownExportMenuTest(+2) — grouping, submenu ordering, group addressable + tooltip key; the whole-menu emoji invariant now descends into children, which it never didheaderMenus.test.tsx(new, 9) ·leftMenu.test.tsx(new, 10) ·shell.test.tsretargeted from the destructive flatten toisSubmenuParenti18n:
menu.exportGroup+menu.exportGroup.tooltip(adopting #1248's.tooltipconvention) in both catalogs; drift guard green.Verified locally
dotnet build memex/Memex.Portal.Monolith -c Release -p:CIRun=true -warnaserrorclean · Graph.Test 23 · Security.Test 19 · DocumentationLinkIntegrity · LocalizationTest · portal-next 92 · react-native 153 · react i18n 15 · both JS typechecks. No lockfiles regenerated.🤖 Generated with Claude Code