Skip to content

ref(cmdk) update cmdk to use scraps input#111493

Merged
JonasBa merged 5 commits intomasterfrom
jb/ref/cmdk-input
Mar 25, 2026
Merged

ref(cmdk) update cmdk to use scraps input#111493
JonasBa merged 5 commits intomasterfrom
jb/ref/cmdk-input

Conversation

@JonasBa
Copy link
Member

@JonasBa JonasBa commented Mar 24, 2026

Updates search input to use scraps input, adds leading search icon, updates the placeholder text to hint at searching, removes the icon wrap and centers the icon and text inside the content.

CleanShot 2026-03-24 at 16 16 58 CleanShot 2026-03-24 at 16 16 49

Fixes DE-1029

@JonasBa JonasBa requested a review from rbro112 March 24, 2026 23:15
@linear-code
Copy link

linear-code bot commented Mar 24, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 24, 2026
) : (
<IconSearch size="sm" variant="muted" />
)}
</InputGroup.LeadingItems>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing disablePointerEvents on non-interactive search icon

Medium Severity

When selectedAction is null, InputGroup.LeadingItems contains the non-interactive IconSearch but doesn't set disablePointerEvents. Per the InputGroup documentation, this means clicking the search icon area won't focus the input underneath. The existing usage in compactSelect/control.tsx and every example in inputGroup.mdx always passes disablePointerEvents for non-interactive icons. The prop needs to be conditionally set — disabled when showing the icon, enabled when showing the interactive back button.

Fix in Cursor Fix in Web

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.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

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

Comment on lines +17 to +27
export const modalCss = (theme: Theme) => {
return css`
[role='document'] {
padding: 0;

background-color: ${theme.tokens.background.primary};
border-top-left-radius: calc(${theme.radius.lg} + 1px);
border-top-right-radius: calc(${theme.radius.lg} + 1px);
}
`;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: The Flex component is used with a render prop pattern, which it may not support. This could prevent the InputGroup from rendering.
Severity: CRITICAL

Suggested Fix

Refactor the code to nest the InputGroup directly inside the Flex component as a standard JSX child, instead of using a function-as-a-child (render prop) pattern. Remove the function wrapper {p => { ... }}.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/components/commandPalette/ui/modal.tsx#L14-L27

Potential issue: The `Flex` component from `@sentry/scraps/layout` is used with a render
prop pattern to render an `InputGroup`. This pattern is anomalous, as no other usage of
`Flex` or `InputGroup` in the codebase follows this pattern. While some components like
`Surface` support render props, there is no evidence that `Flex` does. If `Flex` does
not support this pattern, the child function will not be executed, and the `InputGroup`
containing the search input will fail to render. This would make the command palette
non-functional as users would be unable to type queries.

Did we get this right? 👍 / 👎 to inform future reviews.

@JonasBa JonasBa merged commit f5eb98c into master Mar 25, 2026
70 checks passed
@JonasBa JonasBa deleted the jb/ref/cmdk-input branch March 25, 2026 15:14
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.

2 participants