[fix] reparenting locked shapes (#2070)

This PR allows locked shapes to be reparented. This fixes a bug where
grouped and locked shapes would disappear when their parent group would
be ungrouped.

### Change Type

- [x] `patch` — Bug fix

### Test Plan

1. Create two shapes.
2. Group the shapes.
3. Lock one shape.
4. Ungroup the group

- [x] Unit Tests

### Release Notes

- Fix a bug where grouped locked shapes would be deleted when ungrouped.
This commit is contained in:
Steve Ruiz 2023-10-13 12:18:26 +01:00 committed by GitHub
parent 01d46bbe22
commit bfb61b12ad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 9 deletions

View file

@ -4787,16 +4787,25 @@ export class Editor extends EventEmitter<TLEventMap> {
const invertedParentTransform = parentTransform.clone().invert() const invertedParentTransform = parentTransform.clone().invert()
let id: TLShapeId const shapesToReparent = compact(ids.map((id) => this.getShape(id)))
for (let i = 0; i < ids.length; i++) {
id = ids[i] // The user is allowed to re-parent locked shapes. Unintutive? Yeah! But there are plenty of
const shape = this.getShape(id) // times when a locked shape's parent is deleted... and we need to put that shape somewhere!
if (!shape) continue const lockedShapes = shapesToReparent.filter((shape) => shape.isLocked)
if (lockedShapes.length) {
// If we have locked shapes, unlock them before we update them
this.updateShapes(lockedShapes.map(({ id, type }) => ({ id, type, isLocked: false })))
}
for (let i = 0; i < shapesToReparent.length; i++) {
const shape = shapesToReparent[i]
const pageTransform = this.getShapePageTransform(shape)! const pageTransform = this.getShapePageTransform(shape)!
if (!pageTransform) continue if (!pageTransform) continue
const pagePoint = pageTransform.point()
if (!shape || !pagePoint) continue const pagePoint = pageTransform.point()
if (!pagePoint) continue
const newPoint = invertedParentTransform.applyToPoint(pagePoint) const newPoint = invertedParentTransform.applyToPoint(pagePoint)
const newRotation = pageTransform.rotation() - parentPageRotation const newRotation = pageTransform.rotation() - parentPageRotation
@ -4809,10 +4818,12 @@ export class Editor extends EventEmitter<TLEventMap> {
y: newPoint.y, y: newPoint.y,
rotation: newRotation, rotation: newRotation,
index: indices[i], index: indices[i],
isLocked: shape.isLocked, // this will re-lock locked shapes
}) })
} }
this.updateShapes(changes) this.updateShapes(changes)
return this return this
} }

View file

@ -28,10 +28,9 @@ const ids = {
boxD: createShapeId('boxD'), boxD: createShapeId('boxD'),
boxE: createShapeId('boxE'), boxE: createShapeId('boxE'),
boxF: createShapeId('boxF'), boxF: createShapeId('boxF'),
boxX: createShapeId('boxX'), boxX: createShapeId('boxX'),
lineA: createShapeId('lineA'), lineA: createShapeId('lineA'),
groupA: createShapeId('groupA'),
} }
const box = ( const box = (
@ -1965,3 +1964,38 @@ describe('Group opacity', () => {
expect(group.opacity).toBe(1) expect(group.opacity).toBe(1)
}) })
}) })
describe('Grouping / ungrouping locked shapes', () => {
it('keeps locked shapes locked when grouped', () => {
editor.createShapes([box(ids.boxA, 0, 0), box(ids.boxB, 200, 0)])
editor.groupShapes([ids.boxA, ids.boxB], ids.groupA)
editor.select(ids.boxA)
// Lock boxA
editor.updateShape({ id: ids.boxA, type: 'geo', isLocked: true })
// Select the group; when we ungroup, the children should be selected
editor.select(ids.groupA)
// Move the group shape; this should also move locked children
editor.nudgeShapes([ids.groupA], { x: 100, y: 0 })
// When we ungroup, the locked children should...
editor.ungroupShapes([ids.groupA])
// Still exist
expect(editor.getShape(ids.boxA)).toBeTruthy()
expect(editor.getShape(ids.boxB)).toBeTruthy()
// Both be selected
expect(editor.selectedShapeIds).toMatchObject([ids.boxA, ids.boxB])
// And be in the correct position
expect(editor.getShape(ids.boxA)!.x).toBe(100)
expect(editor.getShape(ids.boxB)!.x).toBe(300)
// And have the correct locked state
expect(editor.getShape(ids.boxA)!.isLocked).toBe(true)
expect(editor.getShape(ids.boxB)!.isLocked).toBe(false)
})
})