diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index ed856f588..7e2b599ec 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -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; } diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 66b4a5f7b..dd3b4548b 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -4245,22 +4245,27 @@ export class Editor extends EventEmitter { 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(shape, 'arrow') || (this.isShapeOfType(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 { 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 { // 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. diff --git a/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts b/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts index 5e300f23a..a84621eb9 100644 --- a/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts +++ b/packages/editor/src/lib/editor/shapes/shared/arrow/curved-arrow.ts @@ -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) { diff --git a/packages/editor/src/lib/primitives/geometry/Geometry2d.ts b/packages/editor/src/lib/primitives/geometry/Geometry2d.ts index b80b85601..5e91dc697 100644 --- a/packages/editor/src/lib/primitives/geometry/Geometry2d.ts +++ b/packages/editor/src/lib/primitives/geometry/Geometry2d.ts @@ -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[] diff --git a/packages/editor/src/lib/primitives/geometry/Group2d.ts b/packages/editor/src/lib/primitives/geometry/Group2d.ts index 9dbda1a80..7d7600605 100644 --- a/packages/editor/src/lib/primitives/geometry/Group2d.ts +++ b/packages/editor/src/lib/primitives/geometry/Group2d.ts @@ -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 } diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx index 507b2eef1..502376b58 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx @@ -157,6 +157,7 @@ export class ArrowShapeUtil extends ShapeUtil { width: width + 8.5, height: height + 8.5, isFilled: true, + isLabel: true, }) } diff --git a/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx b/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx index 848856b66..24566347b 100644 --- a/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/geo/GeoShapeUtil.tsx @@ -321,6 +321,7 @@ export class GeoShapeUtil extends BaseBoxShapeUtil { 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 { height: labelHeight, isFilled: true, isSnappable: false, + isLabel: true, }), ...edges, ], diff --git a/packages/tldraw/src/lib/shapes/line/LineShapeUtil.tsx b/packages/tldraw/src/lib/shapes/line/LineShapeUtil.tsx index 9408dd100..370235c05 100644 --- a/packages/tldraw/src/lib/shapes/line/LineShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/line/LineShapeUtil.tsx @@ -53,6 +53,7 @@ export class LineShapeUtil extends ShapeUtil { id: 'start', type: 'vertex', canBind: false, + canSnap: true, index: 'a1', x: 0, y: 0, @@ -61,6 +62,7 @@ export class LineShapeUtil extends ShapeUtil { id: 'end', type: 'vertex', canBind: false, + canSnap: true, index: 'a2', x: 0, y: 0, diff --git a/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts index 5dd7f0711..fdff89276 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/children/DraggingHandle.ts @@ -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') diff --git a/packages/tldraw/src/lib/tools/selection-logic/updateHoveredId.ts b/packages/tldraw/src/lib/tools/selection-logic/updateHoveredId.ts index 082a69ef8..390977744 100644 --- a/packages/tldraw/src/lib/tools/selection-logic/updateHoveredId.ts +++ b/packages/tldraw/src/lib/tools/selection-logic/updateHoveredId.ts @@ -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, }) diff --git a/packages/tldraw/src/test/arrows-megabus.test.ts b/packages/tldraw/src/test/arrows-megabus.test.ts index 6ee58ca05..49a507f17 100644 --- a/packages/tldraw/src/test/arrows-megabus.test.ts +++ b/packages/tldraw/src/test/arrows-megabus.test.ts @@ -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 }) }) diff --git a/packages/tldraw/src/test/getShapeAtPoint.test.ts b/packages/tldraw/src/test/getShapeAtPoint.test.ts index 69b2f07c8..266b86db6 100644 --- a/packages/tldraw/src/test/getShapeAtPoint.test.ts +++ b/packages/tldraw/src/test/getShapeAtPoint.test.ts @@ -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) }) }) diff --git a/packages/tldraw/src/test/selection-omnibus.test.ts b/packages/tldraw/src/test/selection-omnibus.test.ts index 6a9dd9489..f5d0e7f33 100644 --- a/packages/tldraw/src/test/selection-omnibus.test.ts +++ b/packages/tldraw/src/test/selection-omnibus.test.ts @@ -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', () => { diff --git a/packages/tlschema/api-report.md b/packages/tlschema/api-report.md index 0546e12c1..190712b85 100644 --- a/packages/tlschema/api-report.md +++ b/packages/tlschema/api-report.md @@ -961,6 +961,8 @@ export type TLGroupShape = TLBaseShape<'group', TLGroupShapeProps>; export interface TLHandle { // (undocumented) canBind?: boolean; + // (undocumented) + canSnap?: boolean; id: string; // (undocumented) index: string; diff --git a/packages/tlschema/src/migrations.test.ts b/packages/tlschema/src/migrations.test.ts index 15942ddec..ce642475c 100644 --- a/packages/tlschema/src/migrations.test.ts +++ b/packages/tlschema/src/migrations.test.ts @@ -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) { diff --git a/packages/tlschema/src/misc/TLHandle.ts b/packages/tlschema/src/misc/TLHandle.ts index 9780b356f..2fcb04939 100644 --- a/packages/tlschema/src/misc/TLHandle.ts +++ b/packages/tlschema/src/misc/TLHandle.ts @@ -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 = 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, diff --git a/packages/tlschema/src/shapes/TLLineShape.ts b/packages/tlschema/src/shapes/TLLineShape.ts index 3a07b4bc9..7b7033d99 100644 --- a/packages/tlschema/src/shapes/TLLineShape.ts +++ b/packages/tlschema/src/shapes/TLLineShape.ts @@ -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 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) + 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) + for (const id in handles) { + delete handles[id].canSnap + } + return { ...record, props: { ...record.props, handles } } + }, + }, + }, +})