Skip to content

Fix tooltip accent line and colors #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rdmueller merged 1 commit into from
Mar 16, 2026
Merged

Fix tooltip accent line and colors #18

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/components/RadarChart.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@
registerUpdater = null,
}) {
// Safe fallbacks to prevent math from crashing if data is missing
const safeDims = dimensions || [];

Check warning on line 123 in src/components/RadarChart.jsx

View workflow job for this annotation

GitHub Actions / lint-and-build 

The 'safeDims' logical expression could make the dependencies of useCallback Hook (at line 295) change on every render. To fix this, wrap the initialization of 'safeDims' in its own useMemo() Hook
const safeVals = values || {};

Check warning on line 124 in src/components/RadarChart.jsx

View workflow job for this annotation

GitHub Actions / lint-and-build 

The 'safeVals' logical expression could make the dependencies of useCallback Hook (at line 295) change on every render. To fix this, wrap the initialization of 'safeVals' in its own useMemo() Hook

const cx = size / 2;
const cy = size / 2;
Expand Down Expand Up @@ -407,32 +407,31 @@
width={tooltipW}
height={tooltipH}
rx={10}
fill="var(--bg-card)"
fill="#1e293b"
Copy link

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded colors bypass theme system and contain an inconsistency.

While the PR intentionally uses fixed dark colors for an "always-dark" tooltip, this approach:

  1. Violates the CSS custom properties guideline for theme management.
  2. Contains an inconsistent color: Line 424 uses #475569, which is the light mode --text-muted value. The dark mode equivalent is #cbd5e1. This will make the "/ 4" suffix appear darker than expected in the dark-styled tooltip.

If always-dark tooltips are a deliberate design pattern, consider defining dedicated CSS variables (e.g., --tooltip-bg, --tooltip-text-muted) in theme.js that remain constant across themes. This preserves the design intent while maintaining the theme system's extensibility.

Suggested fix for color consistency 
-            <tspan fill="#475569" fontSize="11" fontWeight="400">
+            <tspan fill="#cbd5e1" fontSize="11" fontWeight="400">

Or better, add dedicated tooltip variables to theme.js that don't change between themes:

// In theme.js, add to both palettes:
"--tooltip-bg": "#1e293b",
"--tooltip-text": "#94a3b8", 
"--tooltip-text-muted": "#cbd5e1",

Then reference these in the component to maintain theme system compliance.

As per coding guidelines: "Implement theme management via CSS custom properties defined in src/theme.js".

Also applies to: 419-419, 424-424, 431-431, 438-438

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/RadarChart.jsx` at line 413, The component RadarChart.jsx
currently hardcodes tooltip colors (e.g., fill="#1e293b" and the inconsistent
"#475569" used for the "/ 4" suffix) instead of using CSS custom properties; add
dedicated tooltip variables to src/theme.js (for example --tooltip-bg,
--tooltip-text, --tooltip-text-muted) and update RadarChart.jsx to replace all
hardcoded color literals (the fill and any color props rendering the muted
suffix and other tooltip text) with references to those new CSS variables so the
tooltip remains consistently dark while honoring the theme system.

stroke={tc}
strokeWidth={1.5}
filter="url(#tt-shadow)"
/>
<rect x={tipX} y={tipY} width={4} height={tooltipH} rx={4} fill={tc} />
<text x={tipX + 14} y={tipY + 19} fill="var(--text-secondary)" fontSize="11" fontWeight="500">
<text x={tipX + 14} y={tipY + 19} fill="#94a3b8" fontSize="11" fontWeight="500">
{tooltip.label}
</text>
<text x={tipX + 14} y={tipY + 39} fill={tc} fontSize="19" fontWeight="800">
{tooltip.value}
<tspan fill="var(--text-muted)" fontSize="11" fontWeight="400">
<tspan fill="#475569" fontSize="11" fontWeight="400">
{" "}
/ 4
</tspan>
</text>
<polygon
points={`${tipX + tooltipW / 2 - 7},${tipY + tooltipH} ${tipX + tooltipW / 2 + 7},${tipY + tooltipH} ${tipX + tooltipW / 2},${tipY + tooltipH + 10}`}
fill="var(--bg-card)"
fill="#1e293b"
stroke={tc}
strokeWidth={1.5}
strokeLinejoin="round"
/>
<polygon
points={`${tipX + tooltipW / 2 - 5},${tipY + tooltipH - 1} ${tipX + tooltipW / 2 + 5},${tipY + tooltipH - 1} ${tipX + tooltipW / 2},${tipY + tooltipH + 8}`}
fill="var(--bg-card)"
fill="#1e293b"
/>
</g>
)}
Expand Down
Loading