-
Notifications
You must be signed in to change notification settings - Fork 432
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
feat(core): store and fetch user settings from backend #5939
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
No changes to documentation |
Component Testing Report Updated Mar 14, 2024 5:29 PM (UTC)
|
e40ccc6
to
edea1f6
Compare
5f1b86d
to
20cacef
Compare
const keyValueLoader = new DataLoader<string, KeyValueStoreValue | null>(async (keys) => { | ||
const value = await client | ||
.request<KeyValuePair[]>({ | ||
uri: `/users/me/keyvalue/${keys.join(',')}`, |
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.
this is probably fine now but if we get to where we do not have complete control over the key names, we may want to be more defensive here
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.
good point, the API will reject it right now but validating client side early would be nice. Not needed right now tho. Edit: not for GET
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.
Created an issue!
* fix: update tests to match api * fix: comment out flaky tests for now
20cacef
to
79da333
Compare
Description
We do not currently persist user settings and actions taken in the Studio in a way that follows the user. We store some user choices and actions to browser localStorage which means they do not follow users between desktop and mobile or different browsers. Browsers may also evict localStorage manually or automatically. These settings and actions are then lost and the user may need to reconfigure their experience or accept that things like search history is lost.
Not persisting this data in a universal manner gives a bad user experience since users need to make the same preference choices and settings again and again. Not having this capability also makes it harder for Sanity to develop new features that should follow the user, and we risk either continuing the localStorage limitation in new feature work, or that new initiatives all have to design their own solution for server-side settings and metadata.
What to review
Every code change in this branch was previously reviewed as part of a separate PR. However, this branch stands as the final state of this feature, and should be given "one last review."
Testing
Tests were mostly introduced in previous pull requests. This PR updates some of these tests to reflect settings no longer residing in local storage.
Notes for release
User settings (like desk list sort orders, view modes, and global search history) are now stored securely server-side by Sanity. This means that these settings do not need to be re-selected across devices or browsers, and will be persisted.