Skip to content

fix(chat-sidebar): close workspace file picker on Escape from search input#266

Merged
mbektas merged 1 commit into
plmbr:mainfrom
pjdoland:fix/262-add-context-dialog-esc
May 16, 2026
Merged

fix(chat-sidebar): close workspace file picker on Escape from search input#266
mbektas merged 1 commit into
plmbr:mainfrom
pjdoland:fix/262-add-context-dialog-esc

Conversation

@pjdoland
Copy link
Copy Markdown
Collaborator

Summary

The "Add files as context" dialog (opened from the @ button in the chat sidebar) ignored the Escape key once the user clicked into the search field. The wrapper popover already handled Escape, but the search input's keydown listener called stopPropagation() for every key to keep the chat sidebar's keyboard shortcuts from firing while typing a filter. Escape was getting eaten as collateral, so the only way to dismiss the dialog was clicking the close icon.

Solution

In src/chat-sidebar.tsx, gate the input's stopPropagation() on event.key !== 'Escape'. Escape bubbles to the popover's existing handler at line 3907; everything else is still swallowed so the sidebar shortcuts don't fire during a search.

Testing

jlpm tsc --noEmit && jlpm lint:check && jlpm jest green. Manual: opened the picker, focused the search field, pressed Escape, dialog closes.

No new automated test. SidebarComponent isn't mounted by any existing jest test (it pulls in JupyterLab services, NBIAPI, and the workspace file fetch); standing up a harness for a 7-line keydown-delegation fix would be a multi-hour scaffold. A Playwright e2e would be the right home if regression coverage becomes interesting later.

Risks / follow-ups

Audited the two sibling popovers (src/components/claude-session-picker.tsx and src/components/notebook-generation-popover.tsx) for the same pattern. Neither has the bug: the session picker has no input to swallow keys, and the notebook-generation popover's textarea handler only intercepts Enter.

Fixes #262

…input

The wrapper popover already had an Escape handler, but the search input
inside it called event.stopPropagation() unconditionally so the chat
sidebar's keyboard shortcuts wouldn't fire while the user typed a
filter. Escape was getting eaten as collateral, so the dialog could
only be closed by clicking the X.

Let Escape bubble while still stopping every other key. The wrapper's
existing handler does the rest.

No new tests: the existing test surface doesn't mount SidebarComponent,
and the keydown delegation is trivial to inspect by hand. Verified
the same pattern in claude-session-picker and notebook-generation-popover
during review; neither has the bug because their popovers don't contain
keystroke-suppressing inputs.

Fixes plmbr#262
@mbektas mbektas merged commit 27e1c59 into plmbr:main May 16, 2026
3 of 4 checks passed
@pjdoland pjdoland deleted the fix/262-add-context-dialog-esc branch May 16, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add file as context dialog should be closable with Esc key

2 participants