Skip to content

ref(cmdk): Update new cmd+k's focus to be clearer#111381

Merged
rbro112 merged 1 commit intomasterfrom
ryan/update_new_cmd_k_s_focus_to_be_clearer
Mar 24, 2026
Merged

ref(cmdk): Update new cmd+k's focus to be clearer#111381
rbro112 merged 1 commit intomasterfrom
ryan/update_new_cmd_k_s_focus_to_be_clearer

Conversation

@rbro112
Copy link
Member

@rbro112 rbro112 commented Mar 24, 2026

Before (note the tiny weird focus state):
Screenshot 2026-03-23 at 4 55 35 PM

After
Screenshot 2026-03-23 at 4 55 01 PM

Copy link
Member Author

rbro112 commented Mar 24, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 24, 2026
@rbro112 rbro112 changed the title Update new cmd+k's focus to be clearer ref(cmdk): Update new cmd+k's focus to be clearer Mar 24, 2026
Comment on lines +202 to +204
li[data-focused] > ${InnerWrap} {
outline: 2px solid ${p => p.theme.tokens.focus.default};
}
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 new CSS rule targets li[data-focused], but this attribute is never set on the list items, so the focus outline will not appear.
Severity: MEDIUM

Suggested Fix

Modify the component that renders the <li> items to add the data-focused attribute when the isFocused state is true. For example: <li data-focused={isFocused} {...props}>. Alternatively, change the CSS selector to use a class or style that is already being applied based on the isFocused state.

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/list.tsx#L202-L204

Potential issue: A new CSS rule was added to display a focus outline on command palette
list items using the `li[data-focused]` selector. However, the `data-focused` attribute
is never actually set on the `<li>` DOM elements. The `useOption` hook from React Aria
provides an `isFocused` state, but this is not being used to apply the `data-focused`
attribute to the element. As a result, the CSS selector will never match, and the
intended focus outline will not be visible, negatively impacting keyboard navigation and
accessibility.

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

@rbro112 rbro112 merged commit f893223 into master Mar 24, 2026
75 checks passed
@rbro112 rbro112 deleted the ryan/update_new_cmd_k_s_focus_to_be_clearer branch March 24, 2026 00:24
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