Skip to content

Fix nav links color issue in light theme (#461)#504

Open
AyalaZisman wants to merge 1 commit into
Deadlink-Hunter:mainfrom
AyalaZisman:fix-light-mode-text
Open

Fix nav links color issue in light theme (#461)#504
AyalaZisman wants to merge 1 commit into
Deadlink-Hunter:mainfrom
AyalaZisman:fix-light-mode-text

Conversation

@AyalaZisman

@AyalaZisman AyalaZisman commented Jul 20, 2026

Copy link
Copy Markdown

Fixes #461

Description

Fixes an issue where navigation links were not clearly visible in light theme due to incorrect color handling.

Changes

  • Updated navbar link color handling to use the correct theme color.
  • Improved text visibility in light theme.

Testing

  • Tested light theme
  • Tested dark theme
  • Verified behavior after page refresh
  • Passed ESLint and Storybook build
image image

Summary by CodeRabbit

  • Bug Fixes
    • Improved theme toggle icon visibility and consistency across light and dark modes.
    • Updated navigation and language selector text colors for better theme compatibility.
    • Preserved active-link highlighting while improving readability of inactive links.

@github-actions

Copy link
Copy Markdown

👋 Thanks for opening this Pull Request! 🎉

We'll review it as soon as possible.

Before we proceed, please double check:

  • The PR title & description clearly explain the changes.
  • All checks/tests pass.
  • You've linked it to an issue (if applicable).

Happy contributing! 🚀

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5e3320f-44b1-43ee-96d4-4ec8d6102150

📥 Commits

Reviewing files that changed from the base of the PR and between 431f497 and 50421c5.

📒 Files selected for processing (2)
  • src/components/Navbar/ThemeToggle.tsx
  • src/components/Navbar/styles.ts

📝 Walkthrough

Walkthrough

Navbar icon, link, and language selector colors now use var(--mantine-color-text) while preserving existing theme toggling and active-link colors.

Changes

Navbar theme colors

Layer / File(s) Summary
Theme-aware navbar color rendering
src/components/Navbar/ThemeToggle.tsx, src/components/Navbar/styles.ts
Theme toggle icons, desktop and mobile link fallbacks, and language selector text now use the Mantine text CSS variable; active links retain the cyan-filled variable.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: tamir198

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR improves link and selector readability, but it does not address the GitHub icon hover state or invisible hamburger-menu text from #461. Update the GitHub button/icon styling and mobile menu text colors so all navbar elements are readable in the light theme.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the navbar light-theme color fix and matches the main change.
Out of Scope Changes check ✅ Passed The changes stay within navbar color and visibility fixes and do not introduce unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

input: {
backgroundColor: 'transparent',
color: theme.colors.primary[1],
color: 'var(--mantine-color-text)',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the theme object like it was before? i dont want to mix global variables in here

>
{theme ? <IconMoon size={16} /> : <IconSun size={16} />}
{theme ? (
<IconMoon size={16} color="var(--mantine-color-text)" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to pass style object to this? instead of inline like this? , also lets use the colors from the theme

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.

Text not clearly visible due to color in nav links in light theme

2 participants