[bugfix] Preserve redo stack when selection changes (#3862)

This PR identifies a bunch of places where the redo stack was being
blown when it shouldn't have been. I think this is going to be a
constant footgun unless we switch back to `ignore-by-default` behavior
for the store.

I also ignored changes to assets, which brings them back to how they
were being handled before the undo/redo change (i.e. they lived outside
of the undo/redo system).

### Change Type

<!--  Please select a 'Scope' label ️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!--  Please select a 'Type' label ️ -->

- [x] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [ ] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know


### Test Plan

1. Add a step-by-step description of how to test your PR here.
2.

- [ ] Unit Tests
- [ ] End to end tests

### Release Notes

- Add a brief release note for your PR here.
This commit is contained in:
David Sheldrick 2024-06-03 13:57:20 +01:00 committed by GitHub
parent 14abf25ab6
commit 9422a0ecc2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1871,7 +1871,7 @@ export class Editor extends EventEmitter<TLEventMap> {
setHoveredShape(shape: TLShapeId | TLShape | null): this { setHoveredShape(shape: TLShapeId | TLShape | null): this {
const id = typeof shape === 'string' ? shape : shape?.id ?? null const id = typeof shape === 'string' ? shape : shape?.id ?? null
if (id === this.getHoveredShapeId()) return this if (id === this.getHoveredShapeId()) return this
this.updateCurrentPageState({ hoveredShapeId: id }) this.updateCurrentPageState({ hoveredShapeId: id }, { history: 'ignore' })
return this return this
} }
@ -3217,7 +3217,7 @@ export class Editor extends EventEmitter<TLEventMap> {
}) })
transact(() => { transact(() => {
this.updateInstanceState({ followingUserId: userId }) this.updateInstanceState({ followingUserId: userId }, { history: 'ignore' })
// we listen for page changes separately from the 'moveTowardsUser' tick // we listen for page changes separately from the 'moveTowardsUser' tick
const dispose = react('update current page', () => { const dispose = react('update current page', () => {
@ -3331,7 +3331,7 @@ export class Editor extends EventEmitter<TLEventMap> {
* @public * @public
*/ */
stopFollowingUser(): this { stopFollowingUser(): this {
this.batch(() => { this.history.ignore(() => {
// commit the current camera to the store // commit the current camera to the store
this.store.put([this.getCamera()]) this.store.put([this.getCamera()])
// this must happen after the camera is committed // this must happen after the camera is committed
@ -3817,7 +3817,8 @@ export class Editor extends EventEmitter<TLEventMap> {
createAssets(assets: TLAsset[]): this { createAssets(assets: TLAsset[]): this {
if (this.getInstanceState().isReadonly) return this if (this.getInstanceState().isReadonly) return this
if (assets.length <= 0) return this if (assets.length <= 0) return this
return this.batch(() => this.store.put(assets)) this.history.ignore(() => this.store.put(assets))
return this
} }
/** /**
@ -3835,7 +3836,7 @@ export class Editor extends EventEmitter<TLEventMap> {
updateAssets(assets: TLAssetPartial[]): this { updateAssets(assets: TLAssetPartial[]): this {
if (this.getInstanceState().isReadonly) return this if (this.getInstanceState().isReadonly) return this
if (assets.length <= 0) return this if (assets.length <= 0) return this
return this.batch(() => { this.history.ignore(() => {
this.store.put( this.store.put(
assets.map((partial) => ({ assets.map((partial) => ({
...this.store.get(partial.id)!, ...this.store.get(partial.id)!,
@ -3843,6 +3844,7 @@ export class Editor extends EventEmitter<TLEventMap> {
})) }))
) )
}) })
return this
} }
/** /**
@ -3866,7 +3868,8 @@ export class Editor extends EventEmitter<TLEventMap> {
: (assets as TLAsset[]).map((a) => a.id) : (assets as TLAsset[]).map((a) => a.id)
if (ids.length <= 0) return this if (ids.length <= 0) return this
return this.batch(() => this.store.remove(ids)) this.history.ignore(() => this.store.remove(ids))
return this
} }
/** /**