Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(ui): localstorage compatibility issue #930

Merged
merged 9 commits into from
Jun 11, 2021
Merged

fix(ui): localstorage compatibility issue #930

merged 9 commits into from
Jun 11, 2021

Conversation

shhdgit
Copy link
Member

@shhdgit shhdgit commented May 26, 2021 •
edited
Loading

close #902
related: docs

Local storage key will have release-version suffix by using useCompatibilityLocalstorage helper function.

image

@unbyte
Copy link
Contributor

unbyte commented May 26, 2021

LGTM, some suggestions:

  • check and auto clear items of older versions to reduce storage

  • the implicitly reset of certain setting items after update may cause trouble to users, which may need to be mentioned in the document

@shhdgit
Copy link
Member Author

shhdgit commented May 26, 2021

LGTM, some suggestions:

  • check and auto clear items of older versions to reduce storage
  • the implicitly reset of certain setting items after update may cause trouble to users, which may need to be mentioned in the document

Thanks for the suggestion. Considering that cleaning up localstorage after a release version change is too frequent on the cloud. Maybe we should simply use the v2 suffix. Do you have any better suggestions?

@unbyte
Copy link
Contributor

unbyte commented May 26, 2021 •
edited
Loading

is 'on cloud' means users always see the latest version? If so, two entries for the old one and the new one should be enough.

However I prefer to mark a version having breaking change of the localstorage schema manually and then migrate automatically rather than use a new entry every time update, which lead to the risk that users lose some settings even when there is no conflict between two versions.

Just a suggestion.

@shhdgit
Copy link
Member Author

shhdgit commented May 28, 2021 •
edited
Loading

How about this?
I use the package.json version field as the compatibility key. And use the release version as the sentry version.

@shhdgit shhdgit force-pushed the fix-localstorage-compatibility branch from 09a77af to 565209d Compare May 28, 2021 07:22
breezewish
breezewish reviewed Jun 5, 2021
View reviewed changes
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM! Let's prioritize this PR so that it can be carried in 4.0.14.

ui/.env.development Outdated Show resolved Hide resolved
ui/config-overrides.js Outdated Show resolved Hide resolved
ui/lib/apps/SlowQuery/pages/List/index.tsx Outdated Show resolved Hide resolved
@breezewish breezewish mentioned this pull request Jun 5, 2021
Closed
@shhdgit shhdgit force-pushed the fix-localstorage-compatibility branch from a44c13d to df93869 Compare June 7, 2021 03:36
@shhdgit shhdgit force-pushed the fix-localstorage-compatibility branch from b12fd74 to 857afcc Compare June 9, 2021 09:35
@shhdgit shhdgit requested a review from breezewish June 9, 2021 09:45
breezewish
breezewish approved these changes Jun 9, 2021
View reviewed changes
ui/.env.development Outdated Show resolved Hide resolved
ui/config-overrides.js Outdated Show resolved Hide resolved
ui/.env.development Outdated Show resolved Hide resolved
@ti-chi-bot

This comment has been minimized.

Sign in to view
breezewish
breezewish reviewed Jun 9, 2021
View reviewed changes
ui/src/sentry.ts Outdated Show resolved Hide resolved
@shhdgit shhdgit force-pushed the fix-localstorage-compatibility branch from a8301e0 to 804db1a Compare June 11, 2021 07:44
@breezewish
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 804db1a

@ti-chi-bot ti-chi-bot merged commit 7fdbb69 into pingcap:master Jun 11, 2021
shhdgit added a commit to shhdgit/tidb-dashboard that referenced this pull request Jun 15, 2021
@shhdgit
fix(ui): localstorage compatibility issue (pingcap#930)
75b77a2
breezewish pushed a commit that referenced this pull request Jun 15, 2021
@shhdgit
Release v2021.06.15.1 (#938)
6bbc7e6 
* fix: label of y-axis is overflow (#924)
* update(deps): upgrade gorm to v2 (#916)
* debug_api: Add more endpoints (#927)
* fix(ui): localstorage compatibility issue (#930)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unbyte unbyte unbyte left review comments

@breezewish breezewish breezewish approved these changes

Assignees
No one assigned
Labels
status/can-merge status/LGT1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error tips: unknown field
4 participants
@shhdgit @unbyte @ti-chi-bot @breezewish