Fix text resizing with alt key (#3632)

This PR fixes a bug where alt-dragging the left or right handles of a
text shape would not produce the correct outcome: the width would double
but the center would change.

![Kapture 2024-04-28 at 13 48
52](https://github.com/tldraw/tldraw/assets/23072548/ad339a57-4efd-4201-86bc-c03a379f7e0c)

This is because the text shape is aspect ratio locked only when dragging
handles other than the left or right, but we didn't have the ability to
differentiate between that. We've had to add that optionality in,
together with a hard-coded override of the normal behavior for text
shapes.

### Change Type

- [x] `sdk` — Changes the tldraw SDK
- [x] `bugfix` — Bug fix

### Test Plan

1. Resize text.
2. Resize text with the alt key held.

- [x] Unit Tests

### Release Notes

- Fixed a bug with resizing text shapes from the left and right while
holding alt.
pull/3640/head
Steve Ruiz 2024-04-29 11:43:02 +01:00 zatwierdzone przez GitHub
rodzic 0d0d38361d
commit 79ca14454e
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: B5690EEEBB952194
7 zmienionych plików z 159 dodań i 42 usunięć

Wyświetl plik

@ -2505,6 +2505,7 @@ export type TLResizeShapeOptions = Partial<{
initialBounds: Box;
initialPageTransform: MatLike;
initialShape: TLShape;
isAspectRatioLocked: boolean;
mode: TLResizeMode;
scaleAxisRotation: number;
scaleOrigin: VecLike;

Wyświetl plik

@ -146,6 +146,7 @@ export type TLResizeShapeOptions = Partial<{
initialShape: TLShape
initialPageTransform: MatLike
dragHandle: TLResizeHandle
isAspectRatioLocked: boolean
mode: TLResizeMode
}>
@ -5391,6 +5392,7 @@ export class Editor extends EventEmitter<TLEventMap> {
initialPageTransform,
initialShape: shape,
mode: 'scale_shape',
isAspectRatioLocked: this.getShapeUtil(shape).isAspectRatioLocked(shape),
scaleOrigin: scaleOriginPage,
scaleAxisRotation: 0,
}
@ -5915,6 +5917,7 @@ export class Editor extends EventEmitter<TLEventMap> {
this.resizeShape(shape.id, scale, {
initialBounds: bounds,
scaleOrigin: new Vec(pageBounds.center.x, commonBounds.minY),
isAspectRatioLocked: this.getShapeUtil(shape).isAspectRatioLocked(shape),
scaleAxisRotation: 0,
})
}
@ -5938,6 +5941,7 @@ export class Editor extends EventEmitter<TLEventMap> {
this.resizeShape(shape.id, scale, {
initialBounds: bounds,
scaleOrigin: new Vec(commonBounds.minX, pageBounds.center.y),
isAspectRatioLocked: this.getShapeUtil(shape).isAspectRatioLocked(shape),
scaleAxisRotation: 0,
})
}
@ -5991,6 +5995,10 @@ export class Editor extends EventEmitter<TLEventMap> {
if (!initialBounds) return this
const isAspectRatioLocked =
options.isAspectRatioLocked ??
this.getShapeUtil(initialShape).isAspectRatioLocked(initialShape)
if (!areAnglesCompatible(pageRotation, scaleAxisRotation)) {
// shape is awkwardly rotated, keep the aspect ratio locked and adopt the scale factor
// from whichever axis is being scaled the least, to avoid the shape getting bigger
@ -6002,13 +6010,14 @@ export class Editor extends EventEmitter<TLEventMap> {
scaleOrigin,
scaleAxisRotation,
initialPageTransform: pageTransform,
isAspectRatioLocked,
initialShape,
})
}
const util = this.getShapeUtil(initialShape)
if (util.isAspectRatioLocked(initialShape)) {
if (isAspectRatioLocked) {
if (Math.abs(scale.x) > Math.abs(scale.y)) {
scale = new Vec(scale.x, Math.sign(scale.y) * Math.abs(scale.x))
} else {
@ -6128,6 +6137,7 @@ export class Editor extends EventEmitter<TLEventMap> {
scaleOrigin: VecLike
scaleAxisRotation: number
initialShape: TLShape
isAspectRatioLocked: boolean
initialPageTransform: MatLike
}
) {
@ -6151,6 +6161,7 @@ export class Editor extends EventEmitter<TLEventMap> {
this.resizeShape(id, shapeScale, {
initialShape: options.initialShape,
initialBounds: options.initialBounds,
isAspectRatioLocked: options.isAspectRatioLocked,
})
// then if the shape is flipped in one axis only, we need to apply an extra rotation

Wyświetl plik

@ -66,7 +66,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
override canEdit = () => true
override isAspectRatioLocked: TLShapeUtilFlag<TLTextShape> = () => true
override isAspectRatioLocked: TLShapeUtilFlag<TLTextShape> = () => true // WAIT NO THIS IS HARD CODED IN THE RESIZE HANDLER
component(shape: TLTextShape) {
const {
@ -136,7 +136,7 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
}
override onResize: TLOnResizeHandler<TLTextShape> = (shape, info) => {
const { initialBounds, initialShape, scaleX, handle } = info
const { newPoint, initialBounds, initialShape, scaleX, handle } = info
if (info.mode === 'scale_shape' || (handle !== 'right' && handle !== 'left')) {
return {
@ -145,25 +145,9 @@ export class TextShapeUtil extends ShapeUtil<TLTextShape> {
...resizeScaled(shape, info),
}
} else {
const prevWidth = initialBounds.width
let nextWidth = prevWidth * scaleX
const offset = new Vec(0, 0)
nextWidth = Math.max(1, Math.abs(nextWidth))
if (handle === 'left') {
offset.x = prevWidth - nextWidth
if (scaleX < 0) {
offset.x += nextWidth
}
} else {
if (scaleX < 0) {
offset.x -= nextWidth
}
}
const { x, y } = offset.rot(shape.rotation).add(initialShape)
const nextWidth = Math.max(1, Math.abs(initialBounds.width * scaleX))
const { x, y } =
scaleX < 0 ? Vec.Sub(newPoint, Vec.FromAngle(shape.rotation).mul(nextWidth)) : newPoint
return {
id: shape.id,

Wyświetl plik

@ -48,7 +48,7 @@ export class Pointing extends StateNode {
target: 'selection',
handle: 'right',
isCreating: true,
creationCursorOffset: { x: 1, y: 1 },
creationCursorOffset: { x: 18, y: 1 },
onInteractionEnd: 'text',
onCreate: () => {
this.editor.setEditingShape(shape.id)

Wyświetl plik

@ -13,6 +13,7 @@ import {
TLShape,
TLShapeId,
TLShapePartial,
TLTextShape,
Vec,
VecLike,
areAnglesCompatible,
@ -175,7 +176,14 @@ export class Resizing extends StateNode {
canShapesDeform,
} = this.snapshot
const isAspectRatioLocked = shiftKey || !canShapesDeform
let isAspectRatioLocked = shiftKey || !canShapesDeform
if (shapeSnapshots.size === 1) {
const onlySnapshot = [...shapeSnapshots.values()][0]!
if (this.editor.isShapeOfType<TLTextShape>(onlySnapshot.shape, 'text')) {
isAspectRatioLocked = !(this.info.handle === 'left' || this.info.handle === 'right')
}
}
// first negate the 'cursor handle offset'
// we need to do this because we do grid snapping based on the page point of the handle
@ -216,6 +224,7 @@ export class Resizing extends StateNode {
.clone()
.sub(cursorHandleOffset)
.sub(this.creationCursorOffset)
const originPagePoint = this.editor.inputs.originPagePoint.clone().sub(cursorHandleOffset)
if (this.editor.getInstanceState().isGridMode && !ctrlKey) {
@ -253,6 +262,8 @@ export class Resizing extends StateNode {
// calculate the scale by measuring the current distance between the drag handle and the scale origin
// and dividing by the original distance between the drag handle and the scale origin
// bug: for edges, the page point doesn't matter, the
const distanceFromScaleOriginNow = Vec.Sub(currentPagePoint, scaleOriginPage).rot(
-selectionRotation
)
@ -316,6 +327,7 @@ export class Resizing extends StateNode {
? 'resize_bounds'
: 'scale_shape',
scaleOrigin: scaleOriginPage,
isAspectRatioLocked,
scaleAxisRotation: selectionRotation,
})
}

Wyświetl plik

@ -8,6 +8,7 @@ import {
TLSelectionHandle,
TLShapeId,
TLShapePartial,
TLTextShape,
Vec,
canonicalizeRotation,
createShapeId,
@ -3917,3 +3918,111 @@ describe('When resizing near the edges of the screen', () => {
expect(after.props.h).toBeGreaterThan(before.props.h)
})
})
describe('resizing text with autosize true', () => {
it('resizes text from the right side', () => {
editor.createShape<TLTextShape>({
type: 'text',
x: 0,
y: 0,
props: {
text: 'Hello',
autoSize: false,
w: 200,
},
})
const shape = editor.getLastCreatedShape()
const bounds = editor.getShapePageBounds(shape.id)!
editor
.select(shape)
.pointerDown(bounds.maxX, bounds.midY, { target: 'selection', handle: 'right' }) // right edge
.expectToBeIn('select.pointing_resize_handle')
.pointerMove(bounds.maxX + 100, bounds.midY)
.expectToBeIn('select.resizing')
.expectShapeToMatch({ ...shape, x: 0, y: 0, props: { w: 300 } })
.pointerMove(bounds.maxX - 10, bounds.midY)
.expectShapeToMatch({ ...shape, x: 0, y: 0, props: { w: 190 } })
})
it('resizes text from the right side when alt key is pressed', () => {
editor.createShape<TLTextShape>({
type: 'text',
x: 0,
y: 0,
props: {
text: 'Hello',
autoSize: false,
w: 200,
},
})
const shape = editor.getLastCreatedShape()
const bounds = editor.getShapePageBounds(shape.id)!
editor
.select(shape)
.keyDown('Alt')
.pointerDown(bounds.maxX, bounds.midY, { target: 'selection', handle: 'right' }) // right edge
.expectToBeIn('select.pointing_resize_handle')
.pointerMove(bounds.maxX + 100, bounds.midY)
.expectToBeIn('select.resizing')
.expectShapeToMatch({ ...shape, x: -100, y: 0, props: { w: 400 } })
.pointerMove(bounds.maxX - 10, bounds.midY)
.expectShapeToMatch({ ...shape, x: 10, y: 0, props: { w: 180 } })
})
it('resizes text from the left side', () => {
editor.createShape<TLTextShape>({
type: 'text',
x: 0,
y: 0,
props: {
text: 'Hello',
autoSize: false,
w: 200,
},
})
const shape = editor.getLastCreatedShape()
const bounds = editor.getShapePageBounds(shape.id)!
editor
.select(shape)
.pointerDown(bounds.minX, bounds.midY, { target: 'selection', handle: 'left' }) // right edge
.expectToBeIn('select.pointing_resize_handle')
.pointerMove(bounds.minX - 100, bounds.midY)
.expectToBeIn('select.resizing')
.expectShapeToMatch({ ...shape, x: -100, y: 0, props: { w: 300 } })
.pointerMove(bounds.minX + 10, bounds.midY)
.expectShapeToMatch({ ...shape, x: 10, y: 0, props: { w: 190 } })
})
it('resizes text from the left side when alt is pressed', () => {
editor.createShape<TLTextShape>({
type: 'text',
x: 0,
y: 0,
props: {
text: 'Hello',
autoSize: false,
w: 200,
},
})
const shape = editor.getLastCreatedShape()
const bounds = editor.getShapePageBounds(shape.id)!
editor
.select(shape)
.keyDown('Alt')
.pointerDown(bounds.minX, bounds.midY, { target: 'selection', handle: 'left' }) // right edge
.expectToBeIn('select.pointing_resize_handle')
.pointerMove(bounds.minX - 100, bounds.midY)
.expectToBeIn('select.resizing')
.expectShapeToMatch({ ...shape, x: -100, y: 0, props: { w: 400 } })
.pointerMove(bounds.minX + 10, bounds.midY)
.expectShapeToMatch({ ...shape, x: 10, y: 0, props: { w: 180 } })
})
})

Wyświetl plik

@ -245,25 +245,25 @@ describe('When changing text size', () => {
expect(boundsA!.maxX).toEqual(boundsB!.maxX)
expect(boundsA!.maxY).not.toEqual(boundsB!.maxY)
})
})
it('preserves the top left when the text has text', () => {
const x = 0
const y = 0
const id = createShapeId()
editor.createShapes([
{
id,
type: 'text',
x: 0,
y: 0,
props: {
text: 'Hello',
it('preserves the top left when the text has text', () => {
const x = 0
const y = 0
const id = createShapeId()
editor.createShapes([
{
id,
type: 'text',
x: 0,
y: 0,
props: {
text: 'Hello',
},
},
},
])
expect(editor.getShape(id)).toMatchObject({
x,
y,
])
expect(editor.getShape(id)).toMatchObject({
x,
y,
})
})
})