Fix search history consistency #3680
Conversation
Related to #3676 Unify the search history between the search bar and search icon. * Introduce a shared search history state in `components/AlgoliaSearch.tsx`. * Update `SearchButton` in `components/AlgoliaSearch.tsx` to use the shared search history state. * Modify `AlgoliaModal` in `components/AlgoliaSearch.tsx` to accept and update the shared search history state. * Update `SearchButton` usage in `components/navigation/NavBar.tsx`, `components/navigation/MobileNavMenu.tsx`, `components/Hero.tsx`, and `components/navigation/DocsMobileMenu.tsx` to pass the shared search history state. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/asyncapi/website/issues/3676?shareId=XXXX-XXXX-XXXX-XXXX).
Recxsmacx
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
WalkthroughThe changes enhance the Algolia search feature by introducing a new search history state in the search context. The modifications update interfaces and function signatures in the AlgoliaSearch component to include a Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant AS as AlgoliaSearch
participant TM as transformItems
participant AM as AlgoliaModal
U->>AS: Initiates search query
AS->>TM: Processes query via transformItems
TM->>AS: Calls setSearchHistory (adds unique query)
AS->>AM: Passes updated searchHistory and setter
AM->>U: Displays search results with history context
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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.
asyncapi-bot
requested review from
BhaswatiRoy,
J0SAL,
TRohit20,
VaishnaviNandakumar,
ashmitjsg and
sambhavgupta0705
❌ Deploy Preview for asyncapi-website failed.Built without sensitive environment variables
|
|
We require all PRs to follow Conventional Commits specification. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/navigation/DocsMobileMenu.tsx (1)
53-66: LGTM! Great improvements to keyboard shortcut display.The changes enhance the search experience by:
- Using semantic HTML (
kbd,abbr) for better accessibility- Implementing a flexible render prop pattern
- Providing clear visual feedback for keyboard shortcuts
Consider extracting the "K" text to a constant or i18n key for better internationalization support:
</abbr>{' '} - K + {SEARCH_SHORTCUT_SUFFIX}components/AlgoliaSearch.tsx (1)
139-143: Fix code formatting and improve readability.The
transformItemsfunction needs formatting improvements:
- Add blank line after variable declarations
- Add blank line before return statement
Apply this diff to improve readability:
transformItems={(items) => { const transformedItems = transformItems(items); + setSearchHistory((prevHistory) => [ ...new Set([...prevHistory, ...transformedItems.map((item) => item.query)]) ]); + return transformedItems; }}🧰 Tools
🪛 ESLint
[error] 140-140: Expected blank line after variable declarations.
(newline-after-var)
[error] 141-141: Expected blank line before this statement.
(padding-line-between-statements)
[error] 141-141: Replace
...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)])with⏎··········...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)])⏎········(prettier/prettier)
[error] 142-142: Expected blank line before this statement.
(padding-line-between-statements)
🪛 GitHub Actions: PR testing - if Node project
[error] 140-140: Expected blank line after variable declarations.
[error] 141-141: Expected blank line before this statement.
[error] 141-141: Replace
...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)])with⏎··········...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)])⏎········.
[error] 142-142: Expected blank line before this statement.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/AlgoliaSearch.tsx(7 hunks)components/Hero.tsx(2 hunks)components/navigation/DocsMobileMenu.tsx(1 hunks)
🧰 Additional context used
🪛 ESLint
components/AlgoliaSearch.tsx
[error] 117-117: 'searchHistory' is defined but never used. Allowed unused args must match /^_/u.
(unused-imports/no-unused-vars)
[error] 117-117: 'searchHistory' is defined but never used.
(no-unused-vars)
[error] 140-140: Expected blank line after variable declarations.
(newline-after-var)
[error] 141-141: Expected blank line before this statement.
(padding-line-between-statements)
[error] 141-141: Replace ...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)]) with ⏎··········...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)])⏎········
(prettier/prettier)
[error] 142-142: Expected blank line before this statement.
(padding-line-between-statements)
[error] 304-304: 'searchHistory' is assigned a value but never used.
(unused-imports/no-unused-vars)
[error] 304-304: 'searchHistory' is assigned a value but never used.
(no-unused-vars)
[error] 304-304: 'setSearchHistory' is assigned a value but never used.
(unused-imports/no-unused-vars)
[error] 304-304: 'setSearchHistory' is assigned a value but never used.
(no-unused-vars)
🪛 GitHub Actions: PR testing - if Node project
components/AlgoliaSearch.tsx
[error] 117-117: 'searchHistory' is defined but never used. Allowed unused args must match /^_/u.
[error] 117-117: 'searchHistory' is defined but never used.
[error] 140-140: Expected blank line after variable declarations.
[error] 141-141: Expected blank line before this statement.
[error] 141-141: Replace ...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)]) with ⏎··········...new·Set([...prevHistory,·...transformedItems.map((item)·=>·item.query)])⏎········.
[error] 142-142: Expected blank line before this statement.
[error] 304-304: 'searchHistory' is assigned a value but never used.
[error] 304-304: 'setSearchHistory' is assigned a value but never used.
⏰ Context from checks skipped due to timeout of 180000ms (1)
- GitHub Check: Lighthouse CI
🔇 Additional comments (5)
components/Hero.tsx (2)
7-7: LGTM!The import statement has been correctly updated to only import the
SearchButtoncomponent, which aligns with the PR objective of unifying search history functionality.
55-70: LGTM!The
SearchButtonimplementation is clean and correctly structured with:
- Proper styling classes
- Dynamic keyboard shortcut rendering
- Consistent text and icon placement
components/AlgoliaSearch.tsx (3)
21-22: LGTM!The
ISearchContextinterface has been correctly updated to include search history state management.
42-43: LGTM!The
AlgoliaModalPropsinterface has been properly updated to include search history props.
282-293: LGTM!The
SearchContext.ProviderandAlgoliaModalcomponents are correctly configured with the search history state.
| */ | ||
| export function SearchButton({ children, indexName = INDEX_NAME, ...props }: ISearchButtonProps) { | ||
| const { onOpen, onInput } = useContext(SearchContext); | ||
| const { onOpen, onInput, searchHistory, setSearchHistory } = useContext(SearchContext); |
There was a problem hiding this comment.
Remove unused props in SearchButton component.
The searchHistory and setSearchHistory props are destructured but never used in the component.
Apply this diff to fix the unused variables:
- const { onOpen, onInput, searchHistory, setSearchHistory } = useContext(SearchContext);
+ const { onOpen, onInput } = useContext(SearchContext);📝 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 { onOpen, onInput, searchHistory, setSearchHistory } = useContext(SearchContext); | |
| const { onOpen, onInput } = useContext(SearchContext); |
🧰 Tools
🪛 ESLint
[error] 304-304: 'searchHistory' is assigned a value but never used.
(unused-imports/no-unused-vars)
[error] 304-304: 'searchHistory' is assigned a value but never used.
(no-unused-vars)
[error] 304-304: 'setSearchHistory' is assigned a value but never used.
(unused-imports/no-unused-vars)
[error] 304-304: 'setSearchHistory' is assigned a value but never used.
(no-unused-vars)
🪛 GitHub Actions: PR testing - if Node project
[error] 304-304: 'searchHistory' is assigned a value but never used.
[error] 304-304: 'setSearchHistory' is assigned a value but never used.
3 participants
Related to #3676
Unify the search history between the search bar and search icon.
components/AlgoliaSearch.tsx.SearchButtonincomponents/AlgoliaSearch.tsxto use the shared search history state.AlgoliaModalincomponents/AlgoliaSearch.tsxto accept and update the shared search history state.SearchButtonusage incomponents/navigation/NavBar.tsx,components/navigation/MobileNavMenu.tsx,components/Hero.tsx, andcomponents/navigation/DocsMobileMenu.tsxto pass the shared search history state.Summary by CodeRabbit