feat: enhance dark theme support in search and navigation components#5471
feat: enhance dark theme support in search and navigation components#5471hasithasandunlakshan wants to merge 3 commits into
Conversation
hasithasandunlakshan
requested review from
akshatnema,
anshgoyalevil,
derberg and
sambhavgupta0705
as code owners
hasithasandunlakshan
requested review from
Mayaleeeee,
asyncapi-bot-eve and
princerajpoot20
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## website-ui #5471 +/- ##
============================================
Coverage 100.00% 100.00%
============================================
Files 22 22
Lines 906 906
Branches 171 171
============================================
Hits 906 906 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@hasithasandunlakshan Have a look at sonar issues in your PR |
…e with dataset for theme management
Done. |
| document.documentElement.classList.add('dark'); | ||
| document.documentElement.dataset.theme = 'dark'; |
There was a problem hiding this comment.
why we need this additional variable? can't we use the one present above
There was a problem hiding this comment.
Here The theme variable only stores the value read from localStorage; CSS cannot access that JavaScript variable directly.
classList.add('dark')is used by Tailwind dark mode.dataset.theme = 'dark'addsdata-theme="dark", which is required by Algolia DocSearch’s built-in dark theme selector:html[data-theme=dark].
But here i tested by removing that line it is working correctly. because , DarkModeToggle.tsx also sets document.documentElement.dataset.theme = 'dark' after React mounts. Since DocSearch is opened after the app has mounted, that may be enough.I removed it.
| if (shouldUseDark) { | ||
| document.documentElement.dataset.theme = 'dark'; | ||
| } else { | ||
| delete document.documentElement.dataset.theme; | ||
| } | ||
| setIsDark(shouldUseDark); | ||
| }, []); | ||
|
|
||
| const toggleDarkMode = () => { | ||
| const newTheme = !isDark; | ||
|
|
||
| document.documentElement.classList.toggle('dark', newTheme); | ||
| if (newTheme) { | ||
| document.documentElement.dataset.theme = 'dark'; | ||
| } else { | ||
| delete document.documentElement.dataset.theme; | ||
| } |
There was a problem hiding this comment.
@hasithasandunlakshan too many if else statement, is there a way to make it clean and simple
There was a problem hiding this comment.
- we can use reusable function like this
function applyTheme(isDark: boolean) {
document.documentElement.classList.toggle('dark', isDark);
if (isDark) {
document.documentElement.dataset.theme = 'dark';
return;
}
delete document.documentElement.dataset.theme;
}
There was a problem hiding this comment.
@princerajpoot20 , One thing , since this is very small and tightly coupled to the toggle’s behavior, I kept it inside DarkModeToggle.tsx. If you prefer, I can move it to a separate helper file like theme.ts.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/navigation/DocsMobileMenu.tsx (1)
50-50: ⚡ Quick winConsider adding dark mode hover background.
The button includes
hover:bg-secondary-100which applies in both light and dark modes. In dark mode,secondary-100(likely a light color) may create low contrast or visual inconsistency when hovering over the dark card background. Consider addingdark:hover:bg-dark-backgroundor a similar dark-appropriate hover background color to ensure consistent appearance.🎨 Suggested addition
-className='flex w-full items-center space-x-3 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-left text-sm text-gray-700 shadow-sm transition-all duration-500 ease-in-out dark:border-dark-text dark:bg-dark-card dark:text-dark-text dark:hover:border-secondary-500 dark:hover:text-secondary-500 hover:border-secondary-500 hover:bg-secondary-100 hover:text-secondary-500' +className='flex w-full items-center space-x-3 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-left text-sm text-gray-700 shadow-sm transition-all duration-500 ease-in-out dark:border-dark-text dark:bg-dark-card dark:text-dark-text dark:hover:border-secondary-500 dark:hover:bg-dark-background dark:hover:text-secondary-500 hover:border-secondary-500 hover:bg-secondary-100 hover:text-secondary-500'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/navigation/DocsMobileMenu.tsx` at line 50, The hover background is only set for light mode via the CSS class in the className on the interactive element in DocsMobileMenu (the button/container with className='... hover:bg-secondary-100 ...'); update that class list to include a dark-mode hover background (e.g., add dark:hover:bg-dark-background or dark:hover:bg-dark-card-hover) so hovering in dark theme uses an appropriate dark color; keep the existing light hover class (hover:bg-secondary-100) and add the dark:hover:... token to the same className on the element in DocsMobileMenu.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@components/navigation/DocsMobileMenu.tsx`:
- Line 50: The hover background is only set for light mode via the CSS class in
the className on the interactive element in DocsMobileMenu (the button/container
with className='... hover:bg-secondary-100 ...'); update that class list to
include a dark-mode hover background (e.g., add dark:hover:bg-dark-background or
dark:hover:bg-dark-card-hover) so hovering in dark theme uses an appropriate
dark color; keep the existing light hover class (hover:bg-secondary-100) and add
the dark:hover:... token to the same className on the element in
DocsMobileMenu.tsx.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b0e51c0-2c52-4e02-8b64-dabcf750e36a
📒 Files selected for processing (4)
components/AlgoliaSearch.tsxcomponents/DarkModeToggle.tsxcomponents/navigation/DocsMobileMenu.tsxpages/_document.tsx
…readability and maintainability style(DocsMobileMenu): update search button hover background for better dark theme compatibility fix(_document): remove redundant dataset assignment for dark theme fix(package-lock): mark dependencies as peer
|
2 participants
Description
data-theme="dark"alongside the existingdarkclass so Algolia DocSearch uses its dark theme.New behaviour
Screen.Recording.2026-05-25.at.00.16.56.mov
Related issue(s)
Fixes #5470
Summary by CodeRabbit
Style
Bug Fixes