Persist some dashboard state to local storage and use it when returning to a dashboard #3973
Conversation
AdityaHegde
force-pushed
the
adityahegde/resotre-user-state
branch
from
12cfdf7
to
e6f8132
Compare
web-common/src/features/dashboards/stores/DashboardStateProvider.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The feature isn't working as expected for me. When I select filters on a dashboard, close the page, then re-open the dashboard, my filters are not applied. Am I missing something? Here's a JAM: https://jam.dev/c/f4899bfc-4d2a-4a16-96fe-83d6bcd7a2fd
I also left various comments inline.
web-common/src/features/metrics-views/workspace/editor/update-metrics.ts
Outdated
Show resolved
Hide resolved
web-common/src/features/dashboards/stores/persistent-dashboard-state.ts
Outdated
Show resolved
Hide resolved
web-common/src/features/dashboards/stores/dashboard-store-defaults.ts
Outdated
Show resolved
Hide resolved
web-common/src/features/dashboards/stores/persistent-dashboard-state.ts
Outdated
Show resolved
Hide resolved
web-common/src/features/dashboards/state-managers/state-managers.ts
Outdated
Show resolved
Hide resolved
web-common/src/features/dashboards/state-managers/state-managers.ts
Outdated
Show resolved
Hide resolved
|
|
||
| export type PersistentDashboardStore = Readable<PersistentDashboardState> & | ||
| ReturnType<typeof persistentDashboardActions>; | ||
| export function createPersistentDashboardStore(storeKey: string) { |
There was a problem hiding this comment.
Rather than creating a new store, did you consider taking our existing dashboard store and persisting it to local storage? Pros/cons? I'm a bit hesitant that we'll have to keep two stores in sync with the same data.
There was a problem hiding this comment.
A small section of the store is to be persisted. Check the details section for the exact fields. So it made sense to create a new store.
There was a problem hiding this comment.
Got it, I didn't realize that only a subset of the store is to be persisted.
Though I'm still wondering if it'd be simpler to persist the store in its entirety, then we can just choose which fields to restore in the restorePersistedDashboardState() function. Then it'd be easy to add/subtract fields we want to restore by just editing that one function.
Else, needing to remember to update both stores, I worry the two stores will often get out-of-sync and the feature will appear buggy.
There was a problem hiding this comment.
Another option to avoid synchronization issues – the persisted dashboard store could be a derived store that consumes the root dashboard store
There was a problem hiding this comment.
True. We can do that once the immer solution is in. For now it would be too many updates to the local storage.
There was a problem hiding this comment.
Okay, just opened up an issue to track this: https://github.com/rilldata/rill-private-issues/issues/164
|
@ericpgreen2 Check the details section. Only a very small settings are to be persisted as per product. |
Oh, I hadn't realized that. Yes, then for these fields it works like a charm! |
ericpgreen2
changed the title
…ng to a dashboard (#3973) * Add basic support for storing user perferences for a dashboard * Fix tests and lint * Add sort type to local preferences * Update proto before merging with preferences * Reset local preferences on dashboard edit * Fix leaderboard measure name not persisting * Fix unit tests * Decouple user perference and persistent state * Fix lint * Remove additionalKey param * PR comments
Checklist
Summary
Issue addressed:
closes #3932
Details:
We are storing the selection of measures, dimensions, selected leaderboard measure and leaderboard sort settings.
Steps to Verify