-
-
Notifications
You must be signed in to change notification settings - Fork 848
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
Fix bug #3676 : Display consistent search history for Search documentation and Search resources. #3854
base: master
Are you sure you want to change the base?
Fix bug #3676 : Display consistent search history for Search documentation and Search resources. #3854
Conversation
WalkthroughThis change standardizes the use of a constant Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NB as NavBar/MobileNavMenu
participant SB as SearchButton
participant AS as AlgoliaSearch Modal
U->>NB: Initiates search action
NB->>SB: Renders SearchButton with INDEX_NAME
SB->>AS: Opens modal with INDEX_NAME parameter
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3854 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 667 667
Branches 113 113
=========================================
Hits 667 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/AlgoliaSearch.tsx
(1 hunks)components/navigation/DocsMobileMenu.tsx
(2 hunks)components/navigation/DocsNavWrapper.tsx
(2 hunks)components/navigation/MobileNavMenu.tsx
(2 hunks)components/navigation/NavBar.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 180000ms (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
🔇 Additional comments (8)
components/navigation/NavBar.tsx (3)
9-9
: Import change supports the standardization of search index.Importing
INDEX_NAME
constant allows consistent indexing across the application's search functionality, which addresses the inconsistent search history issue.
163-163
: Consistent search history implementation.Using the standardized
INDEX_NAME
for the mobile search button ensures users will see the same search history across all search interfaces.
224-224
: Standardized search index implementation.Using the same
INDEX_NAME
for the main navigation search button aligns with the fix for inconsistent search history display.components/navigation/DocsNavWrapper.tsx (2)
5-5
: Import change from DOCS_INDEX_NAME to INDEX_NAME.This change supports the standardization of search indices across the application, addressing the inconsistent search history issue.
28-28
: Standardized search index implementation.Using the consistent
INDEX_NAME
instead of the previously usedDOCS_INDEX_NAME
ensures that searches from the docs navigation will share history with other search components in the application.components/navigation/DocsMobileMenu.tsx (2)
5-5
: Import change from DOCS_INDEX_NAME to INDEX_NAME.This change supports the standardization of search indices across the application, addressing the inconsistent search history issue.
51-51
: Standardized search index for mobile docs menu.Using the consistent
INDEX_NAME
instead of the previously usedDOCS_INDEX_NAME
ensures that searches from the mobile docs menu will share history with other search components.components/navigation/MobileNavMenu.tsx (1)
6-6
: LGTM! Consistent use of INDEX_NAME improves search history integration.The change properly imports and implements the standardized INDEX_NAME constant, which directly addresses the bug described in PR #3676 regarding inconsistent search history display. Using a shared constant across search components ensures that search history will be consistent across the application.
Also applies to: 70-70
@@ -122,7 +122,7 @@ function AlgoliaModal({ onClose, initialQuery, indexName }: AlgoliaModalProps) { | |||
}} | |||
placeholder={indexName === DOCS_INDEX_NAME ? 'Search documentation' : 'Search resources'} | |||
onClose={onClose} | |||
indexName={indexName} | |||
indexName={INDEX_NAME} |
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.
Critical fix for search history consistency.
This is the core change that standardizes the search index. By forcing the DocSearchModal
to always use INDEX_NAME
instead of the passed indexName
parameter, all search components will now share the same search history.
It's worth noting that this change means the placeholder
prop (in line 123) may show different text than what's being searched, as it still uses the passed indexName
to determine whether to show "Search documentation" or "Search resources".
@Sharanshetty658 can you please explain to me the issue first |
@sambhavgupta0705 Search Bar: Search Icon: |
@AceTheCreator @anshgoyalevil Can you please review this PR. Thank you |
Made changes in:
Description:
-This PR fixes the Inconsistency shown for the recent searches displayed by the Search Bar and the Search Icon.
Fixes Bug : #3676
Summary by CodeRabbit