Skip to content

fix: Add dark mode and high contrast theme support to header components#231

Open
kr1shnac wants to merge 2 commits intoAOSSIE-Org:mainfrom
kr1shnac:fix/mobile-sidebar-theme
Open

fix: Add dark mode and high contrast theme support to header components#231
kr1shnac wants to merge 2 commits intoAOSSIE-Org:mainfrom
kr1shnac:fix/mobile-sidebar-theme

Conversation

@kr1shnac
Copy link

@kr1shnac kr1shnac commented Jan 14, 2026

Problem

The mobile sidebar (hamburger menu), notification popover, and profile popover were not respecting the user's selected theme. When users switched toDark mode orHigh Contrast mode, these components remained white/light, creating visual inconsistency and accessibility issues.

Before (Bug)

  • Mobile sidebar: Always white background regardless of theme
  • Notification popover: White background in dark mode
  • Profile popover: White background in dark mode

Solution

Demo Video:

https://drive.google.com/file/d/1cij7AQ_JKWPeK-KLRPpkOAYctJJFjeGt/view?usp=sharing

Replaced all hardcoded Tailwind color classes with theme-aware CSS variable classes that automatically adapt to the current theme:

Hardcoded Class Theme-Aware Class
bg-white bg-background
border-gray-200 border-border
text-gray-900 text-foreground
text-gray-500/600 text-muted-foreground
text-blue-600 text-primary
text-red-600 text-destructive
bg-gray-100 bg-muted
hover:bg-gray-50 hover:bg-muted

Files Changed

-frontend/src/components/Header.tsx

  • Mobile drawer/sidebar styling
  • Notification popover styling
  • Profile popover styling
  • Hamburger menu button
  • NavItem component

-frontend/src/components/Sidebar.tsx

  • Desktop sidebar borders

Testing

Manually tested all three themes:

  • Light mode - Components display correctly
  • Dark mode - Components use dark backgrounds with light text
  • High Contrast mode - Components use pure black background with high contrast text

Screenshots

Dark Mode - Mobile Sidebar

Before After
beforeimg updated

High Contrast Mode - Mobile Sidebar

Before After
beforeimg updated

Summary by CodeRabbit

  • New Features

    • Notifications now refresh automatically every 60 seconds for real-time updates.
  • Style

    • Updated header and sidebar visual styling with new color scheme and improved UI components.
    • Enhanced notification badges, status indicators, and user panel appearance for better clarity.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Frontend components receive styling updates applying new color tokens and UI refinements. The Header gains a 60-second polling mechanism to refresh notifications alongside existing triggers. Sidebar receives cosmetic formatting adjustments. No changes to component interfaces or exported signatures.

Changes

Cohort / File(s) Summary
Notification & Polling Logic
frontend/src/components/Header.tsx
Added periodic notification fetching via 60-second polling interval; refined functional state updates for notifications and unread counts; reorganized and expanded notification service imports.
UI & Styling Overhaul
frontend/src/components/Header.tsx, frontend/src/components/Sidebar.tsx
Updated color tokens (border-border, text-foreground, text-muted-foreground, primary colors); adjusted badges, dividers, hover states, avatar styling, user panel borders, nav active states, and drawer transitions; refactored import organization for lucide-react components.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • bhavik-mangla

Poem

🐰 Hop, poll, and refresh with glee,
Sixty seconds keeps us spry!
Colors dance in harmony,
New tokens make the UI fly,
A notification rabbit's sigh. 🔔✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main purpose of the changes: adding dark mode and high contrast theme support to header components through color token updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
frontend/src/components/Header.tsx (1)

83-92: Potential stale closure when checking notification read status.

Line 88 accesses notifications from closure after an async operation. If the notifications state changed during the await deleteNotification(id) call, the isRead check could use stale data, leading to an incorrect unread count.

Consider capturing the notification's read status before the async call:

♻️ Suggested fix
 const handleDeleteNotification = async (e: React.MouseEvent, id: string) => {
   e.stopPropagation();
+  const wasUnread = notifications.find((n) => n.id === id && !n.isRead);
   await deleteNotification(id);
   setNotifications((prev) => prev.filter((n) => n.id !== id));
-  // Update unread count if the deleted notification was unread
-  const notification = notifications.find((n) => n.id === id);
-  if (notification && !notification.isRead) {
+  if (wasUnread) {
     setUnreadCount((prev) => Math.max(0, prev - 1));
   }
 };

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb07eaf and 3381832.

📒 Files selected for processing (2)
  • frontend/src/components/Header.tsx
  • frontend/src/components/Sidebar.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Header.tsx (1)
frontend/src/services/notificationService.ts (1)
  • markAllNotificationsAsRead (44-56)
🔇 Additional comments (10)
frontend/src/components/Sidebar.tsx (3)

2-14: LGTM!

Import statement reorganization and quote style normalization looks good.


18-28: LGTM!

The use of border-border and bg-background theme tokens correctly enables theme support for the sidebar container and logo section.


84-88: LGTM!

The NavItem styling correctly uses theme-aware tokens (bg-secondary, text-secondary-foreground, text-foreground, hover:bg-muted) for active and inactive states, ensuring consistent theme support.

frontend/src/components/Header.tsx (7)

3-12: LGTM!

Clean import reorganization for lucide-react icons.


33-39: LGTM!

Consolidated notification service imports improve readability.


61-66: LGTM!

Good addition of polling for notifications. The cleanup correctly clears the interval when user changes or the component unmounts.


139-216: LGTM!

Theme token updates for the header and notification popover look correct. The use of border-border, text-foreground, text-muted-foreground, bg-primary/10, text-primary, hover:bg-muted, and text-destructive aligns with the PR objectives for dark mode and high contrast support.


219-283: LGTM!

Profile popover and hamburger button styling correctly updated with theme-aware tokens (border-border, text-foreground, text-muted-foreground, bg-muted, text-destructive, hover:bg-destructive/10).


286-340: LGTM!

Mobile drawer styling correctly updated with bg-background, border-border, text-foreground, and text-muted-foreground tokens to support theme consistency.


352-368: LGTM!

NavItem styling in Header mirrors the Sidebar implementation, using consistent theme tokens for active (bg-secondary text-secondary-foreground) and inactive (text-muted-foreground hover:bg-muted hover:text-foreground) states.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant