fix: background color visibility on dropdown hover #3330 #3340
Conversation
yashsaraswat2004
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe changes made in this pull request involve a modification to the hover state CSS class for menu block links in the Changes
Assessment against linked issues
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 (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
asyncapi-bot
requested review from
devilkiller-ag,
sambhavgupta0705,
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
yashsaraswat2004
changed the title
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3340--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: 1
🧹 Outside diff range and nitpick comments (1)
components/navigation/MenuBlocks.tsx (1)
46-46: Add transition for smoother hover effect.While the hover color change improves visibility, the component could benefit from a smoother transition.
The component already has
transition duration-150 ease-in-out, but consider explicitly adding background-color to the transition properties:- className={`-m-3 flex items-start space-x-4 rounded-lg p-3 transition duration-150 ease-in-out ${ + className={`-m-3 flex items-start space-x-4 rounded-lg p-3 transition-all transition-colors duration-150 ease-in-out ${
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- components/navigation/MenuBlocks.tsx (1 hunks)
🔇 Additional comments (2)
components/navigation/MenuBlocks.tsx (2)
46-46: Changes successfully address the visibility issue.The modification from
hover:bg-gray-50tohover:bg-gray-300effectively resolves the background color visibility issue mentioned in #3330. The increased contrast provides better visual feedback during hover interactions.
46-46: Verify color contrast meets accessibility standards.The change from
hover:bg-gray-50tohover:bg-gray-300improves visibility, but we should ensure it meets WCAG contrast requirements for hover states.Consider using the following tools to verify the contrast:
- Check if the color is part of your design system's approved palette
- Verify contrast ratios using a tool like WebAIM's contrast checker
- Test with different color modes (light/dark) if applicable
✅ Verification successful
The hover color change is consistent with the design system and accessibility patterns
The change to
hover:bg-gray-300is verified to be appropriate because:
- It's already used consistently across multiple components in the codebase for interactive elements (Label, DocsNav, Button)
- The color provides sufficient contrast as it's used in other prominent UI elements like buttons and labels where visibility is crucial
- It maintains consistency with the existing color system, as seen in the codebase's usage patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of bg-gray-300 to ensure consistency rg "bg-gray-300" --type tsx --type jsxLength of output: 66
Script:
#!/bin/bash # Search for bg-gray-300 usage across all files (without file type restriction) rg "bg-gray-300" # Search for other background color classes to understand the color system rg "bg-gray-[0-9]+" # Look for any Tailwind config files that might define these colors fd "tailwind.config" --type fLength of output: 16402
| @@ -43,7 +43,7 @@ export default function MenuBlocks({ items = [] }: MenuBlocksProps) { | |||
| <span | |||
| data-testid='MenuBlocks-Link' | |||
| className={`-m-3 flex items-start space-x-4 rounded-lg p-3 transition duration-150 ease-in-out ${ | |||
| router.asPath === item.href ? 'bg-secondary-100 shadow-sm' : 'hover:bg-gray-50' | |||
| router.asPath === item.href ? 'bg-secondary-100 shadow-sm' : 'hover:bg-gray-300' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider active/hover state distinction.
The current implementation uses bg-secondary-100 for active (selected) items and hover:bg-gray-300 for hover. These states should be visually distinct but related.
Consider using a consistent color family:
- router.asPath === item.href ? 'bg-secondary-100 shadow-sm' : 'hover:bg-gray-300'
+ router.asPath === item.href ? 'bg-secondary-100 shadow-sm' : 'hover:bg-secondary-50'📝 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.
| router.asPath === item.href ? 'bg-secondary-100 shadow-sm' : 'hover:bg-gray-300' | |
| router.asPath === item.href ? 'bg-secondary-100 shadow-sm' : 'hover:bg-secondary-50' |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3340 +/- ##
=======================================
Coverage 49.14% 49.14%
=======================================
Files 21 21
Lines 647 647
=======================================
Hits 318 318
Misses 329 329 ☔ View full report in Codecov by Sentry. |
|
The previous color seems relevant too. Why we need more intense color on hover? |
This PR fixes an issue with the visibility of background colors on components in the dropdown menu when hovering. Previously, hovering over dropdown items within the Docs, Community, and Tools menus did not display the intended background color, causing a poor user experience.
To address this:
Updated the background color styling to improve contrast and visibility for dropdown items on hover.
Verified that changes are consistent across different dropdown menus for a cohesive look.
Screenshots
How to Test
Open the ASyncAPI website in Google Chrome.
Hover over Docs, Community, or Tools in the navigation bar to trigger the dropdown menu.
Move the cursor over items in the dropdown menu to see the hover effect.
Related Issue(s)
Fixes #3330
Summary by CodeRabbit