Improve the self-drawn menu bar usability

packages/app/package.json

@@ -31,6 +31,7 @@
     "typecheck:test": "tsc -p tsconfig.test.json --noEmit"
   },
   "dependencies": {
+    "@base-ui/react": "^1.7.0",
     "@codemirror/commands": "6.10.3",
     "@codemirror/lang-json": "6.0.2",
     "@codemirror/lang-markdown": "6.5.0",

packages/app/src/components/AppMenuBar.tsx

@@ -0,0 +1,121 @@
+import { Menu } from "@base-ui/react/menu";
+import { Menubar } from "@base-ui/react/menubar";
+import { ChevronRight } from "lucide-react";
+import type { ContextMenuEntry, ContextMenuItem, ContextMenuSubmenu } from "./ContextMenu";
+
+export type AppMenuConfig = {
+  id: string;
+  label: string;
+  /** Extra classes for this menu's trigger, e.g. app-name weight or per-menu color. */
+  triggerClassName?: string;
+};
+
+export type AppMenuBarProps = {
+  menus: AppMenuConfig[];
+  getEntries: (menuId: string) => ContextMenuEntry[];
+  triggerClassName?: string;
+};
+
+const positionerClassName = "z-[2147483647]";
+
+const popupClassName =
+  "min-w-[216px] max-w-[min(280px,calc(100vw-16px))] rounded-lg border border-(--border-default) bg-(--bg-primary) p-1 text-[13px] leading-5 font-[520] text-(--text-primary) shadow-[0_14px_36px_color-mix(in_srgb,var(--text-heading)_16%,transparent)] outline-none select-none";
+
+const itemClassName =
+  "flex h-7 w-full items-center justify-between gap-4 rounded-md border-0 bg-transparent px-2 text-left font-inherit text-(--text-primary) outline-none transition-[background-color,color,opacity] duration-150 ease-out cursor-pointer hover:bg-(--bg-hover) hover:text-(--text-heading) focus-visible:bg-(--bg-hover) focus-visible:text-(--text-heading) focus-visible:outline-none data-disabled:opacity-45 data-disabled:cursor-default data-disabled:pointer-events-none";
+
+const separatorClassName = "my-1 h-px bg-(--border-default)";
+
+const acceleratorClassName =
+  "shrink-0 text-[12px] font-[520] text-(--text-secondary)";
+
+const submenuTriggerClassName =
+  "flex h-7 w-full items-center justify-between gap-4 rounded-md border-0 bg-transparent px-2 text-left font-inherit text-(--text-primary) outline-none transition-[background-color,color,opacity] duration-150 ease-out cursor-pointer hover:bg-(--bg-hover) hover:text-(--text-heading) focus-visible:bg-(--bg-hover) focus-visible:text-(--text-heading) focus-visible:outline-none data-disabled:opacity-45 data-disabled:cursor-default data-disabled:pointer-events-none";
+
+function formatAccelerator(accelerator: string) {
+  return accelerator.replace(/CmdOrCtrl/gu, "Cmd/Ctrl");
+}
+
+export function AppMenuBar({ menus, getEntries, triggerClassName }: AppMenuBarProps) {
+  return (
+    <Menubar className="flex h-10 items-center gap-1">
+      {menus.map((menu) => (
+        <Menu.Root key={menu.id}>
+          <Menu.Trigger
+            className={`${triggerClassName ?? ""} ${menu.triggerClassName ?? ""}`.trim() || undefined}
+          >
+            {menu.label}
+          </Menu.Trigger>
+          <Menu.Portal>
+            <Menu.Positioner className={positionerClassName} side="bottom" align="start" sideOffset={0}>
+              <Menu.Popup className={popupClassName} aria-label={menu.label}>
+                <AppMenuEntries entries={getEntries(menu.id)} />
+              </Menu.Popup>
+            </Menu.Positioner>
+          </Menu.Portal>
+        </Menu.Root>
+      ))}
+    </Menubar>
+  );
+}
+
+function AppMenuEntries({ entries }: { entries: ContextMenuEntry[] }) {
+  return entries.map((entry, index) => {
+    if (entry.kind === "separator") {
+      return <div className={separatorClassName} key={`separator-${index}`} role="separator" />;
+    }
+
+    if (entry.kind === "submenu") {
+      return <AppSubmenu key={entry.id} entry={entry} />;
+    }
+
+    return <AppMenuItem key={entry.id} entry={entry} />;
+  });
+}
+
+function AppMenuItem({ entry }: { entry: ContextMenuItem }) {
+  const disabled = Boolean(entry.disabled);
+  const acceleratorLabel = entry.accelerator ? formatAccelerator(entry.accelerator) : null;
+  const accessibleLabel = acceleratorLabel ? `${entry.label} ${acceleratorLabel}` : entry.label;
+
+  return (
+    <Menu.Item
+      className={itemClassName}
+      disabled={disabled}
+      aria-label={accessibleLabel}
+      data-menu-item-id={entry.id}
+      onClick={() => {
+        if (disabled) return;
+        Promise.resolve(entry.onSelect?.()).catch(() => {});
+      }}
+    >
+      <span className="flex min-w-0 items-center gap-2">
+        {entry.icon ? <span className="shrink-0 text-(--text-secondary)">{entry.icon}</span> : null}
+        <span className="min-w-0 truncate">{entry.label}</span>
+      </span>
+      {entry.accelerator ? (
+        <span className={acceleratorClassName}>{acceleratorLabel}</span>
+      ) : null}
+    </Menu.Item>
+  );
+}
+
+function AppSubmenu({ entry }: { entry: ContextMenuSubmenu }) {
+  const disabled = Boolean(entry.disabled);
+
+  return (
+    <Menu.SubmenuRoot disabled={disabled}>
+      <Menu.SubmenuTrigger className={submenuTriggerClassName}>
+        <span className="min-w-0 truncate">{entry.label}</span>
+        <ChevronRight aria-hidden="true" className="shrink-0 text-(--text-secondary)" size={14} strokeWidth={2.25} />
+      </Menu.SubmenuTrigger>
+      <Menu.Portal>
+        <Menu.Positioner alignOffset={-4} className={positionerClassName} side="right" align="start" sideOffset={8}>
+          <Menu.Popup className={popupClassName}>
+            <AppMenuEntries entries={entry.entries} />
+          </Menu.Popup>
+        </Menu.Positioner>
+      </Menu.Portal>
+    </Menu.SubmenuRoot>
+  );
+}

packages/app/src/components/NativeTitleBar.test.tsx

@@ -638,85 +638,85 @@ describe("NativeTitleBar", () => {
       />
     );
 
-    fireEvent.click(screen.getByRole("button", { name: "Markra" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Markra" }));
 
     expect(screen.getByRole("menu", { name: "Markra" })).toBeInTheDocument();
     fireEvent.click(screen.getByRole("menuitem", { name: "About Markra" }));
     expect(showAbout).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "Markra" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Markra" }));
     fireEvent.click(screen.getByRole("menuitem", { name: "Settings... Ctrl+," }));
     expect(openSettings).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "Markra" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Markra" }));
     fireEvent.click(screen.getByRole("menuitem", { name: "Check for updates" }));
     expect(checkForUpdates).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "Markra" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Markra" }));
     fireEvent.click(screen.getByRole("menuitem", { name: "Quit Markra" }));
     expect(exitApp).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
 
     expect(screen.getByRole("menu", { name: "File" })).toBeInTheDocument();
     fireEvent.click(screen.getByRole("menuitem", { name: "New Ctrl+N" }));
     expect(openBlankEditorWindow).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
     fireEvent.click(screen.getByRole("menuitem", { name: "Open... Ctrl+O" }));
     expect(openMarkdown).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
     fireEvent.click(screen.getByRole("menuitem", { name: "Save Ctrl+S" }));
     expect(saveMarkdown).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
     const saveAsItem = screen.getByRole("menuitem", { name: "Save As... Ctrl+Shift+S" });
     expect(saveAsItem).not.toBeDisabled();
     fireEvent.click(saveAsItem);
     expect(saveMarkdownAs).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
     const syncItem = screen.getByRole("menuitem", { name: "Sync now Ctrl+Alt+R" });
     expect(syncItem).not.toBeDisabled();
     fireEvent.click(syncItem);
     expect(syncNow).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
     const exportMenuItem = screen.getByRole("menuitem", { name: "Export" });
     expect(exportMenuItem).not.toBeDisabled();
-    fireEvent.pointerEnter(exportMenuItem);
+    fireEvent.click(exportMenuItem);
     const exportPdfItem = screen.getByRole("menuitem", { name: "Export PDF Ctrl+Alt+P" });
     expect(exportPdfItem).not.toBeDisabled();
     fireEvent.click(exportPdfItem);
     expect(exportPdf).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "File" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
     const reopenedExportMenuItem = screen.getByRole("menuitem", { name: "Export" });
-    fireEvent.pointerEnter(reopenedExportMenuItem);
+    fireEvent.click(reopenedExportMenuItem);
     const exportMarkdownItem = screen.getByRole("menuitem", { name: "Export Markdown with attachments" });
     expect(exportMarkdownItem).not.toBeDisabled();
     fireEvent.click(exportMarkdownItem);
     expect(exportMarkdown).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "Edit" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Edit" }));
 
     expect(screen.getByRole("menu", { name: "Edit" })).toBeInTheDocument();
     expect(screen.getByRole("menuitem", { name: "Undo Ctrl+Z" })).toBeInTheDocument();
     expect(screen.getByRole("menuitem", { name: "Select All Ctrl+A" })).toBeInTheDocument();
 
-    fireEvent.click(screen.getByRole("button", { name: "Format" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Format" }));
 
     expect(screen.getByRole("menu", { name: "Format" })).toBeInTheDocument();
     const boldItem = screen.getByRole("menuitem", { name: "Bold Ctrl+B" });
     expect(boldItem).not.toBeDisabled();
     fireEvent.click(boldItem);
     expect(formatBold).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "Format" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "Format" }));
     expect(screen.getByRole("menuitem", { name: "Heading 1 Ctrl+Alt+1" })).toBeInTheDocument();
 
-    fireEvent.click(screen.getByRole("button", { name: "View" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "View" }));
 
     expect(screen.getByRole("menu", { name: "View" })).toBeInTheDocument();
     expect(screen.queryByRole("menuitem", { name: "Enter Full Screen" })).not.toBeInTheDocument();
@@ -728,23 +728,91 @@ describe("NativeTitleBar", () => {
     fireEvent.click(screen.getByRole("button", { name: "Maximize or restore window" }));
     expect(toggleWindowMaximized).toHaveBeenCalledTimes(2);
 
-    fireEvent.click(screen.getByRole("button", { name: "View" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "View" }));
     const aiCommandItem = screen.getByRole("menuitem", { name: "AI writing command Ctrl+Shift+J" });
     expect(aiCommandItem).not.toBeDisabled();
     fireEvent.click(aiCommandItem);
     expect(toggleAiCommand).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "View" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "View" }));
     const foldsItem = screen.getByRole("menuitem", { name: "Toggle all folds Ctrl+Alt+T" });
     expect(foldsItem).not.toBeDisabled();
     fireEvent.click(foldsItem);
     expect(toggleAllFolds).toHaveBeenCalledTimes(1);
 
-    fireEvent.click(screen.getByRole("button", { name: "View" }));
+    fireEvent.click(screen.getByRole("menuitem", { name: "View" }));
     fireEvent.click(screen.getByRole("menuitem", { name: "Toggle file list Ctrl+Shift+M" }));
     expect(toggleMarkdownFiles).toHaveBeenCalledTimes(1);
   });
 
+  it("marks disabled Windows app menu items with data-disabled styling hooks", () => {
+    const onOpenMarkdown = vi.fn();
+    render(
+      <NativeTitleBar
+        aiAgentOpen={false}
+        dirty={false}
+        documentName="Draft.md"
+        markdownFilesOpen={false}
+        platform="windows"
+        saveDisabled
+        theme="light"
+        onToggleAiAgent={() => {}}
+        onOpenBlankEditorWindow={vi.fn()}
+        onOpenMarkdown={onOpenMarkdown}
+        onSaveMarkdown={() => {}}
+        onToggleMarkdownFiles={() => {}}
+        onToggleTheme={() => {}}
+      />
+    );
+
+    fireEvent.click(screen.getByRole("menuitem", { name: "Markra" }));
+
+    const hiddenItem = screen.getByRole("menuitem", { name: "Hide Markra" });
+    expect(hiddenItem).toHaveAttribute("data-disabled");
+    expect(hiddenItem.className).toContain("data-disabled:opacity-45");
+    expect(hiddenItem.className).toContain("data-disabled:pointer-events-none");
+
+    fireEvent.click(screen.getByRole("menuitem", { name: "File" }));
+
+    const saveItem = screen.getByRole("menuitem", { name: "Save Ctrl+S" });
+    expect(saveItem).toHaveAttribute("data-disabled");
+    expect(saveItem.className).toContain("data-disabled:opacity-45");
+
+    const newItem = screen.getByRole("menuitem", { name: "New Ctrl+N" });
+    expect(newItem).not.toHaveAttribute("data-disabled");
+    expect(newItem.className).toContain("hover:bg-(--bg-hover)");
+  });
+
+  it("switches the open Windows app menu when hovering another menubar trigger", () => {
+    render(
+      <NativeTitleBar
+        aiAgentOpen={false}
+        dirty={false}
+        documentName="Draft.md"
+        markdownFilesOpen={false}
+        platform="windows"
+        theme="light"
+        onToggleAiAgent={() => {}}
+        onOpenBlankEditorWindow={() => {}}
+        onOpenMarkdown={() => {}}
+        onSaveMarkdown={() => {}}
+        onToggleMarkdownFiles={() => {}}
+        onToggleTheme={() => {}}
+      />
+    );
+
+    fireEvent.click(screen.getByRole("menuitem", { name: "Markra" }));
+    expect(screen.getByRole("menu", { name: "Markra" })).toBeInTheDocument();
+
+    fireEvent.mouseEnter(screen.getByRole("menuitem", { name: "File" }), {
+      clientX: 80,
+      clientY: 10
+    });
+
+    expect(screen.getByRole("menu", { name: "File" })).toBeInTheDocument();
+    expect(screen.queryByRole("menu", { name: "Markra" })).not.toBeInTheDocument();
+  });
+
   it("toggles the Windows window state from self-drawn titlebar double-click mouse downs", () => {
     const toggleWindowMaximized = vi.fn();
     const { container } = render(