From 2d09cd44beaec5a4cb73b3d2f7703341b1e38aba Mon Sep 17 00:00:00 2001 From: u8array Date: Wed, 7 Oct 2026 11:51:55 +0200 Subject: [PATCH] fix(menu): the menu keeps working after a rebuild Tauri keys click channels by menu id, so closing the superseded tree dropped the new tree's channels. Fixes #532 --- src/hooks/useNativeMenu.rebuild.test.tsx | 201 +++++++++++++++++++++++ src/hooks/useNativeMenu.ts | 9 +- 2 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 src/hooks/useNativeMenu.rebuild.test.tsx diff --git a/src/hooks/useNativeMenu.rebuild.test.tsx b/src/hooks/useNativeMenu.rebuild.test.tsx new file mode 100644 index 00000000..f05a30c4 --- /dev/null +++ b/src/hooks/useNativeMenu.rebuild.test.tsx @@ -0,0 +1,201 @@ +// @vitest-environment jsdom +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { buildMenuModel, type MenuFlags, type HistorySubmenu, type SubmenuLabels } from "../lib/menuModel"; +import { fallbackTranslations as en } from "../locales"; +import type { MenuHandlers, useNativeMenu as nativeMenuHook } from "./useNativeMenu"; + +interface MockItem { + id: string; + text?: string; + items?: MockItem[]; + close: () => Promise; + setAsAppMenu: () => Promise; +} + +/** Mirrors tauri 2.12.0: `menu/plugin.rs` keys click channels by menu id, the `Drop` in + * `menu/mod.rs` removes one by that id, and `muda` hands out a counter value when no id is given. */ +const tauri = vi.hoisted(() => ({ + channels: new Map void>(), + installed: [] as Record[], + closed: [] as string[], + seq: 0, +})); + +vi.mock("../lib/platform", () => ({ isDesktopShell: true, isMacDesktop: false })); + +vi.mock("@tauri-apps/api/menu", () => { + const create = async (opts: Record = {}) => { + const id = typeof opts.id === "string" ? opts.id : `auto-${tauri.seq++}`; + const action = opts.action as ((id: string) => void) | undefined; + if (action) tauri.channels.set(id, action); + const item = { + ...opts, + id, + close: async () => { + tauri.closed.push(id); + tauri.channels.delete(id); + }, + setText: async () => undefined, + setEnabled: async () => undefined, + setChecked: async () => undefined, + insert: async () => undefined, + removeAt: async () => undefined, + setAsAppMenu: async () => { + tauri.installed.push(item); + }, + }; + return item; + }; + const kind = { new: create }; + return { Menu: kind, Submenu: kind, MenuItem: kind, IconMenuItem: kind, PredefinedMenuItem: kind, CheckMenuItem: kind }; +}); + +const BASE_FLAGS: MenuFlags = { + hasObjects: true, + documentEmits: true, + sourceEditing: false, + canBatchExport: false, + batchRowCount: 0, + batchPrintCount: 0, + connectDataWizard: true, + canBatchPdf: false, + pdfCurrentPageOnly: false, + canUndo: false, + canRedo: false, + includeQuit: true, +}; +const BATCH_FLAGS: MenuFlags = { ...BASE_FLAGS, canBatchExport: true, batchRowCount: 3, batchPrintCount: 3 }; + +const LABELS: SubmenuLabels = { file: "File", edit: "Edit", help: "Help", quit: "Quit" }; +const HISTORY: HistorySubmenu = { + label: "History", + clearLabel: "Clear history", + canClear: true, + items: [ + { index: 0, label: "Add text", current: false, enabled: true }, + { index: 1, label: "Move text", current: true, enabled: true }, + ], +}; + +const model = (flags: MenuFlags) => buildMenuModel(en, flags); + +const spyHandlers = (): MenuHandlers => { + const out: Partial = {}; + const full = model(BATCH_FLAGS); + for (const item of [...full.file.flat(), ...full.edit.flat(), ...full.help.flat()]) out[item.id] = vi.fn(); + return out as MenuHandlers; +}; + +const liveItems = (): MockItem[] => { + const walk = (items: MockItem[]): MockItem[] => items.flatMap((i) => [i, ...walk(i.items ?? [])]); + return walk(((tauri.installed.at(-1) as MockItem | undefined)?.items ?? [])); +}; + +/** Fires the click the way Rust does, with the item's own id as the payload. */ +const click = (text: string) => { + const item = liveItems().find((i) => i.text === text); + expect(item, `item "${text}" on the menubar`).toBeTruthy(); + const channel = tauri.channels.get((item as MockItem).id); + expect(channel, `click channel of "${text}"`).toBeTruthy(); + (channel as (id: string) => void)((item as MockItem).id); +}; + +let useNativeMenu: typeof nativeMenuHook; + +beforeEach(async () => { + tauri.channels.clear(); + tauri.installed.length = 0; + tauri.closed.length = 0; + tauri.seq = 0; + // The hook keeps the installed tree in module state, so every case starts with no menu. + vi.resetModules(); + ({ useNativeMenu } = await import("./useNativeMenu")); +}); + +/** The swap closes the superseded tree without awaiting it, so a click before that is no proof. */ +const swapped = (trees: number) => + waitFor(() => { + expect(tauri.installed).toHaveLength(trees); + expect(tauri.closed.length).toBeGreaterThan(0); + }); + +interface Rebuilt { + handlers: MenuHandlers; + onHistoryJump: (index: number) => void; + onHistoryClear: () => void; + onInitError: () => void; +} + +async function rebuild(next: { flags?: MenuFlags; dark?: boolean }): Promise { + const handlers = spyHandlers(); + const onHistoryJump = vi.fn(); + const onHistoryClear = vi.fn(); + const onInitError = vi.fn(); + const { rerender } = renderHook( + ({ flags }: { flags: MenuFlags }) => + useNativeMenu(model(flags), LABELS, handlers, {}, HISTORY, onHistoryJump, onHistoryClear, onInitError), + { initialProps: { flags: BASE_FLAGS } }, + ); + await waitFor(() => expect(tauri.installed).toHaveLength(1)); + if (next.dark !== undefined) act(() => setDark(next.dark as boolean)); + rerender({ flags: next.flags ?? BASE_FLAGS }); + await swapped(2); + expect(onInitError).not.toHaveBeenCalled(); + return { handlers, onHistoryJump, onHistoryClear, onInitError }; +} + +/** The jsdom stub in the test setup never changes, so the theme needs its own. */ +let listeners: (() => void)[] = []; +let darkScheme = false; +function setDark(dark: boolean) { + darkScheme = dark; + for (const fire of listeners) fire(); +} +beforeEach(() => { + listeners = []; + darkScheme = false; + // The hook reads `matches` off the object it subscribed with, so the flag has to be live on it. + window.matchMedia = ((query: string) => ({ + get matches() { + return query.includes("dark") && darkScheme; + }, + addEventListener: (_: string, fire: () => void) => listeners.push(fire), + removeEventListener: () => undefined, + })) as unknown as typeof window.matchMedia; +}); + +describe("useNativeMenu after a rebuild", () => { + it("keeps every command of the new tree clickable when batch entries change the structure", async () => { + const { handlers } = await rebuild({ flags: BATCH_FLAGS }); + + click(en.app.newDesign); + click(en.app.sendToZebraBatchFmt.replace("{n}", "3")); + click(en.app.exportBatchZplFmt.replace("{n}", "3")); + + expect(handlers.new).toHaveBeenCalledOnce(); + expect(handlers.sendToZebra).toHaveBeenCalledOnce(); + expect(handlers.exportBatch).toHaveBeenCalledOnce(); + }); + + it("keeps the history submenu clickable too, the steps and the clear entry", async () => { + const { onHistoryJump, onHistoryClear } = await rebuild({ flags: BATCH_FLAGS }); + + // The steps have no id of their own either since the fix, so they belong in this guard. + click("Move text"); + click("Clear history"); + + expect(onHistoryJump).toHaveBeenCalledWith(1); + expect(onHistoryClear).toHaveBeenCalledOnce(); + }); + + it("keeps the commands clickable when the OS theme flips", async () => { + const { handlers } = await rebuild({ dark: true }); + + click(en.app.newDesign); + click(en.app.sendToZebra); + + expect(handlers.new).toHaveBeenCalledOnce(); + expect(handlers.sendToZebra).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/hooks/useNativeMenu.ts b/src/hooks/useNativeMenu.ts index 79f31f31..d5b34f19 100644 --- a/src/hooks/useNativeMenu.ts +++ b/src/hooks/useNativeMenu.ts @@ -93,8 +93,6 @@ let latest: MenuData | null = null; let installed: InstalledMenu | null = null; let updateGen = 0; let queue: Promise = Promise.resolve(); -/** Unique step ids: positional ids could collide with an item still closing. */ -let stepIdSeq = 0; /** The OS toggles a clicked CheckMenuItem's checkmark itself; after a jump we * re-assert every step's checkmark so that stray toggle can't linger. */ let checkmarkDirty = false; @@ -143,7 +141,6 @@ async function makeStepItem( ): Promise { const { CheckMenuItem } = await import('@tauri-apps/api/menu'); const item = await CheckMenuItem.new({ - id: `history-${stepIdSeq++}`, text: step.label, checked: step.current, enabled: step.enabled, @@ -171,11 +168,12 @@ async function rebuildMenu(structureKey: string, d: MenuData, gen: number): Prom const items = new Map(); const buildItem = async (item: { id: MenuItemId; label: string; enabled: boolean }) => { + // Nothing here names a menu item: Tauri keys click channels by menu id and drops one when that + // id closes, so a rebuild's old tree would take the new tree's channels. Its own ids are unique. const base = { - id: item.id, text: item.label, enabled: item.enabled, - action: (id: string) => latest?.handlers[id as MenuItemId]?.(), + action: () => latest?.handlers[item.id]?.(), }; const icon = await itemIcon(item.id, d.icons, d.dark); const handle = track(icon ? await IconMenuItem.new({ ...base, icon }) : await MenuItem.new(base)); @@ -199,7 +197,6 @@ async function rebuildMenu(structureKey: string, d: MenuData, gen: number): Prom steps.push(hstep); } const clear = track(await MenuItem.new({ - id: 'historyClear', text: d.history.clearLabel, enabled: d.history.canClear, action: () => latest?.onHistoryClear(),