GDB-14831 add local store persistence#6
Conversation
649c348 to
3cb6c34
Compare
## What Add local storage persistence for diagram state. ## Why This change improves user experience by ensuring that the diagram state is preserved across sessions, allowing users to return to their work without losing progress. ## How - Implemented `DiagramStorageService` to handle saving, loading, and clearing the diagram state in `localStorage`. - Introduced `EventService` to manage events related to clearing the diagram state. - Updated `ReactodiaApp` to utilize the new storage service for persisting diagram data. - Added tests for `DiagramStorageService` and `EventService` to ensure functionality. ## Testing jest/cypress
3cb6c34 to
910d5cf
Compare
| * Local storage getter | ||
| */ | ||
| private get storage(): Storage | undefined { | ||
| return service(WindowService).getWindow()?.localStorage; |
There was a problem hiding this comment.
I am lately thinking about this.
When will there be no window and is it not better to throw if there is no local storage, than just silently not doing what the client requested without telling him so.
There was a problem hiding this comment.
hmm. I had AI move a few classes from the workbench. Apparently it added the | undefined to the type of getWindow and I missed it. It never be undefined in a browser context. I removed it and the optional chaining due to it
| subscribeToDiagramChange(model: DataDiagramModel, onChangeCallback: DiagramChangeCallback): Subscription { | ||
| let notifyTimer: ReturnType<typeof setTimeout> | undefined; | ||
|
|
||
| const onHistoryChange = (): void => { |
There was a problem hiding this comment.
Perhaps also add a comment here why you need to debounce this as it is not clear until you read the comment for the debounce time
There was a problem hiding this comment.
moved the comment to the method itself
| subscribeToClearDiagramStorage(onClear: () => void): Subscription { | ||
| const target = service(WindowService).getWindow(); | ||
| if (!target) { | ||
| return () => undefined; |
There was a problem hiding this comment.
The client will think he subscribed successfully
There was a problem hiding this comment.
removed, as we should always have a target
| */ | ||
| export class WindowService implements Service { | ||
| getWindow(): Window | undefined { | ||
| return window; |
There was a problem hiding this comment.
Again, can this be undefined and when?
There was a problem hiding this comment.
never. I removed it and addressed why in the comment above
|
|
||
| connectedCallback(): void { | ||
| this.renderGraph(); | ||
| if (this.config) { |
There was a problem hiding this comment.
Maybe throw when there is no config.
Or is there a listener for the config which will reload?
There was a problem hiding this comment.
* Read once on mount; not watched, as it only sets up the data source and initial canvas.
There was a problem hiding this comment.
this will leave us with a permanent error always, as the first connectedCallback runs before @Props are populated, so initially we always throw. That's the reason I added a condition, instead of throwing an error
- removed the undefined type of window and removed the optional chaining due to it - moved the comment about debounce in the method jsdoc
2 participants
What
Add local storage persistence for diagram state.
Why
This change improves user experience by ensuring that the diagram state is preserved across sessions, allowing users to return to their work without losing progress.
How
DiagramStorageServiceto handle saving, loading, and clearing the diagram state inlocalStorage.EventServiceto manage events related to clearing the diagram state.ReactodiaAppto utilize the new storage service for persisting diagram data.Testing
jest/cypress
Screenshots
Checklist