PrivateCoffee/tldraw
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

[fix] Sticky text content / hovered shapes (#1808)

Browse source 
This PR improves the UX around sticky notes. It fixes were some bugs
related to the editing / hovered shape after cloning a sticky note
shape.

### Change Type

- [x] `patch` — Bug fix

### Test Plan

1. Use the sticky note tool
2. Alt-drag to clone sticky notes
3. Use the Enter key to edit the selected shape.
4. Double click an editable shape and then click once to edit a shape of
the same type.

- [x] Unit Tests
This commit is contained in:
Steve Ruiz 2023-08-15 17:25:50 +01:00 committed by GitHub
parent 22329c51fc
commit 1dc76fe32b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 201 additions and 142 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
packages/editor/src/lib/editor/Editor.ts
View file

@ -1806,22 +1806,23 @@ export class Editor extends EventEmitter<TLEventMap> {
* editor.setEditingShape(myShape.id)
* ```
*
* @param shapes - The shape (or shape id) to set as editing.
* @param shape - The shape (or shape id) to set as editing.
*
* @public
*/
setEditingShape(shape: TLShapeId | TLShape | null): this {
const id = typeof shape === 'string' ? shape : shape?.id ?? null
if (!id) {
this._setInstancePageState({ editingShapeId: null })
} else {
if (id !== this.editingShapeId) {
const shape = this.getShape(id)!
const util = this.getShapeUtil(shape)
if (shape && util.canEdit(shape)) {
this._setInstancePageState({ editingShapeId: id, hoveredShapeId: null })
if (id !== this.editingShapeId) {
if (id) {
const shape = this.getShape(id)
if (shape && this.getShapeUtil(shape).canEdit(shape)) {
this._setInstancePageState({ editingShapeId: id })
return this
}
}
// Either we just set the editing id to null, or the shape was missing or not editable
this._setInstancePageState({ editingShapeId: null })
}
return this
}

4
packages/tldraw/src/lib/shapes/shared/TextLabel.tsx
View file

@ -43,7 +43,7 @@ export const TextLabel = React.memo(function TextLabel<
rInput,
isEmpty,
isEditing,
isEditableFromHover,
isEditingSameShapeType,
handleFocus,
handleChange,
handleKeyDown,
@ -52,7 +52,7 @@ export const TextLabel = React.memo(function TextLabel<
handleDoubleClick,
} = useEditableText(id, type, text)
const isInteractive = isEditing || isEditableFromHover
const isInteractive = isEditing || isEditingSameShapeType
const finalText = TextHelpers.normalizeTextForDom(text)
const hasText = finalText.trim().length > 0
const legacyAlign = isLegacyAlign(align)

124
packages/tldraw/src/lib/shapes/shared/useEditableText.ts
View file

@ -6,7 +6,6 @@ import {
getPointerInfo,
preventDefault,
stopEventPropagation,
transact,
useEditor,
useValue,
} from '@tldraw/editor'
@ -21,63 +20,49 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
const editor = useEditor()
const rInput = useRef<HTMLTextAreaElement>(null)
const isEditing = useValue('isEditing', () => editor.currentPageState.editingShapeId === id, [
editor,
id,
])
const rSkipSelectOnFocus = useRef(false)
const rSelectionRanges = useRef<Range[] | null>()
const isEditableFromHover = useValue(
'is editable hovering',
const isEditing = useValue('isEditing', () => editor.editingShapeId === id, [editor, id])
const isEditingSameShapeType = useValue(
'is editing same shape type',
() => {
const { hoveredShapeId, editingShape } = editor
if (type === 'text' && editor.isIn('text') && hoveredShapeId === id) {
return true
}
if (editingShape) {
return (
// We must be hovering over this shape and not editing it
hoveredShapeId === id &&
editingShape.id !== id &&
// the editing shape must be the same type as this shape
editingShape.type === type &&
// and this shape must be capable of being editing in its current form
editor.getShapeUtil(editingShape).canEdit(editingShape)
)
}
return false
const { editingShape } = editor
return editingShape && editingShape.type === type
},
[type, id]
)
// When the label receives focus, set the value to the most
// recent text value and select all of the text
const handleFocus = useCallback(() => {
// We only want to do the select all thing if the shape
// was the first shape to become editing. Switching from
// one editing shape to another should not select all.
if (isEditableFromHover) return
// If the shape is editing but the input element not focused, focus the element
useEffect(() => {
const elm = rInput.current
if (elm && isEditing && document.activeElement !== elm) {
elm.focus()
}
}, [isEditing])
// When the label receives focus, set the value to the most recent text value and select all of the text
const handleFocus = useCallback(() => {
// Store and turn off the skipSelectOnFocus flag
const skipSelect = rSkipSelectOnFocus.current
rSkipSelectOnFocus.current = false
// On the next frame, if we're not skipping select AND we have text in the element, then focus the text
requestAnimationFrame(() => {
const elm = rInput.current
if (!elm) return
const shape = editor.getShape<TLShape & { props: { text: string } }>(id)
if (shape) {
elm.value = shape.props.text
if (elm.value.length && !rSkipSelectOnFocus.current) {
if (elm.value.length && !skipSelect) {
elm.select()
}
rSkipSelectOnFocus.current = false
}
})
}, [editor, id, isEditableFromHover])
}, [editor, id])
// When the label blurs, deselect all of the text and complete.
// This makes it so that the canvas does not have to be focused
@ -87,11 +72,12 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
requestAnimationFrame(() => {
const elm = rInput.current
const { editingShapeId } = editor
// Did we move to a different shape?
if (elm && editor.editingShapeId) {
if (elm && editingShapeId) {
// important! these ^v are two different things
// is that shape OUR shape?
if (editor.editingShapeId === id) {
if (editingShapeId === id) {
if (ranges) {
if (!ranges.length) {
// If we don't have any ranges, restore selection
@ -122,6 +108,8 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
// When the user presses tab, indent or unindent the text.
const handleKeyDown = useCallback(
(e: React.KeyboardEvent<HTMLTextAreaElement>) => {
if (!isEditing) return
if (e.ctrlKey || e.metaKey) stopEventPropagation(e)
switch (e.key) {
@ -142,12 +130,14 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
}
}
},
[editor]
[editor, isEditing]
)
// When the text changes, update the text value.
const handleChange = useCallback(
(e: React.ChangeEvent<HTMLTextAreaElement>) => {
if (!isEditing) return
let text = TextHelpers.normalizeText(e.currentTarget.value)
// ------- Bug fix ------------
@ -166,12 +156,14 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
{ id, type, props: { text } },
])
},
[editor, id, type]
[editor, id, type, isEditing]
)
const isEmpty = text.trim().length === 0
useEffect(() => {
if (!isEditing) return
const elm = rInput.current
if (elm) {
function updateSelection() {
@ -195,44 +187,40 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
document.removeEventListener('selectionchange', updateSelection)
}
}
}, [])
}, [isEditing])
const handleInputPointerDown = useCallback(
(e: React.PointerEvent) => {
if (isEditableFromHover) {
transact(() => {
editor.setEditingShape(id)
editor.setHoveredShape(id)
editor.setSelectedShapes([id])
})
} else {
editor.dispatch({
...getPointerInfo(e),
type: 'pointer',
name: 'pointer_down',
target: 'shape',
shape: editor.getShape(id)!,
})
const { editingShape } = editor
if (editingShape) {
// If there's an editing shape and it's the same type as this shape,
// then we can "deep edit" into this shape. Note that this won't work
// as expected with the note shape—in that case clicking outside of the
// input will not set skipSelectOnFocus to true, and so the input will
// blur, re-select, and then re-select-all on a second tap.
rSkipSelectOnFocus.current = type === editingShape.type
}
stopEventPropagation(e)
},
[editor, isEditableFromHover, id]
)
editor.dispatch({
...getPointerInfo(e),
type: 'pointer',
name: 'pointer_down',
target: 'shape',
shape: editor.getShape(id)!,
})
useEffect(() => {
const elm = rInput.current
if (elm && isEditing && document.activeElement !== elm) {
elm.focus()
}
}, [isEditing])
stopEventPropagation(e) // we need to prevent blurring the input
},
[editor, id, type]
)
const handleDoubleClick = stopEventPropagation
return {
rInput,
isEditing,
isEditableFromHover,
isEditingSameShapeType,
handleFocus,
handleBlur,
handleKeyDown,

4
packages/tldraw/src/lib/shapes/text/TextShapeUtil.tsx
View file

@ -77,7 +77,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
rInput,
isEmpty,
isEditing,
isEditableFromHover,
isEditingSameShapeType,
handleFocus,
handleChange,
handleKeyDown,
@ -107,7 +107,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
<div className="tl-text tl-text-content" dir="ltr">
{text}
</div>
{isEditing || isEditableFromHover ? (
{isEditing || isEditingSameShapeType ? (
<textarea
ref={rInput}
className="tl-text tl-text-input"

95
packages/tldraw/src/lib/tools/SelectTool/children/EditingShape.ts
View file

@ -1,10 +1,17 @@
import { StateNode, TLArrowShape, TLEventHandlers, TLGeoShape } from '@tldraw/editor'
import { Group2d, StateNode, TLArrowShape, TLEventHandlers, TLGeoShape } from '@tldraw/editor'
import { getHitShapeOnCanvasPointerDown } from '../../selection-logic/getHitShapeOnCanvasPointerDown'
import { updateHoveredId } from '../../selection-logic/updateHoveredId'
export class EditingShape extends StateNode {
static override id = 'editing_shape'
override onEnter = () => {
const { editingShape } = this.editor
if (!editingShape) throw Error('Entered editing state without an editing shape')
updateHoveredId(this.editor)
this.editor.select(editingShape)
}
override onExit = () => {
const { editingShapeId } = this.editor.currentPageState
if (!editingShapeId) return
@ -29,40 +36,10 @@ export class EditingShape extends StateNode {
}
}
override onPointerDown: TLEventHandlers['onPointerDown'] = (info) => {
// This is pretty tricky.
// Most of the time, we wouldn't want pointer events inside of an editing
// shape to de-select the shape or change the editing state. We would just
// ignore those pointer events.
// The exception to this is shapes that have only parts of themselves that are
// editable, such as the label on a geo shape. In this case, we would want clicks
// that are outside of the label but inside of the shape to end the editing session
// and select the shape instead.
// What we'll do here (for now at least) is have the text label / input element
// have a pointer event handler (in useEditableText) that dispatches its own
// "shape" type event, which lets us know to ignore the event. If we instead get
// a "canvas" type event, then we'll check to see if the hovered shape is a geo
// shape and if so, we'll end the editing session and select the shape.
switch (info.target) {
case 'shape': {
if (info.shape.id === this.editor.editingShapeId) {
return
}
break
}
case 'canvas': {
const hitShape = getHitShapeOnCanvasPointerDown(this.editor)
if (
hitShape &&
!(
this.editor.isShapeOfType<TLGeoShape>(hitShape, 'geo') ||
this.editor.isShapeOfType<TLArrowShape>(hitShape, 'arrow')
)
) {
if (hitShape) {
this.onPointerDown({
...info,
shape: hitShape,
@ -70,11 +47,63 @@ export class EditingShape extends StateNode {
})
return
}
break
}
case 'shape': {
const { shape } = info
const { editingShape } = this.editor
if (!editingShape) {
throw Error('Expected an editing shape!')
}
if (shape.type === editingShape.type) {
// clicked a shape of the same type as the editing shape
if (
this.editor.isShapeOfType<TLGeoShape>(shape, 'geo') ||
this.editor.isShapeOfType<TLArrowShape>(shape, 'arrow')
) {
// for shapes with labels, check to see if the click was inside of the shape's label
const geometry = this.editor.getShapeUtil(shape).getGeometry(shape) as Group2d
const labelGeometry = geometry.children[1]
if (labelGeometry) {
const pointInShapeSpace = this.editor.getPointInShapeSpace(
shape,
this.editor.inputs.currentPagePoint
)
if (labelGeometry.bounds.containsPoint(pointInShapeSpace)) {
// it's a hit to the label!
if (shape.id === editingShape.id) {
// If we clicked on the editing geo / arrow shape's label, do nothing
return
} else {
this.editor.setEditingShape(shape)
this.editor.select(shape)
return
}
}
}
} else {
if (shape.id === editingShape.id) {
// If we clicked on the editing shape (which isn't a shape with a label), do nothing
} else {
// But if we clicked on a different shape of the same type, edit it instead
this.editor.setEditingShape(shape)
this.editor.select(shape)
}
return
}
} else {
// clicked a different kind of shape
}
break
}
}
// still here? Cancel editing and transition back to select idle
this.parent.transition('idle', info)
this.parent.current.value?.onPointerDown?.(info)
// then feed the pointer down event back into the state chart as if it happened in that state
this.editor.root.handleEvent(info)
}
override onComplete: TLEventHandlers['onComplete'] = (info) => {

1
packages/tldraw/src/lib/tools/SelectTool/children/Idle.ts
View file

@ -23,6 +23,7 @@ export class Idle extends StateNode {
override onEnter = () => {
this.parent.currentToolIdMask = undefined
updateHoveredId(this.editor)
this.editor.updateInstanceState(
{ cursor: { type: 'default', rotation: 0 } },
{ ephemeral: true }

2
packages/tldraw/src/lib/tools/SelectTool/children/PointingShape.ts
View file

@ -144,8 +144,8 @@ export class PointingShape extends StateNode {
this.editor.batch(() => {
this.editor.mark('editing on pointer up')
this.editor.select(selectingShape.id)
this.editor.setCurrentTool('select.editing_shape')
this.editor.setEditingShape(selectingShape.id)
this.editor.setCurrentTool('select.editing_shape')
})
return
}

3
packages/tldraw/src/lib/tools/SelectTool/children/Resizing.ts
View file

@ -109,8 +109,7 @@ export class Resizing extends StateNode {
if (this.editAfterComplete && this.editor.onlySelectedShape) {
this.editor.setEditingShape(this.editor.onlySelectedShape.id)
this.editor.setCurrentTool('select')
this.editor.root.current.value!.transition('editing_shape', {})
this.editor.setCurrentTool('select.editing_shape')
return
}

42
packages/tldraw/src/lib/tools/SelectTool/children/Translating.ts
View file

@ -55,7 +55,23 @@ export class Translating extends StateNode {
this.markId = isCreating ? `creating:${this.editor.onlySelectedShape!.id}` : 'translating'
this.editor.mark(this.markId)
this.handleEnter(info)
this.isCloning = false
this.info = info
this.editor.setCursor({ type: 'move', rotation: 0 })
this.selectionSnapshot = getTranslatingSnapshot(this.editor)
// Don't clone on create; otherwise clone on altKey
if (!this.isCreating) {
if (this.editor.inputs.altKey) {
this.startCloning()
return
}
}
this.snapshot = this.selectionSnapshot
this.handleStart()
this.updateShapes()
this.editor.on('tick', this.updateParent)
}
@ -153,8 +169,7 @@ export class Translating extends StateNode {
const onlySelected = this.editor.onlySelectedShape
if (onlySelected) {
this.editor.setEditingShape(onlySelected.id)
this.editor.setCurrentTool('select')
this.editor.root.current.value!.transition('editing_shape', {})
this.editor.setCurrentTool('select.editing_shape')
}
} else {
this.parent.transition('idle', {})
@ -171,26 +186,6 @@ export class Translating extends StateNode {
}
}
protected handleEnter(info: TLPointerEventInfo & { target: 'shape' }) {
this.isCloning = false
this.info = info
this.editor.setCursor({ type: 'move', rotation: 0 })
this.selectionSnapshot = getTranslatingSnapshot(this.editor)
// Don't clone on create; otherwise clone on altKey
if (!this.isCreating) {
if (this.editor.inputs.altKey) {
this.startCloning()
return
}
}
this.snapshot = this.selectionSnapshot
this.handleStart()
this.updateShapes()
}
protected handleStart() {
const { movingShapes } = this.snapshot
@ -207,6 +202,7 @@ export class Translating extends StateNode {
if (changes.length > 0) {
this.editor.updateShapes(changes)
}
this.editor.setHoveredShape(null)
}
protected handleEnd() {

3
packages/tldraw/src/test/SelectTool.test.ts
View file

@ -394,7 +394,8 @@ describe('When editing shapes', () => {
const shapeId = editor.selectedShapeIds[0]
// Click another text shape
editor.click(50, 50, { target: 'shape', shape: editor.getShape(ids.text1) })
editor.pointerMove(50, 50)
editor.click()
expect(editor.selectedShapeIds.length).toBe(1)
expect(editor.currentPageShapes.length).toBe(5)
expect(editor.getShape(shapeId)).toBe(undefined)

7
packages/tldraw/src/test/selection-omnibus.test.ts
View file

@ -1326,8 +1326,13 @@ describe('When double clicking an editable shape', () => {
})
it('starts editing arrow on double click', () => {
editor.pointerMove(250, 50).doubleClick()
editor.pointerMove(250, 50)
editor.doubleClick()
expect(editor.selectedShapeIds).toEqual([ids.box2])
expect(editor.editingShapeId).toBe(ids.box2)
editor.expectToBeIn('select.editing_shape')
editor.doubleClick()
expect(editor.selectedShapeIds).toEqual([ids.box2])
expect(editor.editingShapeId).toBe(ids.box2)

39
packages/tldraw/src/test/translating.test.ts
View file

@ -171,6 +171,7 @@ describe('When cloning...', () => {
},
])
})
it('clones a single shape and restores when stopping cloning', () => {
expect(editor.currentPageShapeIds.size).toBe(3)
expect(editor.currentPageShapeIds.size).toBe(3)
@ -1783,3 +1784,41 @@ describe('When dragging shapes', () => {
})
})
})
it('clones a single shape simply', () => {
editor
// create a note shape
.setCurrentTool('note')
.pointerMove(50, 50)
.click()
expect(editor.onlySelectedShape).toBe(editor.currentPageShapes[0])
expect(editor.hoveredShape).toBe(editor.currentPageShapes[0])
// click on the canvas to deselect
editor.pointerMove(200, 50).click()
expect(editor.onlySelectedShape).toBe(null)
expect(editor.hoveredShape).toBe(undefined)
// move back over the the shape
editor.pointerMove(50, 50)
expect(editor.onlySelectedShape).toBe(null)
expect(editor.hoveredShape).toBe(editor.currentPageShapes[0])
// start dragging the shape
editor
.pointerDown()
.pointerMove(50, 500)
// start cloning
.keyDown('Alt')
// stop dragging
.pointerUp()
expect(editor.currentPageShapes).toHaveLength(2)
const [, sticky2] = editor.currentPageShapes
expect(editor.onlySelectedShape).toBe(sticky2)
expect(editor.editingShape).toBe(undefined)
expect(editor.hoveredShape).toBe(sticky2)
})