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
refactor(core): use KeyValueStore for recent searches #5872
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 8, 2024 5:23 PM (UTC)
|
6f0855d
to
ece8b23
Compare
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, great work @cngonzalez! Thank you for such an extensive test suite too, really appreciated!
packages/sanity/src/core/studio/components/navbar/search/datastores/recentSearches.test.tsx
Outdated
Show resolved
Hide resolved
recentTerms = result.current.getRecentSearches() | ||
expect(recentTerms.length).toEqual(2) | ||
|
||
/* There's unfortunately a bit of flakiness with the schema |
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.
thanks for taking the time to explain <3
* refactor(core): create useStoredSearch hook * refactor: pass useStoredSearch to recentSearchesStore * refactor(core): convert createRecentSearchesStore to useRecentSearchesStore * fix(core): use default studio client options * fix: pass recent terms for more predictable UI behavior * refactor: remove recent searches from search state and invoke hook wherever needed * fix: remove typos, spaces
Description
This PR refactors the store for recent searches to be aligned with other user settings values (you can see another recent example of this kind of a refactor here: #5784)
The changes are:
useStoredSearch
hook which depends on aKeyValueStore
backend, which is still currently localStorage. (previously, therecentSearches
functions mutated into localStorage directly)createRecentSearchesStore
intouseRecentSearchesStore
to better deal with asynchronous state coming in the future.useRecentSearchesStore
as a hook with dependencies elsewhere in the Studio.What to review
The changed files to look for any gotchas -- and any excessive re-renders or UI bugs. All seem to match the current experience from my checks.
Testing
The
recentSearches
unit tests were refactored but should still test for the exact same cases. One case was removed -- checking for versions, because this will be handled by KeyValueStore down the line.E2E UI tests were difficult because the recent search UI intercepts pointer events to some extent.