Skip to content

[feature] the app answers a right-click, and remembers where you have been - #339

Merged
Lanznx merged 5 commits into
mainfrom
right-click-and-back
Sep 6, 2026
Merged

Lanznx merged 5 commits into
mainfrom
right-click-and-back

Conversation

@yui0303

@yui0303 yui0303 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #338.

The app did not answer the two things a hand reaches for without thinking. A right-click anywhere in the tree or the library did nothing at all, and there was no way back to where you just were — every navigation overwrote the last one.

What changed

A right-click works. ui/context-menu.tsx wraps Radix's ContextMenu using dropdown-menu.tsx's styling class for class, so the app has one menu look rather than two. Radix's context menu ships in the radix-ui package already depended on, so this adds nothing to package.json.

  • Folder rows offer Rename and Delete; the section headers and the empty space below the tree offer New folder.
  • Recording cards offer Open, Rename, Move to folder (a submenu listing the same targets as the existing move popover, current folder disabled and check-marked) and Delete.

In both places the menu calls the paths that were already there — the same useLibraryTree operations behind the hover icons, the same onRenameStart / onMove / onDelete props on a card. It is a second way in, never a second implementation. Two rules that had been written twice, cardCapabilities() and moveTargets(), are now shared between the toolbar and the menu.

Back and forward. There is no router here; screens change by writing appMode in the store, so a navigation used to overwrite the previous position rather than stack on it. nav/history.ts keeps a capped stack of Location values — home, a library selection, or a recording — bound to ⌘[ / ⌘← and ⌘] / ⌘→ and to the mouse's side buttons.

CommandPalette.activate() was already a complete "go to this location" switch, so it was extracted as navigateTo(location) and both the palette and traversal now go through it. One implementation of "go there" is the point; two would drift.

A shortcut registry. lib/shortcuts.ts gives every window-wide shortcut one shared listener, one platform-correct mod modifier, and — the property the whole thing stands on — one implementation of "not while the user is typing". A global ⌘[ that fires while the caret sits in a rename field navigates the window out from under a half-typed name. ⌘K moved onto it.

Two shortcuts deliberately stayed where they are, and the code says why: zoom (lib/zoom.ts) is installed before React mounts and runs in windows that never render an AppShell; ⌘F (ReplayTranscript) is scoped to one panel and must keep working while its own find field has focus.

Three things a reviewer should look at

1. Deleting a recording still does not ask. Tracing remove() in LibraryScreen through deleteHistoryEntry / deleteCloudRecording / deleteOrgRecording finds no confirmation anywhere; the only globalThis.confirm() calls in src/ are the two folder deletes in useLibraryTree. The card's hover trash icon has always deleted on one click, and the menu matches it rather than growing a prompt only one of the two entry points has.

That consistency is correct and the behaviour is not. A context menu opens under the cursor with Delete a single click away, which makes an already-unguarded destructive action markedly easier to hit by accident. Adding a confirmation belongs on both entry points at once, in its own change — flagging it here because this PR is what makes it urgent.

2. Keyboard invocation is reasoned, not executed. macOS has no context-menu key and WebKit does not synthesise contextmenu from Shift+F10, so openMenuFromKeyboard dispatches a real bubbling MouseEvent — the only thing Radix listens for. The test suite runs in plain Node with no jsdom and no component-testing path, so this was verified by reading Radix's source rather than by running it. It wants a manual check in the running app, along with onCloseAutoFocus={preventFocusRestore}: without that line Radix restores focus off a freshly mounted rename input, the input's blur handler commits the untouched draft, and Rename silently does nothing.

3. Not every navigation is in the stack yet. Only navigateTo call sites are recorded, so sidebar clicks, exitReplay and the history-list open are still invisible to back/forward. locationOf in CommandPalette.tsx shows the mapping; routing the rest through navigateTo is the obvious follow-up.

Two constraints in the traversal are worth reading rather than trusting: a running meeting owns the window and the store refuses mode changes while one is active, so a refused navigation must not enter the stack, and a deleted recording must be dropped rather than throw. refused and unavailable are kept distinct for exactly that reason — collapsing them to a boolean would let one active meeting drain the whole stack.

Not in this PR

The long tail the registry exists to make cheap: ⌘F promoted to app level, arrow-key movement through the tree, Enter to open, F2 to rename, ⌘N, Delete.

Checks

bunx tsc --noEmit clean · bunx vitest run 33 files / 373 tests green, 21 of them new (shortcuts.test.ts, nav/history.test.ts) · bun run build succeeds.

The menus themselves are untested: there are no existing tests for AppSidebar.tsx or LibraryCards.tsx and no component-test harness to add them to. Standing up jsdom and a testing library is its own change.

YJack0000 and others added 4 commits September 6, 2026 03:41
Rename and delete were reachable only by hovering a row and hitting one of
two 12px icons, and creating a folder only by hovering a section header to
reveal its +. Everything the tree can do was invisible until the pointer
happened to be in the right place, and a right-click — the first thing you
try in a tree — did nothing at all.

A folder row now offers Rename and Delete, and the section headers and the
empty space below the tree offer New folder. Both reuse the paths that were
already there: Rename enters the same inline edit the pencil opens, Delete
calls the same useLibraryTree operation, which keeps its confirm() and its
failure toast for shared folders. The menu is a second way in, not a second
implementation.

ui/context-menu.tsx wraps Radix's ContextMenu with dropdown-menu.tsx's
styling, class for class, so the app has one menu look rather than two.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
⌘K could take you anywhere, and nothing could take you back. Every screen
change went straight at the store, so there was no record of the trip and no
way to undo one: land on the wrong recording from the palette and the only
route home was to find where you were by hand.

Two pieces, because one is useless without the other:

lib/shortcuts.ts — one matcher, one document listener, one typing guard. Every
shortcut used to bring its own keydown listener and its own idea of what ⌘
means, which is how a global chord ends up firing into a half-typed folder
name. matchShortcut and the guard are pure and tested; ⌘K moves onto the hook
(opting out of the guard, since it has to close a palette whose own search
field holds focus). ⌘+/−/0 stays in zoom.ts — it installs before React mounts,
in windows that never render a shell — and ⌘F stays in ReplayTranscript, where
it is scoped to one panel.

lib/nav/ — CommandPalette.activate becomes navigateTo(location), the one
implementation of "go there", with a back/forward stack over the same
locations. Two things the stack has to get right:

  * A running meeting owns the window: openHome/openLibrary already no-op
    there, so navigateTo reads the store back afterwards instead of restating
    the guard, and records nothing when the app refused. A refusal is
    temporary, so a traversal stops and leaves the stack intact.
  * A recording can be deleted: loading it fails, the location is dropped from
    the stack, and the traversal keeps walking. Skipping without dropping would
    leave a dead entry for every future ⌘[ to trip over.

Bound to ⌘[ / ⌘← / ⌘] / ⌘→ and the mouse side buttons, from AppShell above its
focused branch so the keys don't die and revive with the tree.

lib/nav.ts moves to lib/nav/settings.ts: navigation is a directory now, and a
file and a folder of the same name beside each other is a trap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cse_014vT2NYKCDzLrGbDydA8ada

# Conflicts:
#	src/i18n/messages.ts
The sidebar tree got a context menu; the recordings did not, and that is
where people actually spend their time. Every action the card already has
was hidden behind icons that only appear on hover — invisible until you
happen to point at the right corner, and unreachable from a keyboard.

The menu calls the card's existing callbacks. Open, Rename, Move and
Delete are the same code paths the hover toolbar runs, so the two can
never disagree about what an action does. Two rules that decided what the
toolbar could offer (can this card be renamed, can it be re-filed) now
live in one helper each, because a menu that offered a rename the toolbar
knows is impossible is exactly the drift a second surface invites.

Delete deliberately does not confirm. Deleting a recording never has —
the trash icon goes straight through — and adding a prompt on only one of
the two ways in would make the same action behave differently depending
on how you reached it. This is the opposite of the sidebar's folder
delete, which confirms inside useLibraryTree and therefore needed the
menu to stay out of its way.

`preventFocusRestore` and `openMenuFromKeyboard` move out of AppSidebar
into ui/context-menu. Both encode a WebKit/Radix quirk that took a while
to find — Radix yanking focus off a freshly mounted rename input, and
macOS having no context-menu key for Shift+F10 to synthesise — and a
second copy of that reasoning would have drifted from the first. Radix's
submenu primitives join the same file, styled off dropdown-menu.tsx like
the rest of it, so Move can list the folders inline.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 5, 2026 •

Copy link
Copy Markdown

✅ SonarQube Quality Gate passed — pathorsAI_parley

0 open issues on this PR.

…head

SonarQube S7747 reads `for (const b of [...bindings])` as an unnecessary array
conversion, because for…of takes an iterable and a Set already is one. Here the
copy is the point rather than a convenience: a handler is allowed to mount or
unmount another shortcut's owner, and iterating the live Set would then dispatch
to a binding that registered during this very keystroke.

Hoisting it to a named local says that in the code rather than only in the
comment, and takes the spread out of the loop head the rule looks at.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Lanznx
Lanznx merged commit b29e055 into main Sep 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The sidebar ignores a right-click, and there is no way back

3 participants