Skip to content

feat: add ⌘K / Ctrl+K command palette to admin UI (#1010) - #1094

Open
AYUshJaiswal454 wants to merge 1 commit into
HelpCode-ai:mainfrom
AYUshJaiswal454:feat/command-palette
Open

AYUshJaiswal454 wants to merge 1 commit into
HelpCode-ai:mainfrom
AYUshJaiswal454:feat/command-palette

Conversation

@AYUshJaiswal454

Copy link
Copy Markdown
Contributor
  • Extract shared navigation items and settings sections into packages/frontend/src/lib/navigation.ts
  • Import shared navigation definitions in app-sidebar.tsx, settings/layout.tsx, and command-palette.tsx
  • Implement CommandPalette component on @radix-ui/react-dialog with 5 grouped sections: Pages, Settings, Connectors, MCP servers, and Actions
  • Support case-insensitive matching across query words, labels, keywords, and descriptions with maximum 8 results per group
  • Cache connectors and MCP servers on first open and invalidate across organization switches
  • Enforce role-based access: admin-only settings only for ADMIN, hide "Add connector" for VIEWER, and hide SSO in Cloud
  • Implement full WAI-ARIA combobox accessibility (role="combobox", role="listbox", role="option", aria-controls, aria-activedescendant, aria-selected)
  • Add visible header trigger showing ⌘K on macOS and Ctrl K elsewhere
  • Add comprehensive Playwright e2e test suite covering keyboard shortcuts, navigation, permissions, and accessibility

Closes #1010

Summary

Changes

Type

  • Bug fix
  • New feature
  • Documentation
  • Refactoring
  • Performance improvement
  • Other (describe below)

Testing

  • Added/updated unit tests
  • Tested manually (describe steps below)
  • No tests needed (explain why)

Checklist

  • My code follows the existing code style
  • I have tested my changes locally
  • I have updated documentation (if applicable)
  • All existing tests pass (npm test)
  • This PR has a descriptive title

Related Issues

- Extract shared navigation items and settings sections into packages/frontend/src/lib/navigation.ts
- Import shared navigation definitions in app-sidebar.tsx, settings/layout.tsx, and command-palette.tsx
- Implement CommandPalette component on @radix-ui/react-dialog with 5 grouped sections: Pages, Settings, Connectors, MCP servers, and Actions
- Support case-insensitive matching across query words, labels, keywords, and descriptions with maximum 8 results per group
- Cache connectors and MCP servers on first open and invalidate across organization switches
- Enforce role-based access: admin-only settings only for ADMIN, hide "Add connector" for VIEWER, and hide SSO in Cloud
- Implement full WAI-ARIA combobox accessibility (role="combobox", role="listbox", role="option", aria-controls, aria-activedescendant, aria-selected)
- Add visible header trigger showing ⌘K on macOS and Ctrl K elsewhere
- Add comprehensive Playwright e2e test suite covering keyboard shortcuts, navigation, permissions, and accessibility

Closes HelpCode-ai#1010

@helpcodeai helpcodeai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, the shape is right and lib/navigation.ts is now the single source the sidebar and settings layout read from, which was the main thing I wanted. Role and Cloud filtering match the sidebar and there's no new dependency. Two things block it though.

  1. The Frontend e2e job didn't just time out, it's a real regression. command-palette.tsx:219 throws (orgs || []).filter is not a function and takes the whole page down into error.tsx. The groupedResults memo at :122 runs on every AppShell render even with the palette closed, and a lot of the existing specs (billing-awareness, card-trial, footer-links, enterprise-offer, first-success-offer...) stub unknown routes with json({}), so orgs ends up as an object. About 25 tests fail plus retries, hence the 15 min timeout. With Array.isArray(orgs) ? orgs : [] the full suite passes for me locally (181/181), but please also avoid building the result list at all while the palette is closed. Moving the list body into a child component rendered inside Dialog.Content would do that, and it fixes the query reset below for free.

  2. Focus doesn't come back after closing. I focused the filter input on /connectors, pressed Ctrl+K, then Esc, and document.activeElement is <body>. Same when it's opened from the header button. Radix's modal content sends focus to the Dialog.Trigger on close, and there isn't one here. Save document.activeElement when it opens and refocus it in onCloseAutoFocus (with preventDefault()). The issue asks for this explicitly, so a test for it would be good too.

Smaller things I'd like fixed in this PR as well:

The reset in handleOpenChange (:108-114) never runs on open, because the shortcut and the header button call setPaletteOpen directly. Close with Esc, reopen, and the old query is still there.

Every page renders its own <AppShell>, so the palette remounts on each navigation and refetches connectors and servers on the first open of every page. A small module-level cache keyed by organizationId would keep them for the session like the issue says. Workspace switches already do a full reload, so you don't need to worry about clearing it there.

On macOS :64 also grabs Ctrl+K, which is "delete to end of line" in Mac text fields. Please only use metaKey on Mac and ctrlKey elsewhere. Ignoring the combo when shift or alt is held wouldn't hurt either.

The adminOnly/selfHostedOnly check is now written twice, in settings/layout.tsx:46-47 and command-palette.tsx:141-145. Could you move it into a helper in navigation.ts that both call? Then the rules can't drift either, which was the whole point of the file. The palette also skips section level adminOnly, which a shared helper would cover.

Optional, but nice: the listbox currently holds plain divs for group headers and the loading row. Use role="group" with aria-labelledby for the groups and put the loading row outside the listbox. Use the connector/server types from lib/api instead of any[]. The SETTINGS_SECTIONS alias at navigation.ts:84 isn't used anywhere. The NAV import in app-sidebar.tsx:42 should go up with the other imports. And skip Enter while isComposing is true.

Last thing: please fill in the PR template, including the commands you ran and their results, plus the keyboard-only walkthrough and the screen reader check from the acceptance criteria.

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.

🎃 Add a ⌘K / Ctrl+K command palette to the admin UI

2 participants