Fix colours for sidebar links in dark mode #3325
Conversation
WalkthroughThe pull request introduces styling enhancements for active states in the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/components/nav-main.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
NdekoCode
changed the title
![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
🧹 Outside diff range and nitpick comments (2)
apps/web/components/nav-main.tsx (1)
83-85: LGTM! Consider adding hover state transition.The dark mode implementation for active menu items looks good. The color choices provide good contrast and visibility.
Consider adding
dark:hover:text-gray-50for consistency with the hover state:- ? 'font-medium bg-[#eaeef4] dark:text-gray-50 dark:bg-sidebar-accent' + ? 'font-medium bg-[#eaeef4] dark:text-gray-50 dark:hover:text-gray-50 dark:bg-sidebar-accent'apps/web/components/pages/team/tasks/tasks-data-table.tsx (1)
47-48: Consider removing the important flag (!) from the focus styles.While the dark mode styling updates look good, using the important flag (!border) in CSS classes is generally discouraged as it can lead to specificity issues. Consider restructuring the CSS to achieve the same effect without the important flag.
-className="max-w-sm pl-10 bg-transparent border-none dark:focus-visible:!border-[#c8c8c8] transition-all duration-200 placeholder:font-normal" +className="max-w-sm pl-10 bg-transparent border-none dark:focus-visible:border-[#c8c8c8] transition-all duration-200 placeholder:font-normal [&:focus-visible]:border-[#c8c8c8]"Also applies to: 55-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/components/nav-main.tsx(3 hunks)apps/web/components/pages/team/tasks/tasks-data-table.tsx(3 hunks)
🔇 Additional comments (3)
apps/web/components/nav-main.tsx (2)
161-161: LGTM! Consistent with parent menu styling.
The dark mode implementation for submenu items maintains visual consistency with parent menu items.
Line range hint 83-161: Verify dark mode color tokens and related components.
Let's ensure the dark mode implementation is complete across related components.
✅ Verification successful
Dark mode implementation is properly configured with CSS variables
The verification shows that the sidebar-accent color token is correctly defined in the Tailwind configuration using CSS variables (hsl(var(--sidebar-accent))). This is a proper implementation that supports both light and dark modes through CSS variable switching, rather than requiring explicit dark mode classes everywhere.
The dark mode classes in the component under review are consistent with this theming approach, where they're used to provide fallback styling while the CSS variables handle the main theming logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other sidebar components that might need dark mode updates
echo "Checking for related sidebar components..."
rg -l "sidebar|dark:" apps/web/components/
# Verify the sidebar-accent color token is properly defined
echo "Checking for sidebar-accent color token definition..."
rg "sidebar-accent" "apps/web/**/*.{css,scss,ts,tsx,js,jsx}"
Length of output: 5794
Script:
#!/bin/bash
# Let's check the tailwind config and theme files for the sidebar-accent color definition
echo "Checking tailwind and theme configurations..."
fd -e js -e ts -e json "tailwind|theme" apps/web/
# Once we find the config files, let's examine their content
echo "Checking color token definitions..."
rg -A 5 "colors|sidebar" apps/web/tailwind.config.js apps/web/theme.config.js 2>/dev/null || true
# Let's also check the specific dark mode implementation in sidebar.tsx
echo "Checking sidebar component implementation..."
rg "dark:" apps/web/components/ui/sidebar.tsx 2>/dev/null || true
Length of output: 1879
apps/web/components/pages/team/tasks/tasks-data-table.tsx (1)
10-10: LGTM: Clean import addition for internationalization.
The addition of the useTranslations import aligns with the project's internationalization strategy.
Fix colours for sidebar links in dark mode
Type of Change
Checklist
Previous screenshots
Please add here videos or images of previous status
Current screenshots
Please add here videos or images of previous status
Screen.Recording.2024-11-12.at.07.56.56.mov
Summary by CodeRabbit
New Features
Style
Bug Fixes