Skip to content

Commit 174e878

Browse files
author
Connor Byrne
committed
fix: keep Ctrl/Cmd shortcuts from falling through to the browser
The modal gate returned before event.preventDefault(), handing a combo the app owns back to the browser whenever any dialog was open. Ctrl+S then opened the browser's save-page dialog instead of saving. Suppress the default for Ctrl/Cmd combos only. Bare keys still reach inputs inside the dialog, which is what the gate was added to protect. - Fixes FE-1188
1 parent b4fed31 commit 174e878

2 files changed

Lines changed: 28 additions & 0 deletions

File tree

browser_tests/tests/defaultKeybindings.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,28 @@ test.describe('Default Keybindings', { tag: '@keyboard' }, () => {
236236
await comfyPage.keyboard.press('Escape')
237237
})
238238

239+
test("'Ctrl+s' does not fall through to the browser while a dialog is open", async ({
240+
comfyPage
241+
}) => {
242+
await comfyPage.keyboard.press('Control+s')
243+
await expect(comfyPage.page.getByRole('dialog')).toBeVisible()
244+
245+
await comfyPage.page.evaluate(() => {
246+
window.TestCommand = false
247+
window.addEventListener('keydown', (event: KeyboardEvent) => {
248+
if (event.key === 's') window.TestCommand = event.defaultPrevented
249+
})
250+
})
251+
252+
await comfyPage.keyboard.press('Control+s')
253+
254+
await expect
255+
.poll(() => comfyPage.page.evaluate(() => window.TestCommand))
256+
.toBe(true)
257+
258+
await comfyPage.keyboard.press('Escape')
259+
})
260+
239261
test("'Ctrl+o' triggers open workflow", async ({ comfyPage }) => {
240262
// Ctrl+o calls app.ui.loadFile() which clicks a hidden file input.
241263
// Detect the file input click via an event listener.

src/platform/keybindings/keybindingService.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ export function useKeybindingService() {
5151
}
5252
}
5353
if (isModalOpen(dialogStore.dialogStack.length)) {
54+
// A combo the app owns must not fall through to the browser just
55+
// because a dialog is open. Only Ctrl/Cmd combos are suppressed —
56+
// bare keys still have to reach inputs inside the dialog.
57+
if (keyCombo.ctrl) {
58+
event.preventDefault()
59+
}
5460
return
5561
}
5662

0 commit comments

Comments
 (0)