fix(app): forward keyboard shortcuts from sandbox iframes#839
fix(app): forward keyboard shortcuts from sandbox iframes#83950rayn wants to merge 2 commits intohistoire-dev:mainfrom
Conversation
|
|
✅ Deploy Preview for histoire-examples-svelte3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-controls ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-examples-vue3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces keyboard shortcut forwarding from sandbox iframes to the main application, ensuring that shortcuts remain functional when the iframe is focused. The implementation refactors the keyboard utility to manage multiple tracked windows and integrates this into the story preview component. Review feedback identifies a potential security exception when interacting with cross-origin iframes and a race condition affecting modifier key states during window transitions, both of which require additional safeguards.
| export function trackWindow(target: Window): () => void { | ||
| if (trackedWindows.value.includes(target)) return () => {} | ||
| const cleanup = bindTracking(target) | ||
| trackedWindows.value = [...trackedWindows.value, target] | ||
| return () => { | ||
| cleanup() | ||
| trackedWindows.value = trackedWindows.value.filter(w => w !== target) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Accessing or adding event listeners to a cross-origin iframe's contentWindow will throw a security error (DOMException). Since Histoire sandboxes can be hosted on different origins (e.g., to prevent CSS/JS leakage), we should wrap the tracking initialization in a try-catch block to prevent the application from crashing when a cross-origin iframe is loaded.
export function trackWindow(target: Window): () => void {
if (trackedWindows.value.includes(target)) return () => {}
try {
const cleanup = bindTracking(target)
trackedWindows.value = [...trackedWindows.value, target]
return () => {
cleanup()
trackedWindows.value = trackedWindows.value.filter(w => w !== target)
}
} catch (e) {
return () => {}
}
}| export function onKeyboardShortcut(shortcut: KeyboardShortcut | Ref<KeyboardShortcut>, handler: KeyboardHandler, options: KeyboardShortcutOptions = {}) { | ||
| useEventListener(trackedWindows, options.event ?? 'keydown', (event) => { | ||
| if (isMatchingShortcut(isRef(shortcut) ? shortcut.value : shortcut)) { | ||
| handler(event) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
The global modifiers state is cleared whenever a tracked window blurs (see onBlur at line 48). This creates a race condition: if a user holds a modifier (like Cmd) and clicks into the iframe, the main window blurs, clearing the state before the shortcut key is pressed in the iframe. To fix this, we should synchronize the modifiers state with the current event's modifier properties (ctrlKey, metaKey, etc.) before checking the shortcut.
export function onKeyboardShortcut(shortcut: KeyboardShortcut | Ref<KeyboardShortcut>, handler: KeyboardHandler, options: KeyboardShortcutOptions = {}) {
useEventListener(trackedWindows, options.event ?? 'keydown', (event: KeyboardEvent) => {
modifiers.ctrl.pressed = event.ctrlKey
modifiers.alt.pressed = event.altKey
modifiers.shift.pressed = event.shiftKey
modifiers.meta.pressed = event.metaKey
if (isMatchingShortcut(isRef(shortcut) ? shortcut.value : shortcut)) {
handler(event)
}
})
}
histoire
@histoire/app
@histoire/controls
@histoire/plugin-nuxt
@histoire/plugin-percy
@histoire/plugin-screenshot
@histoire/plugin-svelte
@histoire/plugin-vue
@histoire/shared
@histoire/vendors
commit: |
1 participant
Why I started this
Every time i focus a story preview the global shortcuts (search, dark-mode toggle, etc.) silently die. On Safari
meta+shift+deven gets stolen by "Add to Reading List". I diagnosed it in #350 back in 2023 — the parent's key listeners only see events fired on the parent window, so anything fired while the iframe has focus is lost.What changed
keyboard.tsexports atrackWindow(target)so additional windows can opt into the same shortcut trackingcontentWindowafter load, cleans up on URL change / unmountevent.preventDefault()now runs inside the iframe, blocking Safari's reading-list grabCloses #350.