-
Notifications
You must be signed in to change notification settings - Fork 98
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
Persist redux state in browser #1178
Persist redux state in browser #1178
Conversation
Signed-off-by: Eric Wei <menwe@amazon.com>
@@ -5,21 +5,45 @@ | |||
|
|||
import { configureStore } from '@reduxjs/toolkit'; | |||
import rootReducer from '../reducers'; | |||
import storage from 'redux-persist/lib/storage'; |
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.
should this use session storage instead of local storage?
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.
Yes, we should use session storage, the default storage is localStorage
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.
Eric, could you please describe in more detail how the user is affected before this change?
What happens if the user navigates away from a page by URL and then back?
Sure, before this change, if some user actions (for example, directly trigger a saved visualizations url in address bar, or simply just reload the page) trigger a page reload/refresh, all the event/visualization data for each tab in event analytics if there's any will get lost. Navigate to other pages from URL or other user actions within the domain would not cause data loss, which is also not part of the problem this change is trying to address. |
@@ -5,21 +5,45 @@ | |||
|
|||
import { configureStore } from '@reduxjs/toolkit'; | |||
import rootReducer from '../reducers'; | |||
import storage from 'redux-persist/lib/storage'; |
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.
Have we considered using url parameters? That way users can share a page with given filters or bookmark a page with commonly used filters for convenience?
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.
Interesting approach. I think that's technically a feature. @mengweieric ?
The specific issue addressed by this Pr is the "lost-form-on-refresh" problem. I agree with this Pr as it stands.
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.
Yes, this is a feature, you may create an issue for a feature request.
linking issue opensearch-project/dashboards-observability#62 this fixes item 3 |
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.
LGTM pending session/local storage changes
Signed-off-by: Eric Wei <menwe@amazon.com>
1ce1f18
* persist redux state Signed-off-by: Eric Wei <menwe@amazon.com> * switch to session storage Signed-off-by: Eric Wei <menwe@amazon.com> Signed-off-by: Eric Wei <menwe@amazon.com> (cherry picked from commit ceac84d)
* persist redux state Signed-off-by: Eric Wei <menwe@amazon.com> * switch to session storage Signed-off-by: Eric Wei <menwe@amazon.com> Signed-off-by: Eric Wei <menwe@amazon.com> (cherry picked from commit ceac84d) Co-authored-by: Eric Wei <menwe@amazon.com>
Signed-off-by: Eric Wei menwe@amazon.com
Description
Currently, redux state data gets lost on page refresh, which leads to bad user experience. This change is to address this problem.
Issues Resolved
#1177
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.