Add frame selection overhaul for grid shapes#8
Conversation
PR SummaryMedium Risk Overview Adjusts frame hit-testing so Written by Cursor Bugbot for commit ae9bec3. This will update automatically on new commits. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Grid body drag check ignores outermost selectable shape
- Added selectingShape field to store the outermost selectable shape in onEnter, and updated onPointerMove to check selectingShape.type instead of hitShape.type.
- ✅ Fixed: Shift+Enter drops top-level shapes from mixed selections
- Updated Shift+Enter logic to preserve shapes that can't scope out (those without grid/group parents) by adding them to the new selection set.
Or push these changes by commenting:
@cursor push 54ff0664d4
Preview (54ff0664d4)
diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts
--- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts
+++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts
@@ -525,18 +525,19 @@
const selectedShapes = this.editor.getSelectedShapes()
if (info.shiftKey) {
- // Shift+Enter: scope out — select the parent grid/group of the current selection
- const parentIds = new Set<TLShapeId>()
+ // Shift+Enter: scope out — select the parent grid/group of the current selection,
+ // preserving shapes that can't scope out (top-level shapes without a grid/group parent)
+ const newSelection = new Set<TLShapeId>()
for (const shape of selectedShapes) {
const parent = this.editor.getShape(shape.parentId)
- if (
- parent &&
- ((parent as any).type === 'grid' || (parent as any).type === 'group')
- )
- parentIds.add(parent.id)
+ if (parent && ((parent as any).type === 'grid' || (parent as any).type === 'group')) {
+ newSelection.add(parent.id)
+ } else {
+ newSelection.add(shape.id)
+ }
}
- if (parentIds.size > 0) {
- this.editor.setSelectedShapes(Array.from(parentIds))
+ if (newSelection.size > 0) {
+ this.editor.setSelectedShapes(Array.from(newSelection))
}
return
}
@@ -544,14 +545,12 @@
// Enter: scope in — expand any grid/group shapes to their children, keep leaf shapes
if (
selectedShapes.some(
- (shape) =>
- this.editor.isShapeOfType(shape, 'group') || (shape as any).type === 'grid'
+ (shape) => this.editor.isShapeOfType(shape, 'group') || (shape as any).type === 'grid'
)
) {
this.editor.setSelectedShapes(
selectedShapes.flatMap((shape) =>
- this.editor.isShapeOfType(shape, 'group') ||
- (shape as any).type === 'grid'
+ this.editor.isShapeOfType(shape, 'group') || (shape as any).type === 'grid'
? this.editor.getSortedChildIdsForParent(shape.id)
: [shape.id]
)
diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts
--- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts
+++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts
@@ -7,6 +7,7 @@
hitShape = {} as TLShape
hitShapeForPointerUp = {} as TLShape
+ selectingShape = {} as TLShape
isDoubleClick = false
didCtrlOnEnter = false
@@ -21,6 +22,7 @@
const { shiftKey, altKey, accelKey } = info
this.hitShape = info.shape
+ this.selectingShape = this.editor.getOutermostSelectableShape(info.shape)
this.isDoubleClick = false
this.didCtrlOnEnter = accelKey
this.didHitGridLabel = false
@@ -36,8 +38,7 @@
}
}
}
- const outermostSelectingShape = this.editor.getOutermostSelectableShape(info.shape)
- const selectedAncestor = this.editor.findShapeAncestor(outermostSelectingShape, (parent) =>
+ const selectedAncestor = this.editor.findShapeAncestor(this.selectingShape, (parent) =>
selectedShapeIds.includes(parent.id)
)
@@ -46,9 +47,9 @@
// If the shape has an onClick handler
this.editor.getShapeUtil(info.shape).onClick ||
// ...or if the shape is the focused layer (e.g. group)
- outermostSelectingShape.id === focusedGroupId ||
+ this.selectingShape.id === focusedGroupId ||
// ...or if the shape is within the selection
- selectedShapeIds.includes(outermostSelectingShape.id) ||
+ selectedShapeIds.includes(this.selectingShape.id) ||
// ...or if an ancestor of the shape is selected
selectedAncestor ||
// ...or if the current point is NOT within the selection bounds
@@ -56,7 +57,7 @@
) {
// We won't select the shape on enter, though we might select it on pointer up!
this.didSelectOnEnter = false
- this.hitShapeForPointerUp = outermostSelectingShape
+ this.hitShapeForPointerUp = this.selectingShape
return
}
@@ -64,13 +65,13 @@
if (shiftKey && !altKey) {
this.editor.cancelDoubleClick()
- if (!selectedShapeIds.includes(outermostSelectingShape.id)) {
+ if (!selectedShapeIds.includes(this.selectingShape.id)) {
this.editor.markHistoryStoppingPoint('shift selecting shape')
- this.editor.setSelectedShapes([...selectedShapeIds, outermostSelectingShape.id])
+ this.editor.setSelectedShapes([...selectedShapeIds, this.selectingShape.id])
}
} else {
this.editor.markHistoryStoppingPoint('selecting shape')
- this.editor.setSelectedShapes([outermostSelectingShape.id])
+ this.editor.setSelectedShapes([this.selectingShape.id])
}
}
@@ -231,7 +232,11 @@
if (this.didCtrlOnEnter) {
this.parent.transition('brushing', info)
- } else if (this.didSelectOnEnter && (this.hitShape as any).type === 'grid' && !this.didHitGridLabel) {
+ } else if (
+ this.didSelectOnEnter &&
+ (this.selectingShape as any).type === 'grid' &&
+ !this.didHitGridLabel
+ ) {
this.editor.setSelectedShapes([])
this.parent.transition('brushing', { ...info, target: 'canvas' as const })
} else {- Grid body returns shape from getShapeAtPoint (clickable/hoverable body) - Disable margin bleed on grid label hitbox - Brush selection requires full enclosure for grid shapes - Deselect children when grid frame is fully enclosed by brush - Enter scopes into grid/group children, Shift+Enter scopes out - Drag from unselected grid body starts brush, drag from title moves frame - EditingShape exits on grid body click - 60% opacity on hover indicators, 60% during brush selection Made-with: Cursor
347a684 to
657c574
Compare
This reverts commit 25a543a.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Shift+drag from grid body loses prior selection
- Added selectedShapeIdsOnEnter property to store original selection before modifications, then restore it (instead of clearing to empty array) when transitioning to brushing from grid body.
Or push these changes by commenting:
@cursor push 16947a71d9
Preview (16947a71d9)
diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts
--- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts
+++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingShape.ts
@@ -1,4 +1,4 @@
-import { Group2d, StateNode, TLPointerEventInfo, TLShape } from '@tldraw/editor'
+import { Group2d, StateNode, TLPointerEventInfo, TLShape, TLShapeId } from '@tldraw/editor'
import { isOverArrowLabel } from '../../../shapes/arrow/arrowLabel'
import { getTextLabels } from '../../../utils/shapes/shapes'
@@ -12,6 +12,7 @@
didCtrlOnEnter = false
didSelectOnEnter = false
didHitGridLabel = false
+ selectedShapeIdsOnEnter: TLShapeId[] = []
override onEnter(info: TLPointerEventInfo & { target: 'shape' }) {
const selectedShapeIds = this.editor.getSelectedShapeIds()
@@ -61,6 +62,7 @@
}
this.didSelectOnEnter = true
+ this.selectedShapeIdsOnEnter = selectedShapeIds
if (shiftKey && !altKey) {
this.editor.cancelDoubleClick()
@@ -231,8 +233,12 @@
if (this.didCtrlOnEnter) {
this.parent.transition('brushing', info)
- } else if (this.didSelectOnEnter && (this.hitShape as any).type === 'grid' && !this.didHitGridLabel) {
- this.editor.setSelectedShapes([])
+ } else if (
+ this.didSelectOnEnter &&
+ (this.hitShape as any).type === 'grid' &&
+ !this.didHitGridLabel
+ ) {
+ this.editor.setSelectedShapes(this.selectedShapeIdsOnEnter)
this.parent.transition('brushing', { ...info, target: 'canvas' as const })
} else {
this.startTranslating(info)| this.parent.transition('brushing', info) | ||
| } else if (this.didSelectOnEnter && (this.hitShape as any).type === 'grid' && !this.didHitGridLabel) { | ||
| this.editor.setSelectedShapes([]) | ||
| this.parent.transition('brushing', { ...info, target: 'canvas' as const }) |
There was a problem hiding this comment.
Shift+drag from grid body loses prior selection
Medium Severity
setSelectedShapes([]) unconditionally clears the entire selection before transitioning to brushing. When shift+click+dragging from an unselected grid body, any previously selected shapes are lost. Before this change, the same interaction hit the canvas path (PointingCanvas), which preserves selection when shift is held and passes it to Brushing as initialSelectedShapeIds. The new grid body path always zeroes out the selection, so Brushing.initialSelectedShapeIds becomes empty and shift-based additive brush selection no longer works from grid bodies.



Made-with: Cursor
Frame Selection Overhaul (Figma Section Behavior)
Reworks frame/grid shape selection to match Figma's section interaction model. Frames are now stateful containers: when unselected, interactions target children; when selected, interactions target the frame.
Selection
Click frame title: selects the frame (was broken — spatial index filtered out grid shapes before label hit-test ran)
Click empty area inside frame: selects the frame (grid body now returns from getShapeAtPoint)
Click asset inside frame: selects the asset, deselects frame if it was selected
Double-click title: enters edit mode for frame name (was broken for same reason as click)
Movement
Drag from title (unselected frame): selects and moves the frame
Drag from empty body (unselected frame): starts rect selection of children
Drag from empty body (selected frame): moves the frame
Rect Selection
Frames require full enclosure: partial rect selection overlap only selects intersecting children, not the frame itself (previously grid shapes were selected by intersection like regular shapes)
Child deselection on enclosure: when the rect selection fully encloses a frame, the frame is selected and all its children are removed from the selection
Recursive nesting: works correctly for nested frames — inner frame's children deselect when inner frame is enclosed, then inner frame deselects when outer frame is enclosed
Hover Indicators
60% opacity on hover: hover stroke is now 60% opacity for both assets and frames, solidifying to 100% on selection
60% during rect selection: shapes show 60% stroke while rect-selecting, solidifying to 100% on mouse release
Frame body hover: frames now show hover indicator when hovering empty body area (previously only showed on title hover)
Keyboard Scope Navigation
Enter: selects all direct children of selected frame(s), expanding one level. Non-frame siblings stay selected while frames expand to their children. Repeated presses drill deeper.
Shift+Enter: selects the parent frame of the current selection (scope out one level)
What to Test
Click empty area inside unselected frame — frame selects. Click asset inside it — asset selects, frame deselects.
Drag from body of unselected frame — selection rectangle appears, selects children by intersection
Drag from title of unselected frame — moves the frame
Drag from body of selected frame — moves the frame
Rect Select across a frame — children select as selection rect intersects them, then when selection rect fully encloses the frame, children drop and only the frame is selected
Nested frames: repeat selection rect test — inner frame's children drop first, then inner frame drops when outer is enclosed
Hover any shape — 60% stroke. Click to select — 100% stroke.
Enter on selected frame — selects children. Shift+Enter — selects parent frame.
Enter with mixed selection (assets + child frame) — assets stay, child frame expands to its children