fix: improve contrast of unselected year buttons in dark mode#344
fix: improve contrast of unselected year buttons in dark mode#344HoussemEddineChaouch wants to merge 4 commits into
Conversation
Welcome to OSSfolio, @HoussemEddineChaouch! 🎉Thank you for opening this pull request and contributing to the open-source community! 🚀 To ensure a smooth review process, please make sure you have:
We will review your PR as soon as possible. Happy coding! 💻✨ |
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesContrast Fix and Formatting Cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
|
@HoussemEddineChaouch fix the ci build error |
Fixed ,build failure was unrelated to the contrast fix, it was a lockfile drift ( |
can u still fix the build error and then i will merge @HoussemEddineChaouch |
…tton-contrast # Conflicts: # package-lock.json # src/components/profile/HeatmapWithYearNav.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/components/profile/HeatmapWithYearNav.tsx`:
- Around line 293-294: Replace the index-based key on the contribution grid week
element in HeatmapWithYearNav with a stable date-based key derived from the
week’s identity, such as the first day’s date, while preserving a safe fallback
for empty weeks.
- Around line 263-266: Update the unselected text color for both toggle buttons
in the heatmap view, including the “Last 365 Days” button and the “Calendar
Year” button, to use var(--color-nav-mute) instead of var(--color-ink-mute);
preserve the existing selected-state colors and background behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: be0b591e-7601-4134-ba17-95f9377cd7b3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/components/profile/HeatmapWithYearNav.tsx
| color: | ||
| viewMode === "calendar" ? "#171717" : "var(--color-ink-mute)", | ||
| backgroundColor: | ||
| viewMode === "calendar" ? "#3ecf8e" : "transparent", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Toggle button unselected text still uses var(--color-ink-mute).
The "Calendar Year" toggle button (and its "Last 365 Days" sibling at Line 246) uses var(--color-ink-mute) for the unselected state — the same token that caused the low-contrast issue this PR fixes for year buttons. In dark mode, these toggle buttons will exhibit the same readability problem. Consider switching both to var(--color-nav-mute) for consistency.
♿ Proposed fix for toggle button contrast consistency
fontSize: "12px",
fontWeight: viewMode === "365" ? 600 : 400,
- color: viewMode === "365" ? "`#171717`" : "var(--color-ink-mute)",
+ color: viewMode === "365" ? "`#171717`" : "var(--color-nav-mute)",
backgroundColor: viewMode === "365" ? "`#3ecf8e`" : "transparent",And similarly for the calendar button:
fontWeight: viewMode === "calendar" ? 600 : 400,
color:
- viewMode === "calendar" ? "`#171717`" : "var(--color-ink-mute)",
+ viewMode === "calendar" ? "`#171717`" : "var(--color-nav-mute)",
backgroundColor:📝 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.
| color: | |
| viewMode === "calendar" ? "#171717" : "var(--color-ink-mute)", | |
| backgroundColor: | |
| viewMode === "calendar" ? "#3ecf8e" : "transparent", | |
| color: | |
| viewMode === "calendar" ? "`#171717`" : "var(--color-nav-mute)", | |
| backgroundColor: | |
| viewMode === "calendar" ? "`#3ecf8e`" : "transparent", |
🤖 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 `@src/components/profile/HeatmapWithYearNav.tsx` around lines 263 - 266, Update
the unselected text color for both toggle buttons in the heatmap view, including
the “Last 365 Days” button and the “Calendar Year” button, to use
var(--color-nav-mute) instead of var(--color-ink-mute); preserve the existing
selected-state colors and background behavior.
| <div | ||
| key={wi} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider stable keys for contribution grid weeks.
Static analysis flags the use of array index keys (key={wi}). If the week array can reorder (e.g., when switching between "365" and "calendar" views changes the filtered set), React may reuse DOM nodes incorrectly. This is a pre-existing pattern, but if week identity is stable, consider using a date-based key (e.g., week.days[0]?.date) instead.
🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 294-294: Your users can see & submit the wrong data when this list reorders.
Use a stable key from your data so reordered items keep the right state and DOM.
(no-array-index-key)
🤖 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 `@src/components/profile/HeatmapWithYearNav.tsx` around lines 293 - 294,
Replace the index-based key on the contribution grid week element in
HeatmapWithYearNav with a stable date-based key derived from the week’s
identity, such as the first day’s date, while preserving a safe fallback for
empty weeks.
Source: Linters/SAST tools
|
@HoussemEddineChaouch pls comment here whether this pr is part of ELUSOC |
|
@PRODHOSH No, this isn't part of ELUSOC ,I just came across the issue and wanted to contribute. |
Summary
This PR improves accessibility for users by enhancing the visibility of the unselected year text in the heatmap. Previously, the text had low contrast, making it difficult to notice, especially in dark mode. This update increases the contrast in dark mode and adjusts it in light mode as well, resulting in better readability and an improved user experience.
Related Issue
Closes #286
Type of Change
Changes Made
--color-nav-muteCSS variable inglobals.css(defaults to--color-ink-mutein light mode,#c2c2c2in dark mode)HeatmapWithYearNav.tsxto use--color-nav-muteinstead of--color-ink-mutefor unselected year button textAI Usage
Tool Name : Used Claude Code for coding
Screenshots (if UI change)
Checklist
mainconsole.logleft insrc/DESIGN.mdand followed the design systemfix:)Summary by CodeRabbit