feat: Add search history & top searches #112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Deployment failed with the following error: |
|
Deployment failed with the following error: |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Preview is readyThis pull request generated a Preview👀 Preview: https://sfj-f1a0e54--nextjs.preview.vtex.app |
Lighthouse ReportsHere are the Lighthouse reports of this Pull Request📝 Based on commit f1a0e54
|
added 2 commits
June 10, 2022 13:48
A cherry-pick from the Gatsby's merge commit of this same feature. * Skip rendering the history if there isn't any * Clear the history when its button is clicked * Stop trying to open the link in a new tab In a previous design spec we opened the links in a new tab but we are not doing this anymore * Remove onClear prop, does not seem to be needed * Show the search history component * Show up to 4 search history items max The design spec mentions 4 max: https://www.figma.com/file/kJywgfIx8LBIzggCY5AeDy/Handoff-v3.1-(Search)?node-id=2%3A58234 * Add the top searches * Extract a `formatSearchPath` util function * Use a link instead of button for the suggested terms Because the `href` is known so we can use links and not navigate programmatically with buttons. * Fix search test I had removed the `navigate` call because it wasn't needed for the links, but it's also used programmatically to trigger the search in the input. # Please enter the commit message for your changes. Lines starting * Fix SonarQube warning * Create a context so the search dropdown can be closed When terms, history, top search, products are clicked. * Consider `closeSearchInputDropdown` to be null Missed including this in the previous commit. * Fix SonarQube warning * Rename button from Clear to Clear History * Adjust Loading text message spacing * Fix link color for Top Search * Fix grayish clock icon color in search history * Limit search to 5 items per category Noticed sometimes we were showing +5 and, apparently, the `first` pagination doesn't really filter the results. * Also update the search input with the selected option * Fix suggested terms link color * Add the magnifying glass icon to suggested terms * Run TS Hero extension to organize imports of touched files * Better handle search input selections Whenever a selection is made, be it from History, Top Search, Suggested Terms or Products, use the same logic: - Add term to the history - Send term to analytics - Close dropdown - Update the input with the term * Do not show an empty gap when there are not results * Move `SearchInputContext` and `useSearchInput` to `sdk/search/…` * Fix warning about `SearchHistoryProps` type export * Fix overflow-x when expanding the search on mobile * Fix React key warning * Debounce the search suggestions * Add entries to the changelog * Adjust `SuggestionsTopSearch` after the merged changes * Fix Storybook search stories * Simplify top search querying by using `useQuery` Address [this suggestion](vtex-sites/gatsby.store#67 (comment)). * Remove debounce package in favor of using `useDeferredValue` from React Address [this comment](vtex-sites/gatsby.store#67 (comment)): > can't we use React18 features like useDefferedValue to not need to implement a debounce function? * Let TypeScript be aware of new types from React 18 So it doesn't complain when we use the new features (e.g., `useDeferredValue`). * Let search history also record the path Instead of using the generic search path for all the searched terms, along with the searched term, also store its path. This way, if a suggested product is selected, we store its name and its PDP path. * Better organize by extracting `useTopSearch` to its own file Address this comment: vtex-sites/gatsby.store#67 (comment) * Better organize by extracting `useSuggestions` to its own file Address this comment: vtex-sites/gatsby.store#67 (comment) * Fix the missing search input (left) padding Pointed out by this comment: vtex-sites/gatsby.store#67 (review) > should we add padding-left inside the input? What happened was that the padding was being overwritten by the styles when the search input is minimized. --- Conflicts: - CHANGELOG.md - src/components/common/SearchInput/SearchInput.tsx - src/components/search/History/SearchHistory.tsx - src/components/search/SuggestionProductCard/SuggestionProductCard.tsx - src/components/search/Suggestions/Suggestions.tsx - src/components/search/Suggestions/SuggestionsTopSearch.tsx - src/components/ui/Search/Suggestions/Suggestions.stories.tsx - src/components/ui/Search/Suggestions/Suggestions.tsx
filipewl
force-pushed
the
feat/search-history-n-top
branch
from
f606e4b to
5be7614
Compare
Cherry-picked from the `gatsby.store` repo. * Fixes `useTopSearch` query * Updates CHANGELOG.md --- Conflicts: - @generated/graphql/index.ts - @generated/graphql/persisted.json - CHANGELOG.md
I had done that to add support for types from React 18 on Gatsby but that is not needed in the NextJS case. The support for React 18 types was done in a previous PR.
added 2 commits
June 13, 2022 13:51
Part of the link had the blue color instead of light gray. That's because the light gray selector had less CSS specificity than the blue one, so I increased the specificity. I think will be addressed when we got CSS modules working for all components.
filipewl
force-pushed
the
feat/search-history-n-top
branch
from
7b3a93f to
f05d65a
Compare
This week querying for the top search for the storeframework account returned no results, that's because this is based on real usage data. Because of that, we were showing the "Top Search" dropdown with the heading but no data. In this scenario, let's not render anything till we decide what to do.
And also adjust the padding so the link covers it. Reported by Fanny!
hellofanny
approved these changes
Jun 14, 2022
eduardoformiga
approved these changes
Jun 14, 2022
I think this was intentional. The bottom corners should be flat (according to the prototype 🤔 ) |
Even when there are no results for the search term? |
|
@eduardoformiga and @hellofanny, I've created an internal task to deal with the border radius issue because I couldn't think of a very straightforward solution. Thanks for all the feedback! |
Conflicts: - CHANGELOG.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
How does it work?
See vtex-sites/gatsby.store#67 for more details.
How to test it?
Because the intelligent search's top search data comes from real usage data from the last 14 days, as of today, we don't have any top search data to show for the storeframework store ID. That means the preview deploy won't display the Top Search component with data. Because of that, and for testing purposes, I've created a temporary branch where I switched the store ID for storecomponents, it has top search data and you can preview it working in this preview deploy.
Known Issues
TopSearchstorie is not loading data correctly and will be fixed in a later PR.