From 20704ea41768f0746480bd840b008ecda9778627 Mon Sep 17 00:00:00 2001 From: Steve Ruiz Date: Sat, 9 Sep 2023 10:41:06 +0100 Subject: [PATCH] [fix] iframe losing focus on pointer down (#1848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/editor/src/lib/TldrawEditor.tsx | 51 ++++++++++------- packages/editor/src/lib/editor/Editor.ts | 32 ----------- .../editor/src/lib/hooks/useDocumentEvents.ts | 14 ----- .../editor/src/lib/hooks/useFocusEvents.ts | 56 +++++++++++++++++++ packages/tldraw/src/test/Editor.test.tsx | 28 ++++++++++ 5 files changed, 115 insertions(+), 66 deletions(-) create mode 100644 packages/editor/src/lib/hooks/useFocusEvents.ts diff --git a/packages/editor/src/lib/TldrawEditor.tsx b/packages/editor/src/lib/TldrawEditor.tsx index 4763c7553..c79438e16 100644 --- a/packages/editor/src/lib/TldrawEditor.tsx +++ b/packages/editor/src/lib/TldrawEditor.tsx @@ -22,13 +22,14 @@ import { ContainerProvider, useContainer } from './hooks/useContainer' import { useCursor } from './hooks/useCursor' import { useDPRMultiple } from './hooks/useDPRMultiple' import { useDarkMode } from './hooks/useDarkMode' -import { EditorContext } from './hooks/useEditor' +import { EditorContext, useEditor } from './hooks/useEditor' import { EditorComponentsProvider, TLEditorComponents, useEditorComponents, } from './hooks/useEditorComponents' import { useEvent } from './hooks/useEvent' +import { useFocusEvents } from './hooks/useFocusEvents' import { useForceUpdate } from './hooks/useForceUpdate' import { useLocalStore } from './hooks/useLocalStore' import { useSafariFocusOutFix } from './hooks/useSafariFocusOutFix' @@ -281,23 +282,6 @@ function TldrawEditorWithReadyStore({ } }, [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( useCallback( (onStoreChange) => { @@ -335,19 +319,31 @@ function TldrawEditorWithReadyStore({ ) : ( - {children} + + {children} + )} ) } -function Layout({ children }: { children: any }) { +function Layout({ + children, + onMount, + autoFocus = false, +}: { + children: any + onMount?: TLOnMountHandler + autoFocus?: boolean +}) { useZoomCss() useCursor() useDarkMode() useSafariFocusOutFix() useForceUpdate() + useFocusEvents(autoFocus) + useOnMount(onMount) useDPRMultiple() return children ?? @@ -373,3 +369,18 @@ export function LoadingScreen({ children }: { children: any }) { export function ErrorScreen({ children }: { children: any }) { return
{children}
} + +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]) +} diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 2b35afb63..a1ec423e6 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -44,7 +44,6 @@ import { annotateError, assert, compact, - debounce, dedupe, deepCopy, getOwnProperty, @@ -585,37 +584,6 @@ export class Editor extends EventEmitter { }) ) - 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() // clear ephemeral state diff --git a/packages/editor/src/lib/hooks/useDocumentEvents.ts b/packages/editor/src/lib/hooks/useDocumentEvents.ts index 281c661b3..9524dfe5a 100644 --- a/packages/editor/src/lib/hooks/useDocumentEvents.ts +++ b/packages/editor/src/lib/hooks/useDocumentEvents.ts @@ -214,14 +214,6 @@ export function useDocumentEvents() { } } - function handleBlur() { - editor.complete() - } - - function handleFocus() { - editor.updateViewportScreenBounds() - } - container.addEventListener('touchstart', handleTouchStart, { passive: false }) container.addEventListener('wheel', handleWheel, { passive: false }) @@ -233,9 +225,6 @@ export function useDocumentEvents() { container.addEventListener('keydown', handleKeyDown) container.addEventListener('keyup', handleKeyUp) - window.addEventListener('blur', handleBlur) - window.addEventListener('focus', handleFocus) - return () => { container.removeEventListener('touchstart', handleTouchStart) @@ -247,9 +236,6 @@ export function useDocumentEvents() { container.removeEventListener('keydown', handleKeyDown) container.removeEventListener('keyup', handleKeyUp) - - window.removeEventListener('blur', handleBlur) - window.removeEventListener('focus', handleFocus) } }, [editor, container, isAppFocused]) } diff --git a/packages/editor/src/lib/hooks/useFocusEvents.ts b/packages/editor/src/lib/hooks/useFocusEvents.ts new file mode 100644 index 000000000..474c8ae90 --- /dev/null +++ b/packages/editor/src/lib/hooks/useFocusEvents.ts @@ -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]) +} diff --git a/packages/tldraw/src/test/Editor.test.tsx b/packages/tldraw/src/test/Editor.test.tsx index 66429bce9..fe998aed5 100644 --- a/packages/tldraw/src/test/Editor.test.tsx +++ b/packages/tldraw/src/test/Editor.test.tsx @@ -4,6 +4,7 @@ import { PageRecordType, TLShape, createShapeId, + debounce, } from '@tldraw/editor' import { TestEditor } from './TestEditor' import { TL } from './test-jsx' @@ -356,6 +357,33 @@ describe('currentToolId', () => { }) 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', () => { expect(editor.instanceState.isFocused).toBe(false) })