feat: nested sub-menus in the node menu, on every client

clients/portal-next/src/client/HeaderMenus.tsx

@@ -7,8 +7,8 @@
 //     permission-filtered MenuControl into the $Menu:{Node|Mesh|AI} slots of the SAME layout-area
 //     stream the page renders — this component reads those slots off the current page's live
 //     AreaSource (the React twin of MenuStreamExtensions.GetMenu / IMenuItemsProvider).
-//   - Hierarchical items flatten inline with separators (FlattenMenuItems); the current node's
-//     name headlines the Node and Mesh menus.
+//   - Hierarchical items render as nested Fluent sub-menus (MenuEntries, the twin of Blazor's
+//     NodeMenuItemList); the current node's name headlines the Node and Mesh menus.
 //   - Item click: Href wins; the AI menu's "ai-new-thread" sentinel opens the chat side panel
 //     fresh; otherwise navigate /{currentPath}/{area}.
 //   - Settings gear: per-node /{path}/Settings; at the root, /GlobalSettings for platform admins
@@ -51,7 +51,35 @@ export interface MenuItemDef {
   children?: MenuItemDef[];
 }
 
-const SEPARATOR: MenuItemDef = { label: "", area: "_separator" };
+/** NodeMenuItemDefinition.SeparatorArea — a divider, never activatable. */
+export const SEPARATOR_AREA = "_separator";
+
+/**
+ * NodeMenuItemDefinition.GroupAreaPrefix — a pure GROUPING parent: it exists only to hold `children`
+ * and has nowhere of its own to go. Rendered as a submenu, never activated.
+ *
+ * A prefix, not one shared sentinel, because `area` is also the key the MenuPresentation catalog
+ * matches on — each group needs its own key to stay addressable (`_group:Export`).
+ */
+export const GROUP_AREA_PREFIX = "_group:";
+
+/** True when `area` marks a pure grouping parent. */
+export const isGroupArea = (area: string | undefined): boolean =>
+  area != null && area.startsWith(GROUP_AREA_PREFIX);
+
+/**
+ * A submenu parent. Any entry carrying children is one — its own area/href is deliberately ignored
+ * for activation, because Fluent v9 (like FAST on the Blazor side) makes a `MenuTrigger`-wrapped
+ * item toggle its submenu rather than invoke. Mirrors NodeMenuItemDefinition.IsSubmenuParent.
+ */
+export function isSubmenuParent(item: MenuItemDef): boolean {
+  return (item.children != null && item.children.length > 0) || isGroupArea(item.area);
+}
+
+/** Ascending by `order` — the wire's sort key at every depth. */
+function sortByOrder(items: MenuItemDef[]): MenuItemDef[] {
+  return [...items].sort((a, b) => (a.order ?? 0) - (b.order ?? 0));
+}
 
 /** The sentinel area of the AI menu's "New thread" item (PortalLayoutBase.AiNewThreadAction). */
 export const AI_NEW_THREAD_ACTION = "ai-new-thread";
@@ -80,19 +108,61 @@ function toMenuItem(raw: Json): MenuItemDef | null {
   };
 }
 
-/** PortalLayoutBase.FlattenMenuItems — parents with children inline as separator + children. */
-export function flattenMenuItems(items: MenuItemDef[]): MenuItemDef[] {
-  if (!items.some((i) => i.children && i.children.length > 0)) return items;
-  const result: MenuItemDef[] = [];
-  for (const item of items) {
-    if (item.children && item.children.length > 0) {
-      if (result.length > 0) result.push(SEPARATOR);
-      result.push(...item.children);
-    } else {
-      result.push(item);
-    }
-  }
-  return result;
+/**
+ * Renders one level of the menu, recursing into `children` as NATIVE Fluent v9 sub-menus.
+ *
+ * This replaces `flattenMenuItems`, which mirrored Blazor's `FlattenMenuItems`: it DELETED the parent
+ * and spliced its children inline behind a divider, throwing away the label/icon/tooltip that told
+ * them apart (with two GitHub sync sources you got two identical action triplets and no way to know
+ * which repo either belonged to).
+ *
+ * The nesting is the component library's own: a `<Menu>` whose `<MenuTrigger>` wraps a `<MenuItem>`
+ * inside the parent `<MenuList>` is Fluent v9's documented submenu shape, so roles, `aria-haspopup` /
+ * `aria-expanded`, and the keyboard model (Enter / ArrowRight to open, ArrowLeft / Escape to close)
+ * all come from Fluent. A parent gets NO onClick — it opens, it never navigates.
+ *
+ * Depth is unbounded — the function recurses into itself.
+ */
+export function MenuEntries({
+  items,
+  onItem,
+}: {
+  items: MenuItemDef[];
+  onItem: (item: MenuItemDef) => void;
+}) {
+  return (
+    <>
+      {items.map((item, i) => {
+        if (item.area === SEPARATOR_AREA) return <MenuDivider key={`sep-${i}`} />;
+        const key = `${item.area}-${item.label}-${i}`;
+        if (isSubmenuParent(item)) {
+          return (
+            <Menu key={key}>
+              <MenuTrigger disableButtonEnhancement>
+                <MenuItem title={item.tooltip ?? item.label}>
+                  <MenuItemIcon icon={item.icon} />
+                  {item.label}
+                </MenuItem>
+              </MenuTrigger>
+              <MenuPopover>
+                <MenuList>
+                  {/* Sorted here as well as on the server: `order` must mean the same thing at
+                      every depth, and the client is the last place that can guarantee it. */}
+                  <MenuEntries items={sortByOrder(item.children ?? [])} onItem={onItem} />
+                </MenuList>
+              </MenuPopover>
+            </Menu>
+          );
+        }
+        return (
+          <MenuItem key={key} title={item.tooltip ?? item.label} onClick={() => onItem(item)}>
+            <MenuItemIcon icon={item.icon} />
+            {item.label}
+          </MenuItem>
+        );
+      })}
+    </>
+  );
 }
 
 /** Read the $Menu:{context} MenuControl items off the current page's live area tree. */
@@ -124,7 +194,6 @@ function MenuButton({
   header?: string | null;
   onItem: (item: MenuItemDef) => void;
 }) {
-  const flat = flattenMenuItems(items);
   return (
     <Menu positioning="below-end">
       <MenuTrigger disableButtonEnhancement>
@@ -154,21 +223,12 @@ function MenuButton({
               <MenuDivider />
             </>
           )}
-          {flat.length === 0 && (
+          {items.length === 0 && (
             <div style={{ padding: "6px 12px", fontSize: "0.8rem", color: "var(--colorNeutralForeground3)" }}>
               No actions available
             </div>
           )}
-          {flat.map((item, i) =>
-            item.area === "_separator" ? (
-              <MenuDivider key={`sep-${i}`} />
-            ) : (
-              <MenuItem key={`${item.area}-${item.label}-${i}`} title={item.tooltip ?? item.label} onClick={() => onItem(item)}>
-                <MenuItemIcon icon={item.icon} />
-                {item.label}
-              </MenuItem>
-            ),
-          )}
+          <MenuEntries items={items} onItem={onItem} />
         </MenuList>
       </MenuPopover>
     </Menu>

clients/portal-next/test/headerMenus.test.tsx

@@ -0,0 +1,165 @@
+// @vitest-environment jsdom
+// Nested node-menu entries render as REAL Fluent sub-menus.
+//
+// The regression these pin: `flattenMenuItems` (the port of Blazor's FlattenMenuItems) used to
+// delete the parent and splice its children inline behind a divider. With two GitHub sync sources
+// configured that produced two identical "Sync now / Update to latest / Check branch" triplets and
+// no way to tell which repo either belonged to — the parent's label, icon and repo tooltip were the
+// only thing that distinguished them, and all three were thrown away.
+//
+// Also pinned here, because a submenu is only usable if these hold:
+//   - a grouping parent is NOT activatable (it opens; it never navigates)
+//   - the submenu is reachable by KEYBOARD, not hover only
+//   - Fluent's menu roles / aria-haspopup / aria-expanded are actually emitted
+//   - children come out in `order`, at depth as well as at the top level
+
+import { describe, expect, it, vi, afterEach } from "vitest";
+import { act, cleanup, fireEvent, render, screen, within } from "@testing-library/react";
+import { FluentProvider, Menu, MenuList, MenuPopover, MenuTrigger, webLightTheme, Button } from "@fluentui/react-components";
+import { MenuEntries, GROUP_AREA_PREFIX, type MenuItemDef } from "../src/client/HeaderMenus";
+
+afterEach(cleanup);
+
+const leaf = (label: string, order = 0): MenuItemDef => ({ label, area: label, order });
+
+/** The menu open, so its items are in the DOM — the header's MenuButton in miniature. */
+function OpenMenu({ items, onItem }: { items: MenuItemDef[]; onItem?: (i: MenuItemDef) => void }) {
+  return (
+    <FluentProvider theme={webLightTheme}>
+      <Menu open>
+        <MenuTrigger disableButtonEnhancement>
+          <Button>trigger</Button>
+        </MenuTrigger>
+        <MenuPopover>
+          <MenuList>
+            <MenuEntries items={items} onItem={onItem ?? (() => {})} />
+          </MenuList>
+        </MenuPopover>
+      </Menu>
+    </FluentProvider>
+  );
+}
+
+const parentOf = (name: RegExp) => screen.getByRole("menuitem", { name });
+
+const click = async (el: Element) => {
+  await act(async () => {
+    fireEvent.click(el);
+  });
+};
+
+const exportGroup: MenuItemDef = {
+  label: "Export",
+  area: `${GROUP_AREA_PREFIX}Export`,
+  icon: "📦",
+  tooltip: "Export or share this document",
+  order: 27,
+  children: [
+    { label: "Share as email", area: "SendDocument", icon: "📤", order: 29 },
+    { label: "Export to PDF", area: "ExportPdf", icon: "📄", order: 27 },
+    { label: "Export to DOCX", area: "ExportDocx", icon: "📝", order: 28 },
+  ],
+};
+
+describe("MenuEntries — nested items render as sub-menus", () => {
+  it("renders the PARENT, not its children spliced inline", () => {
+    render(<OpenMenu items={[leaf("Edit"), exportGroup]} />);
+
+    // The parent survives — this is the whole point.
+    expect(screen.getByText("Export")).toBeTruthy();
+    // Children stay behind the closed submenu until it is opened.
+    expect(screen.queryByText("Export to PDF")).toBeNull();
+  });
+
+  it("exposes the parent with the right role and aria-haspopup, collapsed", () => {
+    render(<OpenMenu items={[exportGroup]} />);
+
+    const parent = parentOf(/Export/);
+    expect(parent.getAttribute("aria-haspopup")).toBe("menu");
+    expect(parent.getAttribute("aria-expanded")).toBe("false");
+  });
+
+  it("opens the submenu from the KEYBOARD — hover is not required", async () => {
+    render(<OpenMenu items={[exportGroup]} />);
+
+    const parent = parentOf(/Export/);
+    await act(async () => {
+      parent.focus();
+      fireEvent.keyDown(parent, { key: "ArrowRight", code: "ArrowRight" });
+    });
+
+    expect(screen.getByText("Export to PDF")).toBeTruthy();
+    expect(parent.getAttribute("aria-expanded")).toBe("true");
+  });
+
+  it("orders children by `order`, not by the order they arrived in", async () => {
+    render(<OpenMenu items={[exportGroup]} />);
+
+    // Declared email(29) → pdf(27) → docx(28); must render pdf → docx → email.
+    await click(parentOf(/Export/));
+
+    const labels = screen
+      .getAllByRole("menuitem")
+      .map((el) => el.textContent ?? "")
+      .filter((t) => t.includes("Export to") || t.includes("as email"));
+    expect(labels.map((l) => l.replace(/[^\w ]/g, "").trim())).toEqual([
+      "Export to PDF",
+      "Export to DOCX",
+      "Share as email",
+    ]);
+  });
+
+  it("never activates a grouping parent — it opens, it does not navigate", async () => {
+    const onItem = vi.fn();
+    render(<OpenMenu items={[exportGroup]} onItem={onItem} />);
+
+    await click(parentOf(/Export/));
+
+    // The submenu opened…
+    expect(screen.getByText("Export to PDF")).toBeTruthy();
+    // …and nothing was navigated to.
+    expect(onItem).not.toHaveBeenCalled();
+  });
+
+  it("still activates a leaf", async () => {
+    const onItem = vi.fn();
+    render(<OpenMenu items={[leaf("Edit")]} onItem={onItem} />);
+
+    await click(parentOf(/Edit/));
+
+    expect(onItem).toHaveBeenCalledTimes(1);
+    expect(onItem.mock.calls[0][0].area).toBe("Edit");
+  });
+
+  it("activates a CHILD, passing the child's own definition", async () => {
+    const onItem = vi.fn();
+    render(<OpenMenu items={[exportGroup]} onItem={onItem} />);
+
+    await click(parentOf(/Export/));
+    await click(screen.getByText("Export to PDF"));
+
+    expect(onItem).toHaveBeenCalledTimes(1);
+    expect(onItem.mock.calls[0][0].area).toBe("ExportPdf");
+  });
+
+  it("nests to arbitrary depth", async () => {
+    const deep: MenuItemDef = {
+      label: "L1",
+      area: `${GROUP_AREA_PREFIX}L1`,
+      children: [{ label: "L2", area: `${GROUP_AREA_PREFIX}L2`, children: [leaf("L3")] }],
+    };
+    render(<OpenMenu items={[deep]} />);
+
+    await click(parentOf(/L1/));
+    await click(screen.getByText("L2"));
+
+    expect(screen.getByText("L3")).toBeTruthy();
+  });
+
+  it("renders a separator as a divider, never as an entry", () => {
+    render(<OpenMenu items={[leaf("Edit"), { label: "", area: "_separator" }, leaf("Delete")]} />);
+
+    const list = screen.getByRole("menu");
+    expect(within(list).getAllByRole("menuitem").map((e) => e.textContent)).toEqual(["Edit", "Delete"]);
+  });
+});

clients/portal-next/test/shell.test.ts

@@ -2,13 +2,13 @@
 //   - search relevance scoring (MeshSearch.ComputeRelevanceScore tiers, normalized per term)
 //   - path-proximity boost (PathProximity.ComputeBoost — MaxBoost/(1+segmentDistance))
 //   - the URL remainder → (area, id) split (ParseSidePanelRemainder / SplitAreaRemainder)
-//   - hierarchical menu flattening (PortalLayoutBase.FlattenMenuItems)
+//   - submenu-parent classification (NodeMenuItemDefinition.IsSubmenuParent)
 //   - avatar initials (UserProfile.GetInitials)
 
 import { describe, expect, it } from "vitest";
 import { proximityBoost, relevanceScore } from "../src/client/SearchBar";
 import { splitRemainder } from "../src/server/snapshot";
-import { flattenMenuItems, type MenuItemDef } from "../src/client/HeaderMenus";
+import { GROUP_AREA_PREFIX, isSubmenuParent, type MenuItemDef } from "../src/client/HeaderMenus";
 import { initialsOf } from "../src/client/UserProfileMenu";
 
 describe("relevanceScore (MeshSearch.ComputeRelevanceScore port)", () => {
@@ -86,23 +86,29 @@ describe("splitRemainder (the {area}/{id} URL split)", () => {
   });
 });
 
-describe("flattenMenuItems (PortalLayoutBase.FlattenMenuItems port)", () => {
+// The old tests here pinned `flattenMenuItems` — the port of Blazor's FlattenMenuItems, which
+// DELETED a parent and spliced its children inline behind a divider. That behaviour is gone: nested
+// items now render as real Fluent sub-menus (see headerMenus.test.tsx for the DOM assertions), so
+// what remains to pin here is which entries count as parents in the first place.
+describe("isSubmenuParent (NodeMenuItemDefinition.IsSubmenuParent port)", () => {
   const leaf = (label: string): MenuItemDef => ({ label, area: label });
 
-  it("returns flat lists unchanged", () => {
-    const items = [leaf("A"), leaf("B")];
-    expect(flattenMenuItems(items)).toBe(items);
+  it("treats a plain entry as activatable", () => {
+    expect(isSubmenuParent(leaf("A"))).toBe(false);
   });
 
-  it("inlines children behind a separator", () => {
-    const items: MenuItemDef[] = [leaf("A"), { label: "Parent", area: "P", children: [leaf("C1"), leaf("C2")] }];
-    const flat = flattenMenuItems(items);
-    expect(flat.map((i) => i.area)).toEqual(["A", "_separator", "C1", "C2"]);
+  it("treats any entry carrying children as a parent", () => {
+    expect(isSubmenuParent({ label: "Parent", area: "P", children: [leaf("C1")] })).toBe(true);
   });
 
-  it("emits no leading separator when the parent comes first", () => {
-    const items: MenuItemDef[] = [{ label: "Parent", area: "P", children: [leaf("C1")] }, leaf("A")];
-    expect(flattenMenuItems(items).map((i) => i.area)).toEqual(["C1", "A"]);
+  it("treats the _group sentinel as a parent even before children arrive", () => {
+    expect(isSubmenuParent({ label: "Export", area: `${GROUP_AREA_PREFIX}Export` })).toBe(true);
+  });
+
+  it("does not treat an empty children array as a parent", () => {
+    // The server prunes emptied groups, but a client must not render a submenu that opens onto
+    // nothing if one ever slips through.
+    expect(isSubmenuParent({ label: "Parent", area: "P", children: [] })).toBe(false);
   });
 });