feat: preserve search query when modal closes and reopens #5167
feat: preserve search query when modal closes and reopens #5167Madhubytee wants to merge 1 commit intoasyncapi:masterfrom
Conversation
Madhubytee
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
princerajpoot20 and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
📝 WalkthroughWalkthroughThe pull request adds query persistence functionality to the Algolia search modal in Changes
Sequence DiagramsequenceDiagram
participant User
participant AlgoliaSearch as AlgoliaSearch<br/>(Parent)
participant AlgoliaModal as AlgoliaModal<br/>(Modal)
participant DocSearch as DocSearch<br/>(Input)
User->>AlgoliaSearch: Opens search modal
AlgoliaSearch->>AlgoliaModal: Render with onQueryChange callback
User->>DocSearch: Types search query
DocSearch->>AlgoliaModal: Input updated
User->>AlgoliaModal: Closes modal (ESC/click outside)
AlgoliaModal->>DocSearch: handleClose reads input value
DocSearch-->>AlgoliaModal: Returns current query text
AlgoliaModal->>AlgoliaSearch: onQueryChange(query) propagates
AlgoliaSearch->>AlgoliaSearch: setInitialQuery updates state
AlgoliaSearch->>AlgoliaModal: Modal unmounts
User->>AlgoliaSearch: Reopens search modal
AlgoliaSearch->>AlgoliaModal: Render with preserved initialQuery
AlgoliaModal->>DocSearch: Input seeded with saved query
User->>User: Continues typing without retyping
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Madhubytee
changed the title
|
|
@Madhubytee please read the community guidelines , in asyncAPI we wait for maintainer to approve then PR is raised , Please close the PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5167 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 796 796
Branches 146 146
=========================================
Hits 796 796 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5167--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@components/AlgoliaSearch.tsx`:
- Around line 117-127: The handleClose callback currently gates calling
onQueryChange behind a truthy check of input.value, so empty string clears are
skipped; update handleClose (the function using
document.querySelector('.DocSearch-Input')) to only check that the input element
exists (not its value) and always call onQueryChange(input.value) before
invoking onClose(event) so an empty string correctly clears the persisted query.
- Around line 141-145: The search query persists after navigation because
handleClose calls onQueryChange(input.value) before router.push; fix by clearing
the saved query on route change: inside AlgoliaSearch add a useEffect that
watches router.asPath and calls onQueryChange('') (or otherwise resets
initialQuery) when it changes so the modal opens empty on the new page;
alternatively, remove the onQueryChange call from the navigator.navigate
implementation (the block invoking handleClose and router.push) if you prefer
not to reset on route change. Ensure you reference the existing functions/props:
AlgoliaSearch, handleClose, navigator.navigate, onQueryChange, initialQuery, and
router.asPath.
| const handleClose = useCallback( | ||
| (event?: React.MouseEvent) => { | ||
| const input = document.querySelector('.DocSearch-Input') as HTMLInputElement; | ||
|
|
||
| if (input?.value) { | ||
| onQueryChange(input.value); | ||
| } | ||
| onClose(event); | ||
| }, | ||
| [onClose, onQueryChange] | ||
| ); |
There was a problem hiding this comment.
Bug: Clearing the search input does not reset the persisted query.
The condition if (input?.value) on line 121 is falsy for an empty string, so onQueryChange is never called when the user deletes all text before closing the modal. On the next open, the stale query reappears — contradicting the PR objective that "query clears if the user manually deletes all text in the input."
Change the guard to only check whether the element was found, not whether its value is truthy:
🐛 Proposed fix
const handleClose = useCallback(
(event?: React.MouseEvent) => {
const input = document.querySelector('.DocSearch-Input') as HTMLInputElement;
- if (input?.value) {
+ if (input) {
onQueryChange(input.value);
}
onClose(event);
},
[onClose, onQueryChange]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleClose = useCallback( | |
| (event?: React.MouseEvent) => { | |
| const input = document.querySelector('.DocSearch-Input') as HTMLInputElement; | |
| if (input?.value) { | |
| onQueryChange(input.value); | |
| } | |
| onClose(event); | |
| }, | |
| [onClose, onQueryChange] | |
| ); | |
| const handleClose = useCallback( | |
| (event?: React.MouseEvent) => { | |
| const input = document.querySelector('.DocSearch-Input') as HTMLInputElement; | |
| if (input) { | |
| onQueryChange(input.value); | |
| } | |
| onClose(event); | |
| }, | |
| [onClose, onQueryChange] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/AlgoliaSearch.tsx` around lines 117 - 127, The handleClose
callback currently gates calling onQueryChange behind a truthy check of
input.value, so empty string clears are skipped; update handleClose (the
function using document.querySelector('.DocSearch-Input')) to only check that
the input element exists (not its value) and always call
onQueryChange(input.value) before invoking onClose(event) so an empty string
correctly clears the persisted query.
| navigator={{ | ||
| navigate({ itemUrl }) { | ||
| onClose(); | ||
| handleClose(); | ||
| router.push(itemUrl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find where AlgoliaSearch is mounted in the app
rg -n "AlgoliaSearch" -g '*.tsx' -g '*.jsx' -A 3 -B 3Repository: asyncapi/website
Length of output: 5533
🏁 Script executed:
# Check the AlgoliaSearch component implementation, especially handleClose and state management
rg -n "handleClose|initialQuery|useEffect" components/AlgoliaSearch.tsx -A 2Repository: asyncapi/website
Length of output: 1392
🏁 Script executed:
# Check if there's any route-change listener in AlgoliaSearch
rg -n "router\.(asPath|pathname|events)" components/AlgoliaSearch.tsx -A 3Repository: asyncapi/website
Length of output: 42
🏁 Script executed:
# Get the full handleClose implementation
sed -n '117,130p' components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 392
🏁 Script executed:
# Get the useEffect hooks to see dependencies
sed -n '202,220p' components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 837
🏁 Script executed:
# Also check the second useEffect
sed -n '309,325p' components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 661
🏁 Script executed:
# Check onQueryChange implementation and how initialQuery is used
sed -n '247,295p' components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 1316
🏁 Script executed:
# Look for where initialQuery state is actually used and modified
rg -n "setInitialQuery|initialQuery" components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 455
🏁 Script executed:
# Check if there's any route-change handling in the component
grep -n "useRouter\|router\." components/AlgoliaSearch.tsx | head -20Repository: asyncapi/website
Length of output: 172
🏁 Script executed:
# Get the complete first useEffect with all dependencies
sed -n '202,235p' components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 1253
🏁 Script executed:
# Get the complete second useEffect with all dependencies
sed -n '309,330p' components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 778
🏁 Script executed:
# Check if initialQuery is ever reset/cleared to undefined
rg -n "initialQuery.*undefined|setInitialQuery.*undefined" components/AlgoliaSearch.tsxRepository: asyncapi/website
Length of output: 42
Query persists across route changes, contradicting stated PR objective.
When a user clicks a search result, handleClose() saves the current search query via onQueryChange(input.value) (line 119) before router.push(itemUrl) navigates away (line 144). Since AlgoliaSearch is mounted at the app level (pages/_app.tsx), it persists across client-side navigations and its state—including initialQuery—survives the route change. No route-change listener clears initialQuery on navigation, so the saved query remains and reappears on the next page when the search modal is opened again.
To align with the stated behavior, either (a) remove the onQueryChange call from the navigator's navigate function, or (b) add a useEffect that clears initialQuery when router.asPath changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/AlgoliaSearch.tsx` around lines 141 - 145, The search query
persists after navigation because handleClose calls onQueryChange(input.value)
before router.push; fix by clearing the saved query on route change: inside
AlgoliaSearch add a useEffect that watches router.asPath and calls
onQueryChange('') (or otherwise resets initialQuery) when it changes so the
modal opens empty on the new page; alternatively, remove the onQueryChange call
from the navigator.navigate implementation (the block invoking handleClose and
router.push) if you prefer not to reset on route change. Ensure you reference
the existing functions/props: AlgoliaSearch, handleClose, navigator.navigate,
onQueryChange, initialQuery, and router.asPath.
3 participants
Description
Added query persistence to the search modal. Query now stays when you close and reopen the modal instead of getting cleared.
Changes:
Related issue
Resolves #5165
Summary by CodeRabbit