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

Fix arrow handle snapping, snapping to text labels, selection of text labels (#1910)

Browse source 
This PR:
- adds `canSnap` as a property to handle and ignores snapping when
dragging a handle that does not have `canSnap` set to true. Arrows no
longer snap.
- adds `isLabel` to Geometry2d
- fixes selection on empty text labels
- fixes vertices / snapping for empty text labels

### Change Type

- [x] `minor` — New feature

### Test Plan

- [x] Unit Tests
This commit is contained in:
Steve Ruiz 2023-09-18 15:59:27 +01:00 committed by GitHub
parent 5458362829
commit beb9db8eb7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 219 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
packages/editor/api-report.md
View file

@ -989,6 +989,8 @@ export abstract class Geometry2d {
// (undocumented)
isFilled: boolean;
// (undocumented)
isLabel: boolean;
// (undocumented)
isPointInBounds(point: Vec2d, margin?: number): boolean;
// (undocumented)
isSnappable: boolean;
@ -1137,8 +1139,6 @@ export class Group2d extends Geometry2d {
// (undocumented)
nearestPoint(point: Vec2d): Vec2d;
// (undocumented)
get outerVertices(): Vec2d[];
// (undocumented)
toSimpleSvgPath(): string;
}

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

@ -4245,22 +4245,27 @@ export class Editor extends EventEmitter<TLEventMap> {
for (let i = shapesToCheck.length - 1; i >= 0; i--) {
const shape = shapesToCheck[i]
let geometry = this.getShapeGeometry(shape)
const geometry = this.getShapeGeometry(shape)
const isGroup = geometry instanceof Group2d
const pointInShapeSpace = this.getPointInShapeSpace(shape, point)
// Check labels first
if (
this.isShapeOfType<TLArrowShape>(shape, 'arrow') ||
(this.isShapeOfType<TLGeoShape>(shape, 'geo') && shape.props.fill === 'none')
) {
if (hitLabels || shape.props.text.trim()) {
if (shape.props.text.trim()) {
// let's check whether the shape has a label and check that
const labelGeometry = (geometry as Group2d).children[1]
if (labelGeometry && labelGeometry.isPointInBounds(pointInShapeSpace)) {
return shape
for (const childGeometry of (geometry as Group2d).children) {
if (childGeometry.isLabel && childGeometry.isPointInBounds(pointInShapeSpace)) {
return shape
}
}
}
} else if (this.isShapeOfType(shape, 'frame')) {
}
if (this.isShapeOfType(shape, 'frame')) {
// On the rare case that we've hit a frame, test again hitInside to be forced true;
// this prevents clicks from passing through the body of a frame to shapes behhind it.
@ -4286,14 +4291,24 @@ export class Editor extends EventEmitter<TLEventMap> {
continue
}
if (geometry instanceof Group2d) {
// this is kind of a mess at the moment;
// we want to check all of the shapes in the group,
// using the group's own properties to decide how that
geometry = geometry.children[0]
}
let distance: number
const distance = geometry.distanceToPoint(pointInShapeSpace, hitInside)
if (isGroup) {
let minDistance = Infinity
for (const childGeometry of geometry.children) {
if (childGeometry.isLabel && !hitLabels) continue
// hit test the all of the child geometries that aren't labels
const tDistance = childGeometry.distanceToPoint(pointInShapeSpace, hitInside)
if (tDistance < minDistance) {
minDistance = tDistance
}
}
distance = minDistance
} else {
distance = geometry.distanceToPoint(pointInShapeSpace, hitInside)
}
if (geometry.isClosed) {
// For closed shapes, the distance will be positive if outside of
@ -4301,7 +4316,7 @@ export class Editor extends EventEmitter<TLEventMap> {
// is greater than the margin, then it's a miss. Otherwise...
if (distance <= margin) {
if (geometry.isFilled) {
if (geometry.isFilled || (isGroup && geometry.children[0].isFilled)) {
// If the shape is filled, then it's a hit. Remember, we're
// starting from the TOP-MOST shape in z-index order, so any
// other hits would be occluded by the shape.

2
packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts
View file

@ -164,7 +164,7 @@ export function getCurvedArrowInfo(
let intersections = fn(
centerInEndShapeLocalSpace,
handleArc.radius,
editor.getShapeGeometry(endShapeInfo.shape).outerVertices
editor.getShapeGeometry(endShapeInfo.shape).vertices
)
if (intersections) {

3
packages/editor/src/lib/primitives/geometry/Geometry2d.ts
View file

@ -5,6 +5,7 @@ import { pointInPolygon } from '../utils'
export interface Geometry2dOptions {
isFilled: boolean
isClosed: boolean
isLabel?: boolean
isSnappable?: boolean
}
@ -12,12 +13,14 @@ export interface Geometry2dOptions {
export abstract class Geometry2d {
isFilled = false
isClosed = true
isLabel = false
isSnappable = true
constructor(opts: Geometry2dOptions) {
this.isFilled = opts.isFilled
this.isClosed = opts.isClosed
this.isSnappable = opts.isSnappable ?? false
this.isLabel = opts.isLabel ?? false
}
abstract getVertices(): Vec2d[]

17
packages/editor/src/lib/primitives/geometry/Group2d.ts
View file

@ -20,7 +20,7 @@ export class Group2d extends Geometry2d {
}
override getVertices(): Vec2d[] {
return this.children.flatMap((child) => child.vertices)
return this.children.filter((c) => !c.isLabel).flatMap((c) => c.vertices)
}
override nearestPoint(point: Vec2d): Vec2d {
@ -50,22 +50,17 @@ export class Group2d extends Geometry2d {
}
override hitTestPoint(point: Vec2d, margin: number, hitInside: boolean): boolean {
return this.children[0].hitTestPoint(point, margin, hitInside)
return !!this.children
.filter((c) => !c.isLabel)
.find((c) => c.hitTestPoint(point, margin, hitInside))
}
override hitTestLineSegment(A: Vec2d, B: Vec2d, zoom: number): boolean {
const { children } = this
// todo: this is a temporary solution, assuming that the first child defines the group size
return children[0].hitTestLineSegment(A, B, zoom)
}
get outerVertices() {
// todo: this is a temporary solution for arrow hit testing to prevent arrows from snapping to the label of a shape
return this.children[0].vertices
return !!this.children.filter((c) => !c.isLabel).find((c) => c.hitTestLineSegment(A, B, zoom))
}
getArea() {
// todo: this is a temporary solution, assuming that the first child defines the group size
// todo: this is a temporary solution, assuming that the first child defines the group size; we would want to flatten the group and then find the area of the hull polygon
return this.children[0].area
}

1
packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx
View file

@ -157,6 +157,7 @@ export class ArrowShapeUtil extends ShapeUtil<TLArrowShape> {
width: width + 8.5,
height: height + 8.5,
isFilled: true,
isLabel: true,
})
}

2
packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx
View file

@ -321,6 +321,7 @@ export class GeoShapeUtil extends BaseBoxShapeUtil<TLGeoShape> {
const labelSize = getLabelSize(this.editor, shape)
const labelWidth = Math.min(w, Math.max(labelSize.w, Math.min(32, Math.max(1, w - 8))))
const labelHeight = Math.min(h, Math.max(labelSize.h, Math.min(32, Math.max(1, w - 8))))
const lines = getLines(shape.props, strokeWidth)
const edges = lines ? lines.map((line) => new Polyline2d({ points: line })) : []
@ -344,6 +345,7 @@ export class GeoShapeUtil extends BaseBoxShapeUtil<TLGeoShape> {
height: labelHeight,
isFilled: true,
isSnappable: false,
isLabel: true,
}),
...edges,
],

2
packages/tldraw/src/lib/shapes/line/LineShapeUtil.tsx
View file

@ -53,6 +53,7 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
id: 'start',
type: 'vertex',
canBind: false,
canSnap: true,
index: 'a1',
x: 0,
y: 0,
@ -61,6 +62,7 @@ export class LineShapeUtil extends ShapeUtil<TLLineShape> {
id: 'end',
type: 'vertex',
canBind: false,
canSnap: true,
index: 'a2',
x: 0,
y: 0,

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

@ -235,7 +235,7 @@ export class DraggingHandle extends StateNode {
// Clear any existing snaps
editor.snaps.clear()
if (isSnapMode ? !ctrlKey : ctrlKey) {
if (initialHandle.canSnap && (isSnapMode ? !ctrlKey : ctrlKey)) {
// We're snapping
const pageTransform = editor.getShapePageTransform(shape.id)
if (!pageTransform) throw Error('Expected a page transform')

1
packages/tldraw/src/lib/tools/selection-logic/updateHoveredId.ts
View file

@ -4,6 +4,7 @@ export function updateHoveredId(editor: Editor) {
// todo: consider replacing `get hoveredShapeId` with this; it would mean keeping hoveredShapeId in memory rather than in the store and possibly re-computing it more often than necessary
const hitShape = editor.getShapeAtPoint(editor.inputs.currentPagePoint, {
hitInside: false,
hitLabels: false,
margin: HIT_TEST_MARGIN / editor.zoomLevel,
})

6
packages/tldraw/src/test/arrows-megabus.test.ts
View file

@ -245,14 +245,14 @@ describe('When shapes are overlapping', () => {
{ id: ids.box3, type: 'geo', props: { fill: 'solid' } },
])
editor.setCurrentTool('arrow')
editor.pointerDown(0, 50)
editor.pointerDown(0, 50) // over nothing
editor.pointerMove(125, 50) // over box1 only
expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box1 })
editor.pointerMove(175, 50) // box2 is higher
editor.pointerMove(175, 50) // box2 is higher but box1 is filled?
expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box1 })
editor.pointerMove(225, 50) // box3 is higher
expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box3 })
editor.pointerMove(275, 50) // box4 is higher
editor.pointerMove(275, 50) // box4 is higher but box 3 is filled
expect(arrow().props.end).toMatchObject({ boundShapeId: ids.box3 })
})

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

@ -144,6 +144,9 @@ describe('with hitLabels=true', () => {
})
it('hits geo shape label behind overlapping hollow shape', () => {
// label is empty
expect(editor.getShapeAtPoint({ x: 350, y: 350 }, opts)?.id).toBe(ids.box3)
editor.updateShape({ id: ids.box2, type: 'geo', props: { text: 'hello' } })
expect(editor.getShapeAtPoint({ x: 350, y: 350 }, opts)?.id).toBe(ids.box2)
})
})

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

@ -57,9 +57,37 @@ describe('Hovering shapes', () => {
expect(editor.hoveredShapeId).toBe(ids.box1)
editor.pointerMove(75, 75)
expect(editor.hoveredShapeId).toBe(null)
// does not hover the label of a geo shape
// does not hover the label of a geo shape when the label is empty
editor.pointerMove(50, 50)
expect(editor.hoveredShapeId).toBe(null)
editor.updateShape({ id: ids.box1, type: 'geo', props: { text: 'hello' } })
// oh there's text now? hover it
editor.pointerMove(50, 50)
expect(editor.hoveredShapeId).toBe(ids.box1)
})
it('selects a shape with a full label on pointer down', () => {
editor.updateShape({ id: ids.box1, type: 'geo', props: { text: 'hello' } })
editor.pointerMove(50, 50)
editor.pointerDown()
expect(editor.isIn('select.pointing_shape')).toBe(true)
expect(editor.selectedShapes.length).toBe(1)
editor.pointerUp()
expect(editor.selectedShapes.length).toBe(1)
expect(editor.isIn('select.idle')).toBe(true)
})
it('selects a shape with an empty label on pointer up', () => {
editor.pointerMove(50, 50)
editor.pointerDown()
expect(editor.isIn('select.pointing_canvas')).toBe(true)
expect(editor.selectedShapes.length).toBe(0)
editor.pointerUp()
expect(editor.isIn('select.idle')).toBe(true)
expect(editor.selectedShapes.length).toBe(1)
})
it('hovers the margins or inside of filled shapes', () => {

2
packages/tlschema/api-report.md
View file

@ -961,6 +961,8 @@ export type TLGroupShape = TLBaseShape<'group', TLGroupShapeProps>;
export interface TLHandle {
// (undocumented)
canBind?: boolean;
// (undocumented)
canSnap?: boolean;
id: string;
// (undocumented)
index: string;

105
packages/tlschema/src/migrations.test.ts
View file

@ -17,6 +17,7 @@ import { drawShapeMigrations } from './shapes/TLDrawShape'
import { embedShapeMigrations } from './shapes/TLEmbedShape'
import { GeoShapeVersions, geoShapeMigrations } from './shapes/TLGeoShape'
import { imageShapeMigrations } from './shapes/TLImageShape'
import { lineShapeMigrations, lineShapeVersions } from './shapes/TLLineShape'
import { noteShapeMigrations } from './shapes/TLNoteShape'
import { textShapeMigrations } from './shapes/TLTextShape'
import { videoShapeMigrations } from './shapes/TLVideoShape'
@ -1430,6 +1431,110 @@ describe('Renames selectedShapeIds in presence', () => {
})
})
describe('Adding canSnap to line handles', () => {
const { up, down } = lineShapeMigrations.migrators[lineShapeVersions.AddSnapHandles]
test(`up works as expected`, () => {
expect(
up({
props: {
handles: {
start: {
id: 'start',
type: 'vertex',
canBind: false,
index: 'a1',
x: 0,
y: 0,
},
end: {
id: 'end',
type: 'vertex',
canBind: false,
index: 'a2',
x: 100.66015625,
y: -22.07421875,
},
},
},
})
).toEqual({
props: {
handles: {
start: {
id: 'start',
type: 'vertex',
canBind: false,
canSnap: true,
index: 'a1',
x: 0,
y: 0,
},
end: {
id: 'end',
type: 'vertex',
canBind: false,
canSnap: true,
index: 'a2',
x: 100.66015625,
y: -22.07421875,
},
},
},
})
})
test(`down works as expected`, () => {
expect(
down({
props: {
handles: {
start: {
id: 'start',
type: 'vertex',
canBind: false,
canSnap: true,
index: 'a1',
x: 0,
y: 0,
},
end: {
id: 'end',
type: 'vertex',
canBind: false,
canSnap: true,
index: 'a2',
x: 100.66015625,
y: -22.07421875,
},
},
},
})
).toEqual({
props: {
handles: {
start: {
id: 'start',
type: 'vertex',
canBind: false,
index: 'a1',
x: 0,
y: 0,
},
end: {
id: 'end',
type: 'vertex',
canBind: false,
index: 'a2',
x: 100.66015625,
y: -22.07421875,
},
},
},
})
})
})
/* --- PUT YOUR MIGRATIONS TESTS ABOVE HERE --- */
for (const migrator of allMigrators) {

2
packages/tlschema/src/misc/TLHandle.ts
View file

@ -23,6 +23,7 @@ export interface TLHandle {
id: string
type: TLHandleType
canBind?: boolean
canSnap?: boolean
index: string
x: number
y: number
@ -33,6 +34,7 @@ export const handleValidator: T.Validator<TLHandle> = T.object({
id: T.string,
type: T.setEnum(TL_HANDLE_TYPES),
canBind: T.boolean.optional(),
canSnap: T.boolean.optional(),
index: T.string,
x: T.number,
y: T.number,

28
packages/tlschema/src/shapes/TLLineShape.ts
View file

@ -1,4 +1,5 @@
import { defineMigrations } from '@tldraw/store'
import { deepCopy } from '@tldraw/utils'
import { T } from '@tldraw/validate'
import { handleValidator } from '../misc/TLHandle'
import { StyleProp } from '../styles/StyleProp'
@ -32,4 +33,29 @@ export type TLLineShapeProps = ShapePropsType<typeof lineShapeProps>
export type TLLineShape = TLBaseShape<'line', TLLineShapeProps>
/** @internal */
export const lineShapeMigrations = defineMigrations({})
export const lineShapeVersions = {
AddSnapHandles: 1,
} as const
/** @internal */
export const lineShapeMigrations = defineMigrations({
currentVersion: lineShapeVersions.AddSnapHandles,
migrators: {
[lineShapeVersions.AddSnapHandles]: {
up: (record: any) => {
const handles = deepCopy(record.props.handles as Record<string, any>)
for (const id in handles) {
handles[id].canSnap = true
}
return { ...record, props: { ...record.props, handles } }
},
down: (record: any) => {
const handles = deepCopy(record.props.handles as Record<string, any>)
for (const id in handles) {
delete handles[id].canSnap
}
return { ...record, props: { ...record.props, handles } }
},
},
},
})