Repository navigation
refactor(landing): share the search submit and suggestion logic between ServiceSearch and the homepage hero (#3686) - #3715
Conversation
…eServiceSearch (#3686) The homepage hero repeated ServiceSearch's search-submit and search-suggestion-select tracking almost line for line, so the two payloads could drift. Both now use one hook. The hero's empty search still goes to /services, as an option on the hook. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
Amplify PR previews
Commit:
|
IsaiahSama
left a comment
There was a problem hiding this comment.
Approving. I found nothing that blocks this.
I checked the hook against both removed copies:
submit:search-submitsends{ query, source }with the trimmed query. An empty search goes to/servicesonly whenemptyTois set, which only HomeHero passes. ServiceSearch still sends an empty search to/search-results?q=, as it did at base.onSuggestionSelect:queryis read from the typed text beforesetText(hit.title), which matches both originals.positionisindex + 1, then it callssubmit(hit.title).hitsis worked out in the same render as before, so the index lines up with the suggestions the user sees.- HomeHero still gets suggestions only in Search mode after typing (
suggesting: mode === 'search' && hasTyped).SOURCEis still'home'.
On #3686: there is now one copy of both payloads, the homepage passes emptyTo: '/services', and the hook sits next to ServiceSearch. The options object differs from the signature the issue suggested. The description explains why: the text state stays with each component because the hero shares it with Ask mode. That holds. Neither test file is changed in the diff.
CI: all checks pass, including Build, Type Check, Lint and Test. The PR is mergeable.
Non-blocking nit, take it or leave it: ServiceSearch now writes 'services' | 'results' out in its props, while the hook has its own SearchSource union. If the hook exported SearchSource, ServiceSearch could use Exclude<SearchSource, 'home'> and the list of sources would live in one place.
…ce (#3686) Export SearchSource from useServiceSearch and type ServiceSearch's source prop as Exclude<SearchSource, 'home'>, so the list of sources lives in one place. Type-only, no behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Done in 59f98a1. The hook now exports |
Closes #3686
The homepage hero repeated ServiceSearch's search logic almost line for line, so there were two copies of the
search-submitandsearch-suggestion-selectpayloads that could drift. Both now use one hook,useServiceSearch, inapps/landing/src/components/. It works out the suggestions, sends both events and navigates. The one real difference stays an option on the hook: the homepage passesemptyTo: '/services', so an empty search there still goes to the services list, and ServiceSearch's still goes to the results page.The hook takes one options object rather than the
(source, viewer, overlay, { emptyTo })shape suggested in review. Each component keeps its own field text and passes it in (text,setText,suggesting), because the hero's text is shared with the Ask field and its suggestions only show in Search mode. Moving that state into the hook would have pulled Ask mode in with it. Both components' field markup is unchanged.How it was checked
HomeHero.test.tsxandServiceSearch.test.tsxpass unchanged, and between them they cover both empty-search destinations and the full payload of both events forhome,servicesandresults. To make sure they'd catch a slip in the hook, I briefly broke the empty-search option and the suggestion position: four tests failed, two in each file. No separate hook tests, since they'd only repeat these.Not in this PR
Tracking the hero's Popular searches links is #3683, which can now use the hook's
submit.🤖 Generated with Claude Code