-
Notifications
You must be signed in to change notification settings - Fork 44
feat(undo/redo): introduce clearUndoRedo in favor of store.clearStack #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3e5fa26
de008d5
dbd3b82
fcf3944
d97a470
b1aa283
2c77af3
cf6d3ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||
| import { TestBed } from '@angular/core/testing'; | ||||||
| import { signalStore, withState } from '@ngrx/signals'; | ||||||
| import { clearUndoRedo } from './clear-undo-redo'; | ||||||
| import { withUndoRedo } from './with-undo-redo'; | ||||||
|
|
||||||
| describe('withUndoRedo', () => { | ||||||
| describe('clearUndoRedo', () => { | ||||||
| it('should throw an error if the store is not configured with withUndoRedo()', () => { | ||||||
| const Store = signalStore({ providedIn: 'root' }, withState({})); | ||||||
| const store = TestBed.inject(Store); | ||||||
|
|
||||||
| expect(() => clearUndoRedo(store)).toThrow( | ||||||
| 'Cannot clear undoRedo, since store is not configured with withUndoRedo()', | ||||||
| ); | ||||||
| }); | ||||||
|
|
||||||
| it('should not throw no error if the store is configured with withUndoRedo()', () => { | ||||||
|
||||||
| it('should not throw no error if the store is configured with withUndoRedo()', () => { | |
| it('should not throw an error if the store is configured with withUndoRedo()', () => { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import { StateSource } from '@ngrx/signals'; | ||
|
|
||
| export type ClearUndoRedoOptions<TState extends object> = { | ||
| lastRecord: Partial<TState> | null; | ||
| }; | ||
|
|
||
| export type ClearUndoRedoFn<TState extends object> = ( | ||
| opts?: ClearUndoRedoOptions<TState>, | ||
| ) => void; | ||
|
|
||
| export function clearUndoRedo<TState extends object>( | ||
| store: StateSource<TState>, | ||
| opts?: ClearUndoRedoOptions<TState>, | ||
| ): void { | ||
| if (canClearUndoRedo(store)) { | ||
| store.__clearUndoRedo__(opts); | ||
| } else { | ||
| throw new Error( | ||
| 'Cannot clear undoRedo, since store is not configured with withUndoRedo()', | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| function canClearUndoRedo<TState extends object>( | ||
| store: StateSource<TState>, | ||
| ): store is StateSource<TState> & { | ||
| __clearUndoRedo__: ClearUndoRedoFn<TState>; | ||
| } { | ||
| if ( | ||
| '__clearUndoRedo__' in store && | ||
| typeof store.__clearUndoRedo__ === 'function' | ||
| ) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,8 @@ import { | |
| withHooks, | ||
| withMethods, | ||
| } from '@ngrx/signals'; | ||
| import { capitalize } from './with-data-service'; | ||
| import { capitalize } from '../with-data-service'; | ||
| import { ClearUndoRedoOptions } from './clear-undo-redo'; | ||
|
|
||
| export type StackItem = Record<string, unknown>; | ||
|
|
||
|
|
@@ -69,11 +70,12 @@ export function withUndoRedo<Input extends EmptyFeatureResult>( | |
| methods: { | ||
| undo: () => void; | ||
| redo: () => void; | ||
| /** @deprecated Use {@link clearUndoRedo} instead. */ | ||
| clearStack: () => void; | ||
| }; | ||
| } | ||
| > { | ||
| let previous: StackItem | null = null; | ||
| let lastRecord: StackItem | null = null; | ||
| let skipOnce = false; | ||
|
|
||
| const normalized = { | ||
|
|
@@ -108,40 +110,50 @@ export function withUndoRedo<Input extends EmptyFeatureResult>( | |
| undo(): void { | ||
| const item = undoStack.pop(); | ||
|
|
||
| if (item && previous) { | ||
| redoStack.push(previous); | ||
| if (item && lastRecord) { | ||
| redoStack.push(lastRecord); | ||
| } | ||
|
|
||
| if (item) { | ||
| skipOnce = true; | ||
| patchState(store, item); | ||
| previous = item; | ||
| lastRecord = item; | ||
| } | ||
|
|
||
| updateInternal(); | ||
| }, | ||
| redo(): void { | ||
| const item = redoStack.pop(); | ||
|
|
||
| if (item && previous) { | ||
| undoStack.push(previous); | ||
| if (item && lastRecord) { | ||
| undoStack.push(lastRecord); | ||
| } | ||
|
|
||
| if (item) { | ||
| skipOnce = true; | ||
| patchState(store, item); | ||
| previous = item; | ||
| lastRecord = item; | ||
| } | ||
|
|
||
| updateInternal(); | ||
| }, | ||
| clearStack(): void { | ||
| __clearUndoRedo__(opts?: ClearUndoRedoOptions<Input['state']>): void { | ||
| undoStack.splice(0); | ||
| redoStack.splice(0); | ||
| previous = null; | ||
|
|
||
| if (opts) { | ||
| lastRecord = opts.lastRecord; | ||
| } | ||
|
|
||
| updateInternal(); | ||
| }, | ||
| })), | ||
| withMethods((store) => ({ | ||
| /** @deprecated Use {@link clearUndoRedo} instead. */ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add here a note as well
|
||
| clearStack(): void { | ||
| store.__clearUndoRedo__(); | ||
| }, | ||
| })), | ||
| withHooks({ | ||
| onInit(store) { | ||
| watchState(store, () => { | ||
|
|
@@ -174,22 +186,22 @@ export function withUndoRedo<Input extends EmptyFeatureResult>( | |
| // if the component sends back the undone filter | ||
| // to the store. | ||
| // | ||
| if (JSON.stringify(cand) === JSON.stringify(previous)) { | ||
| if (JSON.stringify(cand) === JSON.stringify(lastRecord)) { | ||
| return; | ||
| } | ||
|
|
||
| // Clear redoStack after recorded action | ||
| redoStack.splice(0); | ||
|
|
||
| if (previous) { | ||
| undoStack.push(previous); | ||
| if (lastRecord) { | ||
| undoStack.push(lastRecord); | ||
| } | ||
|
|
||
| if (redoStack.length > normalized.maxStackSize) { | ||
| undoStack.unshift(); | ||
| } | ||
|
|
||
| previous = cand; | ||
| lastRecord = cand; | ||
|
|
||
| // Don't propogate current reactive context | ||
| untracked(() => updateInternal()); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add here a note about "Soft Reset (Default)" and "Hard Reset" and that we changed the behavior from Hard to soft.