[fix] Focus events (actually) (#2015)

This PR restores the controlled nature of focus. Focus allows keyboard
shortcuts and other interactions to occur. The editor's focus should
always / entirely be controlled via the autoFocus prop or by manually
setting `editor.instanceState.isFocused`.

Design note: I'm starting to think that focus is the wrong abstraction,
and that we should instead use a kind of "disabled" state for editors
that the user isn't interacting with directly. In a page where multiple
editors exit (e.g. a notion page), a developer could switch from
disabled to enabled using a first interaction.

### Change Type

- [x] `patch` — Bug fix

### Test Plan

- [x] End to end tests
This commit is contained in:
Steve Ruiz 2023-10-04 10:01:48 +01:00 committed by GitHub
parent 6b19d70a9e
commit d715fa3a2e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 236 additions and 122 deletions

View file

@ -1,6 +1,9 @@
import test, { expect } from '@playwright/test' import test, { expect } from '@playwright/test'
import { Editor } from '@tldraw/tldraw'
declare const __tldraw_editor_events: any[] declare const EDITOR_A: Editor
declare const EDITOR_B: Editor
declare const EDITOR_C: Editor
// We're just testing the events, not the actual results. // We're just testing the events, not the actual results.
@ -9,42 +12,98 @@ test.describe('Focus', () => {
await page.goto('http://localhost:5420/multiple') await page.goto('http://localhost:5420/multiple')
await page.waitForSelector('.tl-canvas') await page.waitForSelector('.tl-canvas')
// Component A has autofocus
// Component B does not
const EditorA = (await page.$(`.A`))! const EditorA = (await page.$(`.A`))!
const EditorB = (await page.$(`.B`))! const EditorB = (await page.$(`.B`))!
const EditorC = (await page.$(`.C`))!
expect(EditorA).toBeTruthy() expect(EditorA).toBeTruthy()
expect(EditorB).toBeTruthy() expect(EditorB).toBeTruthy()
expect(EditorC).toBeTruthy()
async function isOnlyFocused(id: 'A' | 'B' | 'C' | null) {
let activeElement: string | null = null
const isA = await EditorA.evaluate(
(node) => document.activeElement === node || node.contains(document.activeElement)
)
const isB = await EditorB.evaluate(
(node) => document.activeElement === node || node.contains(document.activeElement)
)
const isC = await EditorC.evaluate(
(node) => document.activeElement === node || node.contains(document.activeElement)
)
activeElement = isA ? 'A' : isB ? 'B' : isC ? 'C' : null
expect(
activeElement,
`Active element should have been ${id}, but was ${activeElement ?? 'null'} instead`
).toBe(id)
await page.evaluate(
({ id }) => {
if (
!(
EDITOR_A.instanceState.isFocused === (id === 'A') &&
EDITOR_B.instanceState.isFocused === (id === 'B') &&
EDITOR_C.instanceState.isFocused === (id === 'C')
)
) {
throw Error('isFocused is not correct')
}
},
{ id }
)
}
// Component A has autofocus
// Component B does not
// Component C does not
// Component B and C share persistence id
await (await page.$('body'))?.click() await (await page.$('body'))?.click()
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(true) await isOnlyFocused('A')
expect(await EditorB.evaluate((node) => document.activeElement === node)).toBe(false)
await (await page.$('body'))?.click()
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(false)
expect(await EditorB.evaluate((node) => document.activeElement === node)).toBe(false)
await EditorA.click() await EditorA.click()
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(true)
expect(await EditorB.evaluate((node) => document.activeElement === node)).toBe(false) await isOnlyFocused('A')
await EditorA.click() await EditorA.click()
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(false)
expect(await EditorB.evaluate((node) => document.activeElement === node)).toBe(false) await isOnlyFocused('A')
expect(await EditorA.evaluate((node) => node.contains(document.activeElement))).toBe(true)
await EditorB.click() await EditorB.click()
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(false)
expect(await EditorB.evaluate((node) => document.activeElement === node)).toBe(true) await isOnlyFocused('B')
expect(await EditorA.evaluate((node) => node.contains(document.activeElement))).toBe(false)
// Escape does not break focus // Escape does not break focus
await page.keyboard.press('Escape') await page.keyboard.press('Escape')
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(false)
expect(await EditorB.evaluate((node) => node.contains(document.activeElement))).toBe(true) await isOnlyFocused('B')
})
test('kbds when not focused', async ({ page }) => {
await page.goto('http://localhost:5420/multiple')
await page.waitForSelector('.tl-canvas')
// Should not have any shapes on the page
expect(await page.evaluate(() => EDITOR_A.currentPageShapes.length)).toBe(0)
const EditorA = (await page.$(`.A`))!
await page.keyboard.press('r')
await EditorA.click({ position: { x: 100, y: 100 } })
// Should not have created a shape
expect(await page.evaluate(() => EDITOR_A.currentPageShapes.length)).toBe(1)
const TextArea = page.getByTestId(`textarea`)
await TextArea.focus()
await page.keyboard.type('hello world')
await page.keyboard.press('Control+A')
await page.keyboard.press('Delete')
// Should not have deleted the page
expect(await page.evaluate(() => EDITOR_A.currentPageShapes.length)).toBe(1)
}) })
test('kbds when focused', async ({ page }) => { test('kbds when focused', async ({ page }) => {
@ -53,14 +112,13 @@ test.describe('Focus', () => {
const EditorA = (await page.$(`.A`))! const EditorA = (await page.$(`.A`))!
const EditorB = (await page.$(`.B`))! const EditorB = (await page.$(`.B`))!
const EditorC = (await page.$(`.C`))!
expect(EditorA).toBeTruthy() expect(EditorA).toBeTruthy()
expect(EditorB).toBeTruthy() expect(EditorB).toBeTruthy()
expect(EditorC).toBeTruthy()
await (await page.$('body'))?.click() await (await page.$('body'))?.click()
expect(await EditorA.evaluate((node) => document.activeElement === node)).toBe(true)
expect(await EditorB.evaluate((node) => document.activeElement === node)).toBe(false)
expect(await EditorA.$('.tlui-button[data-testid="tools.draw"][data-state="selected"]')).toBe( expect(await EditorA.$('.tlui-button[data-testid="tools.draw"][data-state="selected"]')).toBe(
null null
) )

View file

@ -4,18 +4,18 @@ import { createContext, useCallback, useContext, useState } from 'react'
const focusedEditorContext = createContext( const focusedEditorContext = createContext(
{} as { {} as {
focusedEditor: string focusedEditor: string | null
setFocusedEditor: (id: string) => void setFocusedEditor: (id: string | null) => void
} }
) )
export default function MultipleExample() { export default function MultipleExample() {
const [focusedEditor, focusedEditorSetter] = useState('first') const [focusedEditor, _setFocusedEditor] = useState<string | null>('A')
const setFocusedEditor = useCallback( const setFocusedEditor = useCallback(
(id: string) => { (id: string | null) => {
if (focusedEditor !== id) { if (focusedEditor !== id) {
focusedEditorSetter(id) _setFocusedEditor(id)
} }
}, },
[focusedEditor] [focusedEditor]
@ -29,47 +29,88 @@ export default function MultipleExample() {
}} }}
> >
<focusedEditorContext.Provider value={{ focusedEditor, setFocusedEditor }}> <focusedEditorContext.Provider value={{ focusedEditor, setFocusedEditor }}>
<h1>Focusing: "{focusedEditor}"</h1> <h1>Focusing: {focusedEditor ?? 'none'}</h1>
<FirstEditor /> <EditorA />
<textarea defaultValue="type in me" style={{ margin: 10 }}></textarea> <textarea data-testid="textarea" placeholder="type in me" style={{ margin: 10 }} />
<SecondEditor /> <div
style={{
width: '100%',
display: 'grid',
gridTemplateColumns: 'repeat(auto-fit, minmax(420px, 1fr))',
gap: 64,
}}
>
<EditorB />
<EditorC />
</div>
<p>
These two editors share the same persistence key so they will share a (locally)
synchronized document.
</p>
<ABunchOfText /> <ABunchOfText />
</focusedEditorContext.Provider> </focusedEditorContext.Provider>
</div> </div>
) )
} }
function FirstEditor() { function EditorA() {
const { focusedEditor, setFocusedEditor } = useContext(focusedEditorContext) const { focusedEditor, setFocusedEditor } = useContext(focusedEditorContext)
const isFocused = focusedEditor === 'A'
return ( return (
<div> <div style={{ padding: 32 }}>
<h2>First Example</h2> <h2>A</h2>
<p>This is the second example.</p> <div tabIndex={-1} onFocus={() => setFocusedEditor('A')} style={{ height: 600 }}>
<div <Tldraw
tabIndex={-1} persistenceKey="steve"
onFocus={() => setFocusedEditor('first')} className="A"
style={{ width: '100%', height: '600px', padding: 32 }} autoFocus={isFocused}
> onMount={(editor) => {
<Tldraw persistenceKey="steve" className="A" autoFocus={focusedEditor === 'first'} /> ;(window as any).EDITOR_A = editor
}}
/>
</div> </div>
</div> </div>
) )
} }
function SecondEditor() { function EditorB() {
const { focusedEditor, setFocusedEditor } = useContext(focusedEditorContext) const { focusedEditor, setFocusedEditor } = useContext(focusedEditorContext)
const isFocused = focusedEditor === 'B'
return ( return (
<div> <div>
<h2>Second Example</h2> <h2>B</h2>
<p>This is the second example.</p> <div tabIndex={-1} onFocus={() => setFocusedEditor('B')} style={{ height: 600 }}>
<div <Tldraw
tabIndex={-1} persistenceKey="david"
onFocus={() => setFocusedEditor('second')} className="B"
style={{ width: '100%', height: '600px' }} autoFocus={isFocused}
> onMount={(editor) => {
<Tldraw persistenceKey="david" className="B" autoFocus={focusedEditor === 'second'} /> ;(window as any).EDITOR_B = editor
}}
/>
</div>
</div>
)
}
function EditorC() {
const { focusedEditor, setFocusedEditor } = useContext(focusedEditorContext)
const isFocused = focusedEditor === 'third'
return (
<div>
<h2>C</h2>
<div tabIndex={-1} onFocus={() => setFocusedEditor('C')} style={{ height: 600 }}>
<Tldraw
persistenceKey="david"
className="C"
autoFocus={isFocused}
onMount={(editor) => {
;(window as any).EDITOR_C = editor
}}
/>
</div> </div>
</div> </div>
) )
@ -77,46 +118,44 @@ function SecondEditor() {
function ABunchOfText() { function ABunchOfText() {
return ( return (
<div style={{ width: '100%', display: 'flex', alignItems: 'center', flexDirection: 'column' }}> <article style={{ maxWidth: 500 }}>
<article style={{ maxWidth: 600 }}> <h1>White Board</h1>
<h1>White Board</h1> <h2>Chapter 1: The First Strokes</h2>
<h2>Chapter 1: The First Strokes</h2> <p>
<p> The fluorescent lights flickered overhead as John sat hunched over his desk, his fingers
The fluorescent lights flickered overhead as John sat hunched over his desk, his fingers tapping rhythmically on the keyboard. He was a software developer, and tonight, he had a
tapping rhythmically on the keyboard. He was a software developer, and tonight, he had a peculiar mission. A mission that would take him deep into the labyrinthine world of web
peculiar mission. A mission that would take him deep into the labyrinthine world of web development. John had stumbled upon a new whiteboard library called "tldraw," a seemingly
development. John had stumbled upon a new whiteboard library called "tldraw," a seemingly simple tool that promised to revolutionize collaborative drawing on the web. Little did he
simple tool that promised to revolutionize collaborative drawing on the web. Little did he know that this discovery would set off a chain of events that would challenge his skills,
know that this discovery would set off a chain of events that would challenge his skills, test his perseverance, and blur the line between reality and imagination.
test his perseverance, and blur the line between reality and imagination. </p>
</p> <p>
<p> With a newfound sense of excitement, John began integrating "tldraw" into his latest
With a newfound sense of excitement, John began integrating "tldraw" into his latest project. As lines of code danced across his screen, he imagined the possibilities that lay
project. As lines of code danced across his screen, he imagined the possibilities that lay ahead. The potential to create virtual spaces where ideas could be shared, concepts could be
ahead. The potential to create virtual spaces where ideas could be shared, concepts could visualized, and teams could collaborate seamlessly from different corners of the world. It
be visualized, and teams could collaborate seamlessly from different corners of the world. was a dream that seemed within reach, a vision of a future where creativity and technology
It was a dream that seemed within reach, a vision of a future where creativity and merged into a harmonious symphony.
technology merged into a harmonious symphony. </p>
</p> <p>
<p> As the night wore on, John's mind became consumed with the whiteboard library. He couldn't
As the night wore on, John's mind became consumed with the whiteboard library. He couldn't help but marvel at its elegance and simplicity. With each stroke of his keyboard, he felt a
help but marvel at its elegance and simplicity. With each stroke of his keyboard, he felt surge of inspiration, a connection to something greater than himself. It was as if the lines
a surge of inspiration, a connection to something greater than himself. It was as if the of code he was writing were transforming into a digital canvas, waiting to be filled with
lines of code he was writing were transforming into a digital canvas, waiting to be filled the strokes of imagination. In that moment, John realized that he was not just building a
with the strokes of imagination. In that moment, John realized that he was not just tool, but breathing life into a new form of expression. The whiteboard was no longer just a
building a tool, but breathing life into a new form of expression. The whiteboard was no blank slate; it had become a portal to a world where ideas could flourish and dreams could
longer just a blank slate; it had become a portal to a world where ideas could flourish take shape.
and dreams could take shape. </p>
</p> <p>
<p> Little did John know, this integration of "tldraw" was only the beginning. It would lead him
Little did John know, this integration of "tldraw" was only the beginning. It would lead down a path filled with unforeseen challenges, where he would confront his own limitations
him down a path filled with unforeseen challenges, where he would confront his own and question the very nature of creation. The journey ahead would test his resolve, pushing
limitations and question the very nature of creation. The journey ahead would test his him to the edge of his sanity. And as he embarked on this perilous adventure, he could not
resolve, pushing him to the edge of his sanity. And as he embarked on this perilous shake the feeling that the whiteboard held secrets far beyond his understanding. Secrets
adventure, he could not shake the feeling that the whiteboard held secrets far beyond his that would unfold before his eyes, one stroke at a time.
understanding. Secrets that would unfold before his eyes, one stroke at a time. </p>
</p> </article>
</article>
</div>
) )
} }

View file

@ -2296,7 +2296,7 @@ export type TLOnHandleChangeHandler<T extends TLShape> = (shape: T, info: {
}) => TLShapePartial<T> | void; }) => TLShapePartial<T> | void;
// @public // @public
export type TLOnMountHandler = (editor: Editor) => (() => void) | undefined | void; export type TLOnMountHandler = (editor: Editor) => (() => undefined | void) | undefined | void;
// @public (undocumented) // @public (undocumented)
export type TLOnResizeEndHandler<T extends TLShape> = TLEventChangeHandler<T>; export type TLOnResizeEndHandler<T extends TLShape> = TLEventChangeHandler<T>;

View file

@ -123,7 +123,7 @@ export interface TldrawEditorBaseProps {
* *
* @public * @public
*/ */
export type TLOnMountHandler = (editor: Editor) => (() => void) | undefined | void export type TLOnMountHandler = (editor: Editor) => (() => void | undefined) | undefined | void
declare global { declare global {
interface Window { interface Window {
@ -162,7 +162,7 @@ export const TldrawEditor = memo(function TldrawEditor({
ref={rContainer} ref={rContainer}
draggable={false} draggable={false}
className={classNames('tl-container tl-theme__light', className)} className={classNames('tl-container tl-theme__light', className)}
tabIndex={0} tabIndex={-1}
> >
<OptionalErrorBoundary <OptionalErrorBoundary
fallback={ErrorFallback} fallback={ErrorFallback}

View file

@ -1,5 +1,10 @@
import React, { useMemo } from 'react' import React, { useMemo } from 'react'
import { preventDefault, releasePointerCapture, setPointerCapture } from '../utils/dom' import {
preventDefault,
releasePointerCapture,
setPointerCapture,
stopEventPropagation,
} from '../utils/dom'
import { getPointerInfo } from '../utils/getPointerInfo' import { getPointerInfo } from '../utils/getPointerInfo'
import { useEditor } from './useEditor' import { useEditor } from './useEditor'
@ -12,6 +17,8 @@ export function useCanvasEvents() {
let lastX: number, lastY: number let lastX: number, lastY: number
function onPointerDown(e: React.PointerEvent) { function onPointerDown(e: React.PointerEvent) {
stopEventPropagation(e)
if ((e as any).isKilled) return if ((e as any).isKilled) return
if (e.button === 2) { if (e.button === 2) {
@ -103,6 +110,10 @@ export function useCanvasEvents() {
}) })
} }
function onClick(e: React.MouseEvent) {
stopEventPropagation(e)
}
return { return {
onPointerDown, onPointerDown,
onPointerMove, onPointerMove,
@ -111,6 +122,7 @@ export function useCanvasEvents() {
onDrop, onDrop,
onTouchStart, onTouchStart,
onTouchEnd, onTouchEnd,
onClick,
} }
}, },
[editor] [editor]

View file

@ -7,25 +7,26 @@ export function useFocusEvents(autoFocus: boolean) {
const editor = useEditor() const editor = useEditor()
const container = useContainer() const container = useContainer()
useLayoutEffect(() => { useLayoutEffect(() => {
function handleFocus() { if (autoFocus) {
// When autoFocus is true, update the editor state to be focused
// unless it's already focused
if (!editor.instanceState.isFocused) { if (!editor.instanceState.isFocused) {
editor.updateInstanceState({ isFocused: true }) editor.updateInstanceState({ isFocused: true })
} }
}
container.addEventListener('focus', handleFocus) // Note: Focus is also handled by the side effect manager in tldraw.
container.addEventListener('pointerdown', handleFocus) // Importantly, if a user manually sets isFocused to true (or if it
// changes for any reason from false to true), the side effect manager
if (autoFocus && !editor.instanceState.isFocused) { // in tldraw will also take care of the focus. However, it may be that
editor.updateInstanceState({ isFocused: true }) // on first mount the editor already has isFocused: true in the model,
container.focus() // so we also need to focus it here just to be sure.
} else if (editor.instanceState.isFocused) { editor.getContainer().focus()
editor.updateInstanceState({ isFocused: false }) } else {
} // When autoFocus is false, update the editor state to be not focused
// unless it's already not focused
return () => { if (editor.instanceState.isFocused) {
container.removeEventListener('focus', handleFocus) editor.updateInstanceState({ isFocused: false })
container.removeEventListener('pointerdown', handleFocus) }
} }
}, [editor, container, autoFocus]) }, [editor, container, autoFocus])
} }

View file

@ -1,6 +1,11 @@
import { useMemo } from 'react' import { useMemo } from 'react'
import { TLSelectionHandle } from '../editor/types/selection-types' import { TLSelectionHandle } from '../editor/types/selection-types'
import { loopToHtmlElement, releasePointerCapture, setPointerCapture } from '../utils/dom' import {
loopToHtmlElement,
releasePointerCapture,
setPointerCapture,
stopEventPropagation,
} from '../utils/dom'
import { getPointerInfo } from '../utils/getPointerInfo' import { getPointerInfo } from '../utils/getPointerInfo'
import { useEditor } from './useEditor' import { useEditor } from './useEditor'
@ -48,7 +53,7 @@ export function useSelectionEvents(handle: TLSelectionHandle) {
handle, handle,
...getPointerInfo(e), ...getPointerInfo(e),
}) })
e.stopPropagation() stopEventPropagation(e)
} }
// Track the last screen point // Track the last screen point

View file

@ -5,7 +5,6 @@ export function registerDefaultSideEffects(editor: Editor) {
editor.sideEffects.registerAfterChangeHandler('instance', (prev, next) => { editor.sideEffects.registerAfterChangeHandler('instance', (prev, next) => {
if (prev.isFocused !== next.isFocused) { if (prev.isFocused !== next.isFocused) {
if (next.isFocused) { if (next.isFocused) {
editor.complete() // stop any interaction
editor.getContainer().focus() editor.getContainer().focus()
editor.updateViewportScreenBounds() editor.updateViewportScreenBounds()
} else { } else {

View file

@ -1,4 +1,4 @@
import { TLFrameShape, TLShapeId, useEditor } from '@tldraw/editor' import { TLFrameShape, TLShapeId, stopEventPropagation, useEditor } from '@tldraw/editor'
import { forwardRef, useCallback } from 'react' import { forwardRef, useCallback } from 'react'
import { defaultEmptyAs } from '../FrameShapeUtil' import { defaultEmptyAs } from '../FrameShapeUtil'
@ -13,7 +13,7 @@ export const FrameLabelInput = forwardRef<
if (e.key === 'Enter' && !e.nativeEvent.isComposing) { if (e.key === 'Enter' && !e.nativeEvent.isComposing) {
// need to prevent the enter keydown making it's way up to the Idle state // need to prevent the enter keydown making it's way up to the Idle state
// and sending us back into edit mode // and sending us back into edit mode
e.stopPropagation() stopEventPropagation(e)
e.currentTarget.blur() e.currentTarget.blur()
editor.setEditingShape(null) editor.setEditingShape(null)
} }

View file

@ -131,7 +131,7 @@ const TldrawUiContent = React.memo(function TldrawUI({
return ( return (
<ToastProvider> <ToastProvider>
<main <div
className={classNames('tlui-layout', { className={classNames('tlui-layout', {
'tlui-layout__mobile': breakpoint < 5, 'tlui-layout__mobile': breakpoint < 5,
})} })}
@ -180,7 +180,7 @@ const TldrawUiContent = React.memo(function TldrawUI({
<Dialogs /> <Dialogs />
<ToastViewport /> <ToastViewport />
<FollowingIndicator /> <FollowingIndicator />
</main> </div>
</ToastProvider> </ToastProvider>
) )
}) })

View file

@ -1,4 +1,4 @@
import { useEditor } from '@tldraw/editor' import { stopEventPropagation, useEditor } from '@tldraw/editor'
import classNames from 'classnames' import classNames from 'classnames'
import * as React from 'react' import * as React from 'react'
import { TLUiTranslationKey } from '../../hooks/useTranslation/TLUiTranslationKey' import { TLUiTranslationKey } from '../../hooks/useTranslation/TLUiTranslationKey'
@ -94,14 +94,14 @@ export const Input = React.forwardRef<HTMLInputElement, TLUiInputProps>(function
switch (e.key) { switch (e.key) {
case 'Enter': { case 'Enter': {
e.currentTarget.blur() e.currentTarget.blur()
e.stopPropagation() stopEventPropagation(e)
onComplete?.(e.currentTarget.value) onComplete?.(e.currentTarget.value)
break break
} }
case 'Escape': { case 'Escape': {
e.currentTarget.value = rInitialValue.current e.currentTarget.value = rInitialValue.current
e.currentTarget.blur() e.currentTarget.blur()
e.stopPropagation() stopEventPropagation(e)
onCancel?.(e.currentTarget.value) onCancel?.(e.currentTarget.value)
break break
} }