Skip to content

Commit 4e35d36

Browse files
JonasBaclaude
andcommitted
fix(cmdk): Omit empty groups and reset scroll on query change
Empty CMDKGroups (no registered children) were appearing as clickable action items in browse mode. Filter them out in flattenActions so only groups with children or executable actions are shown. Also reset the results list scroll position to the top whenever the search query changes. The actual scroll element is the inner virtualizer container inside ListBox, not the outer ResultsList wrapper, so expose a scrollContainerRef prop on ListBox and wire it up in CommandPalette's onChange handler. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4406dc2 commit 4e35d36

File tree

3 files changed

+36
-4
lines changed

3 files changed

+36
-4
lines changed

static/app/components/commandPalette/ui/commandPalette.spec.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,21 @@ describe('CommandPalette', () => {
577577
expect(screen.getAllByRole('option', {name: 'Action B'})).toHaveLength(1);
578578
});
579579

580+
it('a group with no children is omitted from the list', async () => {
581+
render(
582+
<CommandPaletteProvider>
583+
<CMDKGroup display={{label: 'Empty Group'}} />
584+
<CMDKAction display={{label: 'Real Action'}} onAction={jest.fn()} />
585+
<CommandPalette onAction={jest.fn()} />
586+
</CommandPaletteProvider>
587+
);
588+
589+
expect(
590+
await screen.findByRole('option', {name: 'Real Action'})
591+
).toBeInTheDocument();
592+
expect(screen.queryByRole('option', {name: 'Empty Group'})).not.toBeInTheDocument();
593+
});
594+
580595
it('direct CMDKAction registrations outside slots are not duplicated', async () => {
581596
render(
582597
<CommandPaletteProvider>

static/app/components/commandPalette/ui/commandPalette.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Fragment, useCallback, useLayoutEffect, useMemo} from 'react';
1+
import {Fragment, useCallback, useLayoutEffect, useMemo, useRef} from 'react';
22
import {preload} from 'react-dom';
33
import {useTheme} from '@emotion/react';
44
import styled from '@emotion/styled';
@@ -223,6 +223,8 @@ export function CommandPalette(props: CommandPaletteProps) {
223223
[actions, analytics, dispatch, props]
224224
);
225225

226+
const resultsListRef = useRef<HTMLDivElement>(null);
227+
226228
const debouncedQuery = useDebouncedValue(state.query, 300);
227229

228230
const isLoading = state.query.length > 0 && debouncedQuery !== state.query;
@@ -287,6 +289,9 @@ export function CommandPalette(props: CommandPaletteProps) {
287289
onChange: (e: React.ChangeEvent<HTMLInputElement>) => {
288290
dispatch({type: 'set query', query: e.target.value});
289291
treeState.selectionManager.setFocusedKey(null);
292+
if (resultsListRef.current) {
293+
resultsListRef.current.scrollTop = 0;
294+
}
290295
},
291296
onKeyDown: (e: React.KeyboardEvent<HTMLInputElement>) => {
292297
if (e.key === 'Backspace' && state.query.length === 0) {
@@ -364,6 +369,7 @@ export function CommandPalette(props: CommandPaletteProps) {
364369
overflow="auto"
365370
>
366371
<ListBox
372+
scrollContainerRef={resultsListRef}
367373
listState={treeState}
368374
keyDownHandler={() => true}
369375
overlayIsOpen
@@ -448,6 +454,11 @@ function flattenActions(
448454
const results: CMDKFlatItem[] = [];
449455
for (const node of nodes) {
450456
const isGroup = node.children.length > 0;
457+
// Skip groups that have no children and no executable action — they are
458+
// empty section headers (e.g. a CMDKGroup whose children didn't render).
459+
if (!isGroup && !('to' in node) && !('onAction' in node)) {
460+
continue;
461+
}
451462
results.push({...node, listItemType: isGroup ? 'section' : 'action'});
452463
if (isGroup) {
453464
for (const child of node.children) {

static/app/components/core/compactSelect/listBox/index.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ interface ListBoxProps<T extends ObjectLike>
8282
*/
8383
overlayIsOpen?: boolean;
8484
ref?: React.Ref<HTMLUListElement>;
85+
/**
86+
* Ref forwarded to the inner scroll container div (the element the virtualizer
87+
* uses as its scroll element). Useful for callers that need to reset scrollTop.
88+
*/
89+
scrollContainerRef?: React.Ref<HTMLDivElement>;
8590
/**
8691
* Whether the select has a search input field.
8792
*/
@@ -139,6 +144,7 @@ export function ListBox<T extends ObjectLike>({
139144
showDetails = true,
140145
onAction,
141146
virtualized,
147+
scrollContainerRef,
142148
className,
143149
...props
144150
}: ListBoxProps<T>) {
@@ -188,15 +194,15 @@ export function ListBox<T extends ObjectLike>({
188194
const virtualizer = useVirtualizedItems({listItems, virtualized, size});
189195

190196
const refs = useMemo(() => {
191-
const scrollContainerRef = (scrollContainer: HTMLDivElement | null) => {
197+
const overflowTracker = (scrollContainer: HTMLDivElement | null) => {
192198
if (hasEverOverflowed || listItems.length === 0 || !scrollContainer) {
193199
return;
194200
}
195201

196202
setHasEverOverflowed(scrollContainer.scrollHeight > scrollContainer.clientHeight);
197203
};
198-
return mergeRefs(scrollContainerRef, virtualizer.scrollElementRef);
199-
}, [hasEverOverflowed, virtualizer.scrollElementRef, listItems]);
204+
return mergeRefs(overflowTracker, virtualizer.scrollElementRef, scrollContainerRef);
205+
}, [hasEverOverflowed, virtualizer.scrollElementRef, listItems, scrollContainerRef]);
200206

201207
return (
202208
<Fragment>

0 commit comments

Comments
 (0)