Skip to content

ref(nav) promote search to primary nav#111430

Merged
JonasBa merged 2 commits intomasterfrom
jb/ref/nav-cmd-k
Mar 24, 2026
Merged

ref(nav) promote search to primary nav#111430
JonasBa merged 2 commits intomasterfrom
jb/ref/nav-cmd-k

Conversation

@JonasBa
Copy link
Member

@JonasBa JonasBa commented Mar 24, 2026

Promotes CMD+K action to primary nav when page-frame is enabled and replaces the help menu search when cmd+k supercharged flag is enabled

CleanShot 2026-03-24 at 11 12 18 CleanShot 2026-03-24 at 11 12 13

Pending feature flag release, but this fixes DE-722

@JonasBa JonasBa requested a review from a team as a code owner March 24, 2026 18:13
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 24, 2026

return (
<Fragment>
{hasPageFrame ? (
<PrimaryNavigation.Button
label={t('Search support, docs and more')}
Copy link
Member

Choose a reason for hiding this comment

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

Would adding a "(cmd+k)" prompt be good to add to the label so users can learn the shortkey?

Would need to accommodate windows too, and additionally would love if we could have a kbd component like https://ui.shadcn.com/docs/components/radix/kbd

This can always come later, just an idea though

Copy link
Member

Choose a reason for hiding this comment

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

Adding a kbd component has been on our list! Happy to tackle that in a follow-up

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we do actually have something somewhere already, I just cant remember the name of it rn

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

<MenuIcon>
<IconSearch />
</MenuIcon>
) : undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Dead hasPageFrame check inside !hasPageFrame branch

Low Severity

The leadingItems: hasPageFrame ? ... : undefined conditional on the search menu item is now inside the !hasPageFrame branch of the outer ternary (lines 52-70), so hasPageFrame is always false here. This means leadingItems is always undefined, making the ternary dead code. It looks like a leftover from before the refactor that moved the search item into the conditional — it can be simplified by just removing the leadingItems property entirely.

Fix in Cursor Fix in Web

The search item is only rendered in the !hasPageFrame branch, so the
hasPageFrame ternary on leadingItems was always false and always
produced undefined. Remove the dead code and the now-unused IconSearch
import.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code
Copy link

linear-code bot commented Mar 24, 2026

@JonasBa JonasBa enabled auto-merge (squash) March 24, 2026 18:38
@JonasBa JonasBa merged commit 790a1da into master Mar 24, 2026
71 checks passed
@JonasBa JonasBa deleted the jb/ref/nav-cmd-k branch March 24, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants