fix: #3371 visibility issue with algolia search icon in navbar #3400
Conversation
jyotishpro
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
CodeRabbit Configuration File (
|
![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.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Fixes #3371 |
![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
🧹 Outside diff range and nitpick comments (1)
components/navigation/NavBar.tsx (1)
43-43: Fix code formatting issuesSeveral formatting issues were detected by static analysis:
- Remove trailing spaces and extra blank lines
- Use single quotes consistently
- Remove unnecessary trailing commas
Apply these formatting fixes:
- "mr-2 p-3 flex items-center space-x-2 rounded-md opacity-40 text-left text-gray-400 transition duration-150 ease-in-out hover:bg-gray-100 hover:text-gray-500 focus:bg-gray-100 focus:text-gray-500 focus:outline-none" + 'mr-2 p-3 flex items-center space-x-2 rounded-md opacity-40 text-left text-gray-400 transition duration-150 ease-in-out hover:bg-gray-100 hover:text-gray-500 focus:bg-gray-100 focus:text-gray-500 focus:outline-none' - "Open Search" + 'Open Search' backgroundPosition: 'center left', // Remove trailing comma - <IconLoupe/> // Remove extra spaces and lines + <IconLoupe />Also applies to: 240-240, 242-243, 248-248, 251-254
🧰 Tools
🪛 eslint
[error] 43-43: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 43-44: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 43-44: Delete
⏎··(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/navigation/NavBar.tsx(4 hunks)
🧰 Additional context used
🪛 eslint
components/navigation/NavBar.tsx
[error] 43-43: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 43-44: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 43-44: Delete ⏎··
(prettier/prettier)
[error] 240-240: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 240-241: Delete ⏎··········
(prettier/prettier)
[error] 242-242: Replace "mr-2·p-3·flex·items-center·space-x-2·rounded-md·opacity-40·text-left·text-gray-400·transition·duration-150·ease-in-out·hover:bg-gray-100·hover:text-gray-500·focus:bg-gray-100·focus:text-gray-500·focus:outline-none" with 'mr-2·p-3·flex·items-center·space-x-2·rounded-md·opacity-40·text-left·text-gray-400·transition·duration-150·ease-in-out·hover:bg-gray-100·hover:text-gray-500·focus:bg-gray-100·focus:text-gray-500·focus:outline-none'
(prettier/prettier)
[error] 242-242: Unexpected usage of doublequote.
(jsx-quotes)
[error] 243-243: Replace "Open·Search" with 'Open·Search'
(prettier/prettier)
[error] 243-243: Unexpected usage of doublequote.
(jsx-quotes)
[error] 248-248: Delete ,
(prettier/prettier)
[error] 248-248: Unexpected trailing comma.
(comma-dangle)
[error] 251-251: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 251-254: Replace ⏎⏎··············<IconLoupe/>⏎·············· with <IconLoupe·/>
(prettier/prettier)
[error] 252-253: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 254-254: Trailing spaces not allowed.
(no-trailing-spaces)
🔇 Additional comments (2)
components/navigation/NavBar.tsx (2)
149-164: LGTM: SVG definition is well-structured
The SVG definition includes proper accessibility attributes and styling parameters.
Line range hint 181-185: LGTM: Mobile search button implementation is correct
The mobile view implementation uses IconLoupe component directly with appropriate styling classes.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3400--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/navigation/NavBar.tsx(4 hunks)
🧰 Additional context used
🪛 eslint
components/navigation/NavBar.tsx
[error] 43-43: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 43-44: More than 1 blank line not allowed.
(no-multiple-empty-lines)
[error] 43-44: Delete ⏎··
(prettier/prettier)
[error] 240-240: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 240-241: Delete ⏎··········
(prettier/prettier)
[error] 242-242: Replace "mr-2·p-3·flex·items-center·space-x-2·rounded-md·text-left··text-gray-400·transition·duration-150·ease-in-out·hover:bg-gray-100·hover:text-gray-500·focus:bg-gray-100·focus:text-gray-500·focus:outline-none" with 'mr-2·p-3·flex·items-center·space-x-2·rounded-md·text-left··text-gray-400·transition·duration-150·ease-in-out·hover:bg-gray-100·hover:text-gray-500·focus:bg-gray-100·focus:text-gray-500·focus:outline-none'
(prettier/prettier)
[error] 242-242: Unexpected usage of doublequote.
(jsx-quotes)
[error] 243-243: Replace "Open·Search" with 'Open·Search'
(prettier/prettier)
[error] 243-243: Unexpected usage of doublequote.
(jsx-quotes)
[error] 248-248: Delete ,
(prettier/prettier)
[error] 248-248: Unexpected trailing comma.
(comma-dangle)
[error] 250-250: Trailing spaces not allowed.
(no-trailing-spaces)
🔇 Additional comments (1)
components/navigation/NavBar.tsx (1)
149-164:
Remove redundant SVG icon definition
The SVG icon definition is redundant since the IconLoupe component is already imported and used. Having both a background image and the IconLoupe component creates duplicate icons.
This issue was previously identified. Please refer to the existing suggestion to simplify the search button implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
components/navigation/MobileNavMenu.tsx (3)
44-59: SVG implementation looks good, with room for improvementThe SVG implementation is well-structured with proper accessibility attributes and styling. However, consider these improvements:
- Move the SVG to a separate file in your icons directory for better maintainability
- Make the size configurable through props for better responsiveness
Consider refactoring to:
- const loupeSvg = encodeURIComponent(`...`); + import { ReactComponent as LoupeIcon } from '../icons/loupe.svg';
72-79: Search button styling effectively addresses visibility issueThe styling changes successfully improve the search icon's visibility and usability by:
- Increasing the clickable area with larger padding
- Ensuring consistent icon size and positioning
Fix the trailing comma in the style object:
backgroundImage: `url("data:image/svg+xml;charset=utf-8,${loupeSvg}")`, backgroundSize: '1.5rem', backgroundRepeat: 'no-repeat', - backgroundPosition: 'center left', + backgroundPosition: 'center left'🧰 Tools
🪛 eslint
[error] 78-78: Delete
,(prettier/prettier)
[error] 78-78: Unexpected trailing comma.
(comma-dangle)
81-81: Remove unnecessary whitespaceSince the search icon is now implemented as a background image, the empty line with whitespace between SearchButton tags can be removed.
<SearchButton className='flex items-center space-x-2 rounded-md p-4 text-left text-gray-400 transition duration-150 ease-in-out hover:bg-gray-100 hover:text-gray-500 focus:bg-gray-100 focus:text-gray-500 focus:outline-none' aria-label='Open Search' style={{ backgroundImage: `url("data:image/svg+xml;charset=utf-8,${loupeSvg}")`, backgroundSize: '1.5rem', backgroundRepeat: 'no-repeat', backgroundPosition: 'center left' }} > - </SearchButton>🧰 Tools
🪛 eslint
[error] 81-81: Trailing spaces not allowed.
(no-trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/navigation/MobileNavMenu.tsx(2 hunks)
🧰 Additional context used
🪛 eslint
components/navigation/MobileNavMenu.tsx
[error] 78-78: Delete ,
(prettier/prettier)
[error] 78-78: Unexpected trailing comma.
(comma-dangle)
[error] 81-81: Trailing spaces not allowed.
(no-trailing-spaces)
🔇 Additional comments (1)
components/navigation/MobileNavMenu.tsx (1)
Line range hint 44-81: Implementation successfully addresses the search icon visibility issue
The changes effectively solve the visibility issue reported in #3371 by:
- Implementing a properly sized search icon (24x24)
- Ensuring good visibility through proper styling and positioning
- Maintaining accessibility with proper ARIA attributes
- Increasing the clickable area for better user interaction
The implementation aligns well with the PR objectives and maintains good accessibility standards.
Let's verify the implementation is consistent across the codebase:
✅ Verification successful
Search icon implementation is consistent across the codebase
The implementation in MobileNavMenu.tsx aligns with other search button implementations in the codebase:
- The search icon styling (24x24, stroke properties) matches the standard
SearchIconcomponent - The implementation is consistent with other search buttons in:
NavBar.tsx(desktop navigation)DocsMobileMenu.tsx(docs mobile menu)AlgoliaSearch.tsx(search functionality component)
All implementations maintain proper accessibility with ARIA labels and consistent styling patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar search button implementations that might need the same fix
rg -l 'SearchButton|SearchIcon' --type tsx
Length of output: 72
Script:
#!/bin/bash
# Check for similar search button implementations that might need the same fix
# Using more generic file extensions and searching for both SearchButton and search-related SVG implementations
rg -l -t ts -t tsx -t jsx -t js 'SearchButton|<svg.*search|loupe'
# Also check for any other SVG implementations with similar attributes
rg -l -t ts -t tsx -t jsx -t js 'stroke="#9CA3AF"|stroke-width="2"|stroke-linecap="round"'
Length of output: 216
Script:
#!/bin/bash
# Search for similar implementations using ripgrep without file type restrictions
rg -l 'SearchButton'
rg -l '<svg[^>]*>(.*?)</svg>' --multiline
# Find all TypeScript/JavaScript files that might contain search-related components
fd -e ts -e tsx -e js -e jsx
Length of output: 13588
Script:
#!/bin/bash
# Check the implementation of SearchButton in other components
rg -A 10 'SearchButton' components/navigation/DocsMobileMenu.tsx components/navigation/NavBar.tsx components/AlgoliaSearch.tsx
# Check for search icon implementations
rg -A 10 'SearchIcon' components/icons/SearchIcon.tsx components/icons/Loupe.tsx
Length of output: 8999
|
/update |
|
Thanks for the PR, @jyotishpro, but it looks like another PR, #3447, fixes the issue with much smaller changes. |
Description
fix: #3371
Related issue(s)
-Fixes #3371
Summary by CodeRabbit
New Features
Style
These changes enhance user experience by providing better language options and a visually appealing interface.