[fix] iframe losing focus on pointer down (#1848)

This PR fixes a bug that would cause an interactive iframe (e.g. a
youtube video) to lose its editing state once clicked.

### Change Type

- [x] `patch` — Bug fix

### Test Plan

1. Create an interactive iframe.
2. Begin editing.
3. Click inside of the iframe
This commit is contained in:
Steve Ruiz 2023-09-09 10:41:06 +01:00 committed by GitHub
parent 48a1bb4d88
commit 20704ea417
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 115 additions and 66 deletions

View file

@ -22,13 +22,14 @@ import { ContainerProvider, useContainer } from './hooks/useContainer'
import { useCursor } from './hooks/useCursor' import { useCursor } from './hooks/useCursor'
import { useDPRMultiple } from './hooks/useDPRMultiple' import { useDPRMultiple } from './hooks/useDPRMultiple'
import { useDarkMode } from './hooks/useDarkMode' import { useDarkMode } from './hooks/useDarkMode'
import { EditorContext } from './hooks/useEditor' import { EditorContext, useEditor } from './hooks/useEditor'
import { import {
EditorComponentsProvider, EditorComponentsProvider,
TLEditorComponents, TLEditorComponents,
useEditorComponents, useEditorComponents,
} from './hooks/useEditorComponents' } from './hooks/useEditorComponents'
import { useEvent } from './hooks/useEvent' import { useEvent } from './hooks/useEvent'
import { useFocusEvents } from './hooks/useFocusEvents'
import { useForceUpdate } from './hooks/useForceUpdate' import { useForceUpdate } from './hooks/useForceUpdate'
import { useLocalStore } from './hooks/useLocalStore' import { useLocalStore } from './hooks/useLocalStore'
import { useSafariFocusOutFix } from './hooks/useSafariFocusOutFix' import { useSafariFocusOutFix } from './hooks/useSafariFocusOutFix'
@ -281,23 +282,6 @@ function TldrawEditorWithReadyStore({
} }
}, [container, shapeUtils, tools, store, user, initialState]) }, [container, shapeUtils, tools, store, user, initialState])
React.useLayoutEffect(() => {
if (editor && autoFocus) {
editor.getContainer().focus()
}
}, [editor, autoFocus])
const onMountEvent = useEvent((editor: Editor) => {
const teardown = onMount?.(editor)
editor.emit('mount')
window.tldrawReady = true
return teardown
})
React.useLayoutEffect(() => {
if (editor) return onMountEvent?.(editor)
}, [editor, onMountEvent])
const crashingError = useSyncExternalStore( const crashingError = useSyncExternalStore(
useCallback( useCallback(
(onStoreChange) => { (onStoreChange) => {
@ -335,19 +319,31 @@ function TldrawEditorWithReadyStore({
<Crash crashingError={crashingError} /> <Crash crashingError={crashingError} />
) : ( ) : (
<EditorContext.Provider value={editor}> <EditorContext.Provider value={editor}>
<Layout>{children}</Layout> <Layout autoFocus={autoFocus} onMount={onMount}>
{children}
</Layout>
</EditorContext.Provider> </EditorContext.Provider>
)} )}
</OptionalErrorBoundary> </OptionalErrorBoundary>
) )
} }
function Layout({ children }: { children: any }) { function Layout({
children,
onMount,
autoFocus = false,
}: {
children: any
onMount?: TLOnMountHandler
autoFocus?: boolean
}) {
useZoomCss() useZoomCss()
useCursor() useCursor()
useDarkMode() useDarkMode()
useSafariFocusOutFix() useSafariFocusOutFix()
useForceUpdate() useForceUpdate()
useFocusEvents(autoFocus)
useOnMount(onMount)
useDPRMultiple() useDPRMultiple()
return children ?? <Canvas /> return children ?? <Canvas />
@ -373,3 +369,18 @@ export function LoadingScreen({ children }: { children: any }) {
export function ErrorScreen({ children }: { children: any }) { export function ErrorScreen({ children }: { children: any }) {
return <div className="tl-loading">{children}</div> return <div className="tl-loading">{children}</div>
} }
function useOnMount(onMount?: TLOnMountHandler) {
const editor = useEditor()
const onMountEvent = useEvent((editor: Editor) => {
const teardown = onMount?.(editor)
editor.emit('mount')
window.tldrawReady = true
return teardown
})
React.useLayoutEffect(() => {
if (editor) return onMountEvent?.(editor)
}, [editor, onMountEvent])
}

View file

@ -44,7 +44,6 @@ import {
annotateError, annotateError,
assert, assert,
compact, compact,
debounce,
dedupe, dedupe,
deepCopy, deepCopy,
getOwnProperty, getOwnProperty,
@ -585,37 +584,6 @@ export class Editor extends EventEmitter<TLEventMap> {
}) })
) )
const container = this.getContainer()
// We need to debounce this because when focus changes, the body
// becomes focused for a brief moment. Debouncing means that we
// check only when focus stops changing: when it settles, what
// has it settled on? If it's settled on the container or something
// inside of the container, then focus or preserve the current focus;
// if not, then turn off focus. Turning off focus is a trigger to
// also turn off keyboard shortcuts and other things.
const updateFocus = debounce(() => {
const { activeElement } = document
const { isFocused } = this.instanceState
const hasFocus = container === activeElement || container.contains(activeElement)
if ((!isFocused && hasFocus) || (isFocused && !hasFocus)) {
this.updateInstanceState({ isFocused: hasFocus })
this.updateViewportScreenBounds()
}
}, 32)
container.addEventListener('focusin', updateFocus)
container.addEventListener('focus', updateFocus)
container.addEventListener('focusout', updateFocus)
container.addEventListener('blur', updateFocus)
this.disposables.add(() => {
container.removeEventListener('focusin', updateFocus)
container.removeEventListener('focus', updateFocus)
container.removeEventListener('focusout', updateFocus)
container.removeEventListener('blur', updateFocus)
})
this.store.ensureStoreIsUsable() this.store.ensureStoreIsUsable()
// clear ephemeral state // clear ephemeral state

View file

@ -214,14 +214,6 @@ export function useDocumentEvents() {
} }
} }
function handleBlur() {
editor.complete()
}
function handleFocus() {
editor.updateViewportScreenBounds()
}
container.addEventListener('touchstart', handleTouchStart, { passive: false }) container.addEventListener('touchstart', handleTouchStart, { passive: false })
container.addEventListener('wheel', handleWheel, { passive: false }) container.addEventListener('wheel', handleWheel, { passive: false })
@ -233,9 +225,6 @@ export function useDocumentEvents() {
container.addEventListener('keydown', handleKeyDown) container.addEventListener('keydown', handleKeyDown)
container.addEventListener('keyup', handleKeyUp) container.addEventListener('keyup', handleKeyUp)
window.addEventListener('blur', handleBlur)
window.addEventListener('focus', handleFocus)
return () => { return () => {
container.removeEventListener('touchstart', handleTouchStart) container.removeEventListener('touchstart', handleTouchStart)
@ -247,9 +236,6 @@ export function useDocumentEvents() {
container.removeEventListener('keydown', handleKeyDown) container.removeEventListener('keydown', handleKeyDown)
container.removeEventListener('keyup', handleKeyUp) container.removeEventListener('keyup', handleKeyUp)
window.removeEventListener('blur', handleBlur)
window.removeEventListener('focus', handleFocus)
} }
}, [editor, container, isAppFocused]) }, [editor, container, isAppFocused])
} }

View file

@ -0,0 +1,56 @@
import { debounce } from '@tldraw/utils'
import { useLayoutEffect } from 'react'
import { useContainer } from './useContainer'
import { useEditor } from './useEditor'
/** @internal */
export function useFocusEvents(autoFocus: boolean) {
const editor = useEditor()
const container = useContainer()
useLayoutEffect(() => {
if (!container) return
// We need to debounce this because when focus changes, the body
// becomes focused for a brief moment. Debouncing means that we
// check only when focus stops changing: when it settles, what
// has it settled on? If it's settled on the container or something
// inside of the container, then focus or preserve the current focus;
// if not, then turn off focus. Turning off focus is a trigger to
// also turn off keyboard shortcuts and other things.
const updateFocus = debounce(() => {
const { activeElement } = document
const { isFocused: wasFocused } = editor.instanceState
const isFocused =
document.hasFocus() && (container === activeElement || container.contains(activeElement))
if (wasFocused !== isFocused) {
editor.updateInstanceState({ isFocused })
editor.updateViewportScreenBounds()
if (!isFocused) {
// When losing focus, run complete() to ensure that any interacts end
editor.complete()
}
}
}, 32)
container.addEventListener('focusin', updateFocus)
container.addEventListener('focus', updateFocus)
container.addEventListener('focusout', updateFocus)
container.addEventListener('blur', updateFocus)
return () => {
container.removeEventListener('focusin', updateFocus)
container.removeEventListener('focus', updateFocus)
container.removeEventListener('focusout', updateFocus)
container.removeEventListener('blur', updateFocus)
}
}, [container, editor])
useLayoutEffect(() => {
if (autoFocus) {
editor.getContainer().focus()
}
}, [editor, autoFocus])
}

View file

@ -4,6 +4,7 @@ import {
PageRecordType, PageRecordType,
TLShape, TLShape,
createShapeId, createShapeId,
debounce,
} from '@tldraw/editor' } from '@tldraw/editor'
import { TestEditor } from './TestEditor' import { TestEditor } from './TestEditor'
import { TL } from './test-jsx' import { TL } from './test-jsx'
@ -356,6 +357,33 @@ describe('currentToolId', () => {
}) })
describe('isFocused', () => { describe('isFocused', () => {
beforeEach(() => {
// lame but duplicated here since this was moved into a hook
const container = editor.getContainer()
const updateFocus = debounce(() => {
const { activeElement } = document
const { isFocused: wasFocused } = editor.instanceState
const isFocused =
document.hasFocus() && (container === activeElement || container.contains(activeElement))
if (wasFocused !== isFocused) {
editor.updateInstanceState({ isFocused })
editor.updateViewportScreenBounds()
if (!isFocused) {
// When losing focus, run complete() to ensure that any interacts end
editor.complete()
}
}
}, 32)
container.addEventListener('focusin', updateFocus)
container.addEventListener('focus', updateFocus)
container.addEventListener('focusout', updateFocus)
container.addEventListener('blur', updateFocus)
})
it('is false by default', () => { it('is false by default', () => {
expect(editor.instanceState.isFocused).toBe(false) expect(editor.instanceState.isFocused).toBe(false)
}) })