Skip to content

Commit b9b2efd

Browse files
Waleed Latifwaleedlatif1
authored andcommitted
fix(sidebar): do not arm Enter without a visible target
Review round 1: gating the cursor's paint on keyboard mode left Enter still acting on the seeded first row while that row was unmarked. The search field takes focus on open, so Enter could switch workspace with nothing shown as the target — emcn's popover instead holds its selection at -1 and ignores Enter until keyboard nav begins. Enter now acts only once a cursor is on screen, and typing counts as keyboard intent so the common "filter, then Enter" flow lands on a visible top result. Every path to Enter therefore has a marked target.
1 parent 634f3d7 commit b9b2efd

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.test.tsx

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const { mockNavigateToSettings } = vi.hoisted(() => ({ mockNavigateToSettings: vi.fn() }))
99

10+
const onWorkspaceSwitch = vi.fn()
11+
1012
vi.mock('@tanstack/react-query', () => ({
1113
useQueryClient: () => ({ invalidateQueries: vi.fn(), setQueryData: vi.fn() }),
1214
}))
@@ -88,7 +90,7 @@ function render() {
8890
isCreatingWorkspace={false}
8991
isWorkspaceMenuOpen
9092
setIsWorkspaceMenuOpen={() => {}}
91-
onWorkspaceSwitch={() => {}}
93+
onWorkspaceSwitch={onWorkspaceSwitch}
9294
onCreateWorkspace={async () => {}}
9395
onRenameWorkspace={async () => {}}
9496
onDeleteWorkspace={async () => {}}
@@ -122,6 +124,17 @@ function isMarked(name: string): boolean {
122124
)
123125
}
124126

127+
/**
128+
* Types into a React-controlled input. Assigning `.value` directly is ignored: React
129+
* tracks the previous value on the node, so the change must go through the native
130+
* setter for its synthetic `onChange` to fire.
131+
*/
132+
function typeInto(input: HTMLInputElement, value: string) {
133+
const setValue = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set
134+
setValue?.call(input, value)
135+
input.dispatchEvent(new Event('input', { bubbles: true }))
136+
}
137+
125138
beforeEach(() => {
126139
vi.clearAllMocks()
127140
// jsdom implements neither; the component scrolls the active row into view.
@@ -134,6 +147,37 @@ afterEach(() => {
134147
})
135148

136149
describe('WorkspaceHeader workspace switcher highlight', () => {
150+
it('leaves Enter unarmed until a cursor is on screen', () => {
151+
render()
152+
153+
const search = document.querySelector('input[placeholder="Search workspaces..."]')
154+
act(() => {
155+
search?.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))
156+
})
157+
158+
// The search field is focused on open, so acting on the seeded row here would
159+
// switch workspace with nothing marked.
160+
expect(onWorkspaceSwitch).not.toHaveBeenCalled()
161+
})
162+
163+
it('arms Enter on the top result once the user types', () => {
164+
render()
165+
166+
const search = document.querySelector(
167+
'input[placeholder="Search workspaces..."]'
168+
) as HTMLInputElement | null
169+
act(() => {
170+
if (search) typeInto(search, 'Acme')
171+
})
172+
// Typing counts as keyboard intent, so the target is visible before Enter fires.
173+
expect(isMarked('Acme')).toBe(true)
174+
175+
act(() => {
176+
search?.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))
177+
})
178+
expect(onWorkspaceSwitch).toHaveBeenCalledWith(expect.objectContaining({ id: 'ws-acme' }))
179+
})
180+
137181
it('marks only the current workspace when the menu opens', () => {
138182
render()
139183

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,12 @@ function WorkspaceHeaderImpl({
540540
icon={Search}
541541
placeholder='Search workspaces...'
542542
value={workspaceSearch}
543-
onChange={(e) => setWorkspaceSearch(e.target.value)}
543+
onChange={(e) => {
544+
// Typing is keyboard intent, so the cursor appears on the top
545+
// result and Enter has a visible target.
546+
setIsKeyboardNav(true)
547+
setWorkspaceSearch(e.target.value)
548+
}}
544549
onKeyDown={(e) => {
545550
e.stopPropagation()
546551
if (e.nativeEvent.isComposing) return
@@ -557,6 +562,11 @@ function WorkspaceHeaderImpl({
557562
(activeIndex - 1 + filteredWorkspaces.length) % filteredWorkspaces.length
558563
setHighlightedId(filteredWorkspaces[next].id)
559564
} else if (e.key === 'Enter') {
565+
// Only armed once a cursor is actually on screen. The search
566+
// field is focused on open, so acting on the seeded row here
567+
// would switch workspace with nothing marked — emcn's popover
568+
// likewise holds its selection at -1 until keyboard nav starts.
569+
if (!isKeyboardNav) return
560570
e.preventDefault()
561571
const target = filteredWorkspaces[activeIndex]
562572
if (target) onWorkspaceSwitch(target)

0 commit comments

Comments
 (0)