Skip to content

Commit 8d9a7c9

Browse files
committed
perf(search): rank groups by relevance, fix broken memoization, cross-review cleanup
Cmd-K performance/DX pass on top of the palette redesign: - Fix a systemic stale-closure/broken-memoization bug: every row previously got a fresh inline onSelect closure per render while the memo comparators excluded onSelect from equality checks. Replaced with useItemDispatch, a stable Map-based dispatcher shared by a new createRowGroup factory that all groups are now built from (collapses ~15 hand-copied group shells into one). - Rank the typed catalog groups by their best-matching item's fuzzy score instead of a fixed group order, so a strong Docs hit outranks a weak Blocks hit — the biggest DX gap vs. Linear/kbar-style palettes. - Add a "+N more, refine your search" row when a group's cap trims real matches, instead of silently dropping them. - Fix a real animation bug: the dialog panel toggled visible/invisible with no transition class at all, popping in/out instantly while only the backdrop faded. Fixed with the CSS-spec visibility-transition trick plus a subtle scale, animating both directions without a JS deferred-unmount hack. - content-visibility:auto + contain-intrinsic-size on rows, deferring layout/paint for off-screen rows without full virtualization (which would require reimplementing cmdk's DOM-based keyboard nav). - Drop a stray regenerated docx.cjs sandbox-bundle diff that had leaked into the branch. - Cross-review cleanup (/simplify + /cleanup): merged a duplicate MemoizedCategoryItem into MemoizedIconItem, removed 3 useCallbacks with no observer, hoisted a duplicated FALLBACK_BG_COLOR constant, migrated arbitrary z-40/z-50 to the z-modal token, simplified a dead ternary and a 4-way copy-pasted branch in the recents resolver, and split an over-broad-dependency memo so switching keystrokes inside a scope doesn't re-filter the whole tools catalog. Added search-groups.test.tsx (dispatch correctness against a real cmdk root) and filterAndScore tests in utils.test.ts.
1 parent 9e262b0 commit 8d9a7c9

9 files changed

Lines changed: 960 additions & 653 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-items/command-items.tsx

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export const MemoizedCommandItem = memo(
9090
},
9191
(prev, next) =>
9292
prev.value === next.value &&
93+
prev.onSelect === next.onSelect &&
9394
prev.icon === next.icon &&
9495
prev.bgColor === next.bgColor &&
9596
prev.showColoredIcon === next.showColoredIcon &&
@@ -107,7 +108,7 @@ export const MemoizedActionItem = memo(
107108
query,
108109
}: {
109110
value: string
110-
onSelect: () => void
111+
onSelect: (value: string) => void
111112
icon: ComponentType<{ className?: string }>
112113
name: string
113114
shortcut?: string
@@ -129,6 +130,7 @@ export const MemoizedActionItem = memo(
129130
},
130131
(prev, next) =>
131132
prev.value === next.value &&
133+
prev.onSelect === next.onSelect &&
132134
prev.icon === next.icon &&
133135
prev.name === next.name &&
134136
prev.shortcut === next.shortcut &&
@@ -145,7 +147,7 @@ export const MemoizedWorkflowItem = memo(
145147
query,
146148
}: {
147149
value: string
148-
onSelect: () => void
150+
onSelect: (value: string) => void
149151
name: string
150152
folderPath?: string[]
151153
isCurrent?: boolean
@@ -180,6 +182,7 @@ export const MemoizedWorkflowItem = memo(
180182
},
181183
(prev, next) =>
182184
prev.value === next.value &&
185+
prev.onSelect === next.onSelect &&
183186
prev.name === next.name &&
184187
prev.isCurrent === next.isCurrent &&
185188
prev.query === next.query &&
@@ -197,7 +200,7 @@ export const MemoizedFileItem = memo(
197200
query,
198201
}: {
199202
value: string
200-
onSelect: () => void
203+
onSelect: (value: string) => void
201204
name: string
202205
folderPath?: string[]
203206
query?: string
@@ -230,6 +233,7 @@ export const MemoizedFileItem = memo(
230233
},
231234
(prev, next) =>
232235
prev.value === next.value &&
236+
prev.onSelect === next.onSelect &&
233237
prev.name === next.name &&
234238
prev.query === next.query &&
235239
(prev.folderPath === next.folderPath ||
@@ -245,7 +249,7 @@ export const MemoizedTaskItem = memo(
245249
query,
246250
}: {
247251
value: string
248-
onSelect: () => void
252+
onSelect: (value: string) => void
249253
name: string
250254
query?: string
251255
}) {
@@ -269,7 +273,7 @@ export const MemoizedWorkspaceItem = memo(
269273
query,
270274
}: {
271275
value: string
272-
onSelect: () => void
276+
onSelect: (value: string) => void
273277
name: string
274278
isCurrent?: boolean
275279
query?: string
@@ -287,6 +291,7 @@ export const MemoizedWorkspaceItem = memo(
287291
},
288292
(prev, next) =>
289293
prev.value === next.value &&
294+
prev.onSelect === next.onSelect &&
290295
prev.name === next.name &&
291296
prev.isCurrent === next.isCurrent &&
292297
prev.query === next.query
@@ -302,7 +307,7 @@ export const MemoizedPageItem = memo(
302307
query,
303308
}: {
304309
value: string
305-
onSelect: () => void
310+
onSelect: (value: string) => void
306311
icon: ComponentType<{ className?: string }>
307312
name: string
308313
shortcut?: string
@@ -324,75 +329,54 @@ export const MemoizedPageItem = memo(
324329
},
325330
(prev, next) =>
326331
prev.value === next.value &&
332+
prev.onSelect === next.onSelect &&
327333
prev.icon === next.icon &&
328334
prev.name === next.name &&
329335
prev.shortcut === next.shortcut &&
330336
prev.query === next.query
331337
)
332338

333-
export const MemoizedCategoryItem = memo(
334-
function CategoryItem({
335-
value,
336-
onSelect,
337-
icon: Icon,
338-
name,
339-
count,
340-
query,
341-
}: {
342-
value: string
343-
onSelect: () => void
344-
icon: ComponentType<{ className?: string }>
345-
name: string
346-
count: number
347-
query?: string
348-
}) {
349-
return (
350-
<Command.Item value={value} onSelect={onSelect} className={COMMAND_ITEM_CLASSNAME}>
351-
<Icon className='size-[16px] flex-shrink-0 text-[var(--text-icon)]' />
352-
<span className='truncate text-[var(--text-body)]'>
353-
<HighlightedText text={name} query={query} />
354-
</span>
355-
<span className='ml-auto flex flex-shrink-0 items-center gap-1.5 text-[var(--text-subtle)] text-small'>
356-
{count}
357-
<ChevronRight className='size-[14px]' />
358-
</span>
359-
</Command.Item>
360-
)
361-
},
362-
(prev, next) =>
363-
prev.value === next.value &&
364-
prev.icon === next.icon &&
365-
prev.name === next.name &&
366-
prev.count === next.count &&
367-
prev.query === next.query
368-
)
369-
339+
/**
340+
* Also used for the browse-category rows (`count` set) — those are otherwise
341+
* identical to a plain icon row, so a trailing count + chevron is a prop
342+
* rather than a forked component.
343+
*/
370344
export const MemoizedIconItem = memo(
371345
function IconItem({
372346
value,
373347
onSelect,
374348
name,
375349
icon: Icon,
376350
query,
351+
count,
377352
}: {
378353
value: string
379-
onSelect: () => void
354+
onSelect: (value: string) => void
380355
name: string
381356
icon: ComponentType<{ className?: string }>
382357
query?: string
358+
count?: number
383359
}) {
384360
return (
385361
<Command.Item value={value} onSelect={onSelect} className={COMMAND_ITEM_CLASSNAME}>
386362
<Icon className='size-[16px] flex-shrink-0 text-[var(--text-icon)]' />
387363
<span className='truncate text-[var(--text-body)]'>
388364
<HighlightedText text={name} query={query} />
389365
</span>
366+
{count !== undefined && (
367+
<span className='ml-auto flex flex-shrink-0 items-center gap-1.5 text-[var(--text-subtle)] text-small'>
368+
{count}
369+
<ChevronRight className='size-[14px]' />
370+
</span>
371+
)}
390372
</Command.Item>
391373
)
392374
},
393375
(prev, next) =>
394376
prev.value === next.value &&
377+
prev.onSelect === next.onSelect &&
395378
prev.name === next.name &&
396379
prev.icon === next.icon &&
397-
prev.query === next.query
380+
prev.query === next.query &&
381+
prev.count === next.count
398382
)

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-items/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
export {
22
HighlightedText,
33
MemoizedActionItem,
4-
MemoizedCategoryItem,
54
MemoizedCommandItem,
65
MemoizedFileItem,
76
MemoizedIconItem,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, type ReactNode } from 'react'
5+
import { Command } from 'cmdk'
6+
import { createRoot, type Root } from 'react-dom/client'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
import { BlocksGroup } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/search-groups/search-groups'
9+
import type { SearchBlockItem } from '@/stores/modals/search/types'
10+
11+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
12+
13+
function TestIcon() {
14+
return <svg data-testid='icon' />
15+
}
16+
17+
function block(id: string, name: string): SearchBlockItem {
18+
return { id, name, icon: TestIcon, bgColor: '#000', type: id }
19+
}
20+
21+
let container: HTMLDivElement
22+
let root: Root
23+
24+
function mount(ui: ReactNode) {
25+
act(() => {
26+
root.render(<Command shouldFilter={false}>{ui}</Command>)
27+
})
28+
}
29+
30+
function selectByText(text: string) {
31+
const el = Array.from(container.querySelectorAll('[cmdk-item]')).find((node) =>
32+
node.textContent?.includes(text)
33+
)
34+
if (!el) throw new Error(`row not found: ${text}`)
35+
act(() => {
36+
el.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }))
37+
})
38+
}
39+
40+
beforeEach(() => {
41+
container = document.createElement('div')
42+
document.body.appendChild(container)
43+
root = createRoot(container)
44+
})
45+
46+
afterEach(() => {
47+
act(() => root.unmount())
48+
container.remove()
49+
})
50+
51+
describe('BlocksGroup value-dispatch', () => {
52+
it('routes a click on a row to onSelect with the matching item', () => {
53+
const onSelect = vi.fn()
54+
const items = [block('a', 'Alpha'), block('b', 'Beta')]
55+
mount(<BlocksGroup items={items} onSelect={onSelect} />)
56+
57+
selectByText('Beta')
58+
59+
expect(onSelect).toHaveBeenCalledTimes(1)
60+
expect(onSelect).toHaveBeenCalledWith(items[1])
61+
})
62+
63+
it('resolves the CURRENT item after items is replaced with a new array (no stale Map lookups)', () => {
64+
const onSelect = vi.fn()
65+
const first = [block('a', 'Alpha'), block('b', 'Beta')]
66+
mount(<BlocksGroup items={first} onSelect={onSelect} />)
67+
68+
const second = [block('a', 'Alpha Renamed'), block('c', 'Gamma')]
69+
mount(<BlocksGroup items={second} onSelect={onSelect} />)
70+
71+
selectByText('Gamma')
72+
73+
expect(onSelect).toHaveBeenCalledTimes(1)
74+
expect(onSelect).toHaveBeenCalledWith(second[1])
75+
})
76+
77+
it('still dispatches correctly after onSelect identity changes between renders', () => {
78+
const first = vi.fn()
79+
const second = vi.fn()
80+
const items = [block('a', 'Alpha'), block('b', 'Beta')]
81+
mount(<BlocksGroup items={items} onSelect={first} />)
82+
mount(<BlocksGroup items={items} onSelect={second} />)
83+
84+
selectByText('Alpha')
85+
86+
expect(first).not.toHaveBeenCalled()
87+
expect(second).toHaveBeenCalledTimes(1)
88+
expect(second).toHaveBeenCalledWith(items[0])
89+
})
90+
})
91+
92+
describe('BlocksGroup truncation affordance', () => {
93+
it('renders nothing extra when nothing was truncated', () => {
94+
mount(<BlocksGroup items={[block('a', 'Alpha')]} onSelect={vi.fn()} truncatedCount={0} />)
95+
expect(container.textContent).not.toMatch(/more/i)
96+
})
97+
98+
it('surfaces a non-selectable "+N more" row when the cap trimmed real matches', () => {
99+
mount(<BlocksGroup items={[block('a', 'Alpha')]} onSelect={vi.fn()} truncatedCount={12} />)
100+
expect(container.textContent).toContain('+12 more')
101+
// Must not be a cmdk row — it should never be selectable via keyboard/click.
102+
const items = container.querySelectorAll('[cmdk-item]')
103+
expect(items).toHaveLength(1)
104+
})
105+
})

0 commit comments

Comments
 (0)