Remove targeted editing from text (#1962)

This PR has been tested on Mac Chrome, iOS Safari, and Android Chrome.

---
 
This PR removes 'targeted editing' from text.
This affects when you're:
* using the text tool
* editing a text shape
* editing a text label
* editing an arrow label

When in one of these modes, you were able to click on some other text to
immediately start editing it (as long as that text is the same type).

It was a bit broken with some of the newer changes, so this PR removes
it. The issues included:
* selected text 'flashing'
* caret going to the start of the text
* empty text shapes not disappearing
* inconsistent behaviour when clicking near a shape VS on a shape

It feels a bit simpler now too, I like it... 🤔💭 


![2023-09-28 at 15 36 15 - Beige
Parrotfish](https://github.com/tldraw/tldraw/assets/15892272/955e80b7-71d4-4f5d-9647-423dde5f279b)



### Change Type

- [x] `patch` — Bug fix

### Test Plan



- [ ] Unit Tests
- [ ] End to end tests

### Release Notes

- Fixed some cases where text would get selected in the wrong place.
- Changed the behaviour of text selection. Removed 'deep editing'.

---------

Co-authored-by: Steve Ruiz <steveruizok@gmail.com>
This commit is contained in:
Lu Wilson 2023-09-29 16:59:33 +01:00 committed by GitHub
parent 5668209b01
commit b149fe7e98
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 58 additions and 95 deletions

View file

@ -25,9 +25,13 @@ export const ArrowTextLabel = React.memo(function ArrowTextLabel({
handleChange, handleChange,
isEmpty, isEmpty,
handleInputPointerDown, handleInputPointerDown,
handleDoubleClick,
} = useEditableText(id, 'arrow', text) } = useEditableText(id, 'arrow', text)
if (!isEditing && isEmpty) { const finalText = TextHelpers.normalizeTextForDom(text)
const hasText = finalText.trim().length > 0
if (!isEditing && !hasText) {
return null return null
} }
@ -74,6 +78,7 @@ export const ArrowTextLabel = React.memo(function ArrowTextLabel({
onBlur={handleBlur} onBlur={handleBlur}
onContextMenu={stopEventPropagation} onContextMenu={stopEventPropagation}
onPointerDown={handleInputPointerDown} onPointerDown={handleInputPointerDown}
onDoubleClick={handleDoubleClick}
/> />
)} )}
</div> </div>

View file

@ -46,7 +46,6 @@ export const TextLabel = React.memo(function TextLabel<
rInput, rInput,
isEmpty, isEmpty,
isEditing, isEditing,
isEditingSameShapeType,
handleFocus, handleFocus,
handleChange, handleChange,
handleKeyDown, handleKeyDown,
@ -55,13 +54,15 @@ export const TextLabel = React.memo(function TextLabel<
handleDoubleClick, handleDoubleClick,
} = useEditableText(id, type, text) } = useEditableText(id, type, text)
const isInteractive = isEditing || isEditingSameShapeType
const finalText = TextHelpers.normalizeTextForDom(text) const finalText = TextHelpers.normalizeTextForDom(text)
const hasText = finalText.trim().length > 0 const hasText = finalText.trim().length > 0
const legacyAlign = isLegacyAlign(align) const legacyAlign = isLegacyAlign(align)
const theme = useDefaultColorTheme() const theme = useDefaultColorTheme()
if (!isInteractive && !hasText) return null if (!isEditing && !hasText) {
return null
}
return ( return (
<div <div
@ -85,48 +86,46 @@ export const TextLabel = React.memo(function TextLabel<
: {}), : {}),
}} }}
> >
{isEmpty && !isInteractive ? null : ( <div
<div className="tl-text-label__inner"
className="tl-text-label__inner" style={{
style={{ fontSize: LABEL_FONT_SIZES[size],
fontSize: LABEL_FONT_SIZES[size], lineHeight: LABEL_FONT_SIZES[size] * TEXT_PROPS.lineHeight + 'px',
lineHeight: LABEL_FONT_SIZES[size] * TEXT_PROPS.lineHeight + 'px', minHeight: TEXT_PROPS.lineHeight + 32,
minHeight: TEXT_PROPS.lineHeight + 32, minWidth: 0,
minWidth: 0, color: theme[labelColor].solid,
color: theme[labelColor].solid, }}
}} >
> <div className="tl-text tl-text-content" dir="ltr">
<div className="tl-text tl-text-content" dir="ltr"> {finalText}
{finalText}
</div>
{isInteractive && (
<textarea
ref={rInput}
className="tl-text tl-text-input"
name="text"
tabIndex={-1}
autoComplete="false"
autoCapitalize="false"
autoCorrect="false"
autoSave="false"
autoFocus={isEditing}
placeholder=""
spellCheck="true"
wrap="off"
dir="auto"
datatype="wysiwyg"
defaultValue={text}
onFocus={handleFocus}
onChange={handleChange}
onKeyDown={handleKeyDown}
onBlur={handleBlur}
onContextMenu={stopEventPropagation}
onPointerDown={handleInputPointerDown}
onDoubleClick={handleDoubleClick}
/>
)}
</div> </div>
)} {isEditing && (
<textarea
ref={rInput}
className="tl-text tl-text-input"
name="text"
tabIndex={-1}
autoComplete="false"
autoCapitalize="false"
autoCorrect="false"
autoSave="false"
autoFocus
placeholder=""
spellCheck="true"
wrap="off"
dir="auto"
datatype="wysiwyg"
defaultValue={text}
onFocus={handleFocus}
onChange={handleChange}
onKeyDown={handleKeyDown}
onBlur={handleBlur}
onContextMenu={stopEventPropagation}
onPointerDown={handleInputPointerDown}
onDoubleClick={handleDoubleClick}
/>
)}
</div>
</div> </div>
) )
}) })

View file

@ -25,15 +25,6 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
const isEditing = useValue('isEditing', () => editor.editingShapeId === id, [editor, id]) const isEditing = useValue('isEditing', () => editor.editingShapeId === id, [editor, id])
const isEditingSameShapeType = useValue(
'is editing same shape type',
() => {
const { editingShape } = editor
return editingShape && editingShape.type === type
},
[type, id]
)
// If the shape is editing but the input element not focused, focus the element // If the shape is editing but the input element not focused, focus the element
useEffect(() => { useEffect(() => {
const elm = rInput.current const elm = rInput.current
@ -191,17 +182,6 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
const handleInputPointerDown = useCallback( const handleInputPointerDown = useCallback(
(e: React.PointerEvent) => { (e: React.PointerEvent) => {
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
}
editor.dispatch({ editor.dispatch({
...getPointerInfo(e), ...getPointerInfo(e),
type: 'pointer', type: 'pointer',
@ -212,7 +192,7 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
stopEventPropagation(e) // we need to prevent blurring the input stopEventPropagation(e) // we need to prevent blurring the input
}, },
[editor, id, type] [editor, id]
) )
const handleDoubleClick = stopEventPropagation const handleDoubleClick = stopEventPropagation
@ -220,7 +200,6 @@ export function useEditableText<T extends Extract<TLShape, { props: { text: stri
return { return {
rInput, rInput,
isEditing, isEditing,
isEditingSameShapeType,
handleFocus, handleFocus,
handleBlur, handleBlur,
handleKeyDown, handleKeyDown,

View file

@ -78,12 +78,12 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
rInput, rInput,
isEmpty, isEmpty,
isEditing, isEditing,
isEditingSameShapeType,
handleFocus, handleFocus,
handleChange, handleChange,
handleKeyDown, handleKeyDown,
handleBlur, handleBlur,
handleInputPointerDown, handleInputPointerDown,
handleDoubleClick,
} = useEditableText(id, type, text) } = useEditableText(id, type, text)
const zoomLevel = useValue('zoomLevel', () => this.editor.zoomLevel, [this.editor]) const zoomLevel = useValue('zoomLevel', () => this.editor.zoomLevel, [this.editor])
@ -113,7 +113,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
<div className="tl-text tl-text-content" dir="ltr"> <div className="tl-text tl-text-content" dir="ltr">
{text} {text}
</div> </div>
{isEditing || isEditingSameShapeType ? ( {isEditing ? (
<textarea <textarea
ref={rInput} ref={rInput}
className="tl-text tl-text-input" className="tl-text tl-text-input"
@ -137,6 +137,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
onTouchEnd={stopEventPropagation} onTouchEnd={stopEventPropagation}
onContextMenu={stopEventPropagation} onContextMenu={stopEventPropagation}
onPointerDown={handleInputPointerDown} onPointerDown={handleInputPointerDown}
onDoubleClick={handleDoubleClick}
/> />
) : null} ) : null}
</div> </div>

View file

@ -1,4 +1,4 @@
import { StateNode, TLEventHandlers, TLGroupShape, TLTextShape } from '@tldraw/editor' import { StateNode, TLEventHandlers } from '@tldraw/editor'
import { updateHoveredId } from '../../../tools/selection-logic/updateHoveredId' import { updateHoveredId } from '../../../tools/selection-logic/updateHoveredId'
export class Idle extends StateNode { export class Idle extends StateNode {
@ -14,26 +14,6 @@ export class Idle extends StateNode {
} }
override onPointerDown: TLEventHandlers['onPointerDown'] = (info) => { override onPointerDown: TLEventHandlers['onPointerDown'] = (info) => {
const { hoveredShape } = this.editor
const hitShape =
hoveredShape && !this.editor.isShapeOfType<TLGroupShape>(hoveredShape, 'group')
? hoveredShape
: this.editor.getShapeAtPoint(this.editor.inputs.currentPagePoint)
if (hitShape) {
if (this.editor.isShapeOfType<TLTextShape>(hitShape, 'text')) {
requestAnimationFrame(() => {
this.editor.setSelectedShapes([hitShape.id])
this.editor.setEditingShape(hitShape.id)
this.editor.setCurrentTool('select.editing_shape', {
...info,
target: 'shape',
shape: hitShape,
})
})
return
}
}
this.parent.transition('pointing', info) this.parent.transition('pointing', info)
} }

View file

@ -84,8 +84,7 @@ export class EditingShape extends StateNode {
// If we clicked on the editing geo / arrow shape's label, do nothing // If we clicked on the editing geo / arrow shape's label, do nothing
return return
} else { } else {
this.editor.setEditingShape(shape) this.parent.transition('pointing_shape', info)
this.editor.select(shape)
return return
} }
} }
@ -98,9 +97,9 @@ export class EditingShape extends StateNode {
} }
// If we clicked on the editing shape (which isn't a shape with a label), do nothing // If we clicked on the editing shape (which isn't a shape with a label), do nothing
} else { } else {
// But if we clicked on a different shape of the same type, edit it instead // But if we clicked on a different shape of the same type, transition to pointing_shape instead
this.editor.setEditingShape(shape) this.parent.transition('pointing_shape', info)
this.editor.select(shape) return
} }
return return
} }