Skip to content

feat(frontend): theme charts and the correlation heatmap for dark mode - #532

Merged
ivantha merged 1 commit into
c2siorg:mainfrom
rishiraj38:fix/210-chart-dark-mode
Oct 2, 2026
Merged

feat(frontend): theme charts and the correlation heatmap for dark mode#532
ivantha merged 1 commit into
c2siorg:mainfrom
rishiraj38:fix/210-chart-dark-mode

Conversation

@rishiraj38

@rishiraj38 rishiraj38 commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Description

ChartRenderer and CorrelationHeatmap landed before dark mode and hardcode a light palette, so on a dark background they render as bright panels: grey grid and axis lines, a white tooltip and hover cursor, white-midpoint heatmap cells and a white diagonal in the PNG export.

This points the chart chrome at the theme tokens from index.css instead of a second palette in the chart files:

  • ChartRenderer.tsx: grid, axis line, tick and label text, tooltip box (background, border, text) and the tooltip cursor reference the --app-* custom properties; pie slices are separated with --app-surface instead of #fff. The .dark block restyles them, so the navbar toggle repaints an open chart without a rerender.
  • CorrelationHeatmap.tsx: cellColor mixes the red/blue hue toward --app-surface with color-mix(), so the diverging scale is centred on the panel colour in both themes; textColor keeps white on strong fills and uses --app-foreground otherwise. Null cells, the diagonal, the highlights dividers and the hover ring use the existing theme classes, and the highlight values use text-blue-600 dark:text-blue-400 / text-red-600 dark:text-red-400. The legend gradient passes through --app-surface.
  • The SVG export cannot resolve var(), so exportPalette reads the token values off the document element at click time and buildMatrixSvg bakes them in. The exported PNG matches the theme it was downloaded from.

Series colours and the pie ramp are data colours and are unchanged.

Fixes #210

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

How Has This Been Tested?

  • ChartRenderer.test.tsx (new): mounts a bar chart under a dark ThemeProvider and asserts the grid, axis, ticks and tooltip reference the theme tokens; asserts pie slices use the surface token; toggles the theme through useTheme and asserts the chart keeps the same token references while the dark class flips.

  • CorrelationHeatmap.test.tsx: three new cases for the color-mix() cell fills and text colours, the null cell and legend classes, the highlight value classes, and the exported SVG carrying the resolved dark token values.

  • Manual: ran the app locally, rendered bar (with tooltip), area, scatter, pie and the heatmap (matrix, highlights, PNG export) in light and dark mode before and after; screenshots attached.

  • Existing tests pass

  • New tests added

  • Manual testing

Screenshots (if applicable)

Taken from the app running locally, main vs this branch.

Dark mode, before Dark mode, after

Light mode, after:

Light, after

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review
  • I have added/updated documentation as needed
  • My changes generate no new warnings
  • Tests pass locally

@ivantha ivantha added enhancement New feature or request ready-for-maintainer Reviewed; looks mergeable, awaiting maintainer labels Oct 2, 2026
@ivantha

ivantha commented Oct 2, 2026

Copy link
Copy Markdown
Member

Automated review summary

Triage: enhancement | Recommendation: ready-for-maintainer

Findings

  • [nit/test] dataloom-frontend/src/Components/profiling/__tests__/CorrelationHeatmap.test.tsx:162 The test sets inline custom properties on document.documentElement and only clears them with root.removeAttribute("style") on the last line. If any assertion before it fails, the dark tokens leak into later tests. Move the cleanup to afterEach or a try/finally.
  • [nit/test] dataloom-frontend/src/Components/visualizations/__tests__/ChartRenderer.test.tsx:58 The ChartRenderer tests query Recharts internal class names such as .recharts-cartesian-grid line, .recharts-default-tooltip and .recharts-pie-sector path. A Recharts upgrade can break them without any real regression. The 3s waitFor for pie sectors also depends on entry animation timing.
  • [nit/quality] dataloom-frontend/src/Components/profiling/CorrelationHeatmap.tsx:183 The export bakes the heatmap fills as color-mix(in srgb, ...) strings inside the standalone SVG. That relies on the rasterizer supporting color-mix() in SVG fill attributes, which modern browsers do. The PR's manual export screenshots cover this, but no automated test rasterizes it, since jsdom has no canvas.

Conventions

  • PR title: follows Conventional Commits
  • Description: clear and complete
  • Commits (1): clean, conventional history

This PR replaces the hardcoded light palette in ChartRenderer and CorrelationHeatmap with the existing --app-* theme tokens. I confirmed that every token and Tailwind alias it uses (--app-grid, --app-elevated, --app-disabled-foreground, bg-surface-hover, ring-foreground and the rest) is defined in index.css for both light and dark. The PNG export correctly resolves the token values at click time, since a standalone SVG cannot read var(). I found no correctness or security problems, only minor test-robustness nits (the leaked-style cleanup and the reliance on Recharts internal class names). The PR follows Conventional Commits, has a single commit, and its description links the issue it closes (#210). It is fine to merge once the repo's CI checks pass.

Generated by /review-prs. A maintainer will follow up.

@rishiraj38 @ivantha
feat(frontend): theme charts and the correlation heatmap for dark mode
3778581 
ChartRenderer and CorrelationHeatmap were written before dark mode
landed and hardcode a light palette, so on a dark background they render
as bright panels: grey grid and axis lines, a white tooltip and hover
cursor, white-midpoint heatmap cells and a white diagonal in the PNG
export.

Point the chart chrome at the theme tokens from index.css instead. Grid,
axis, ticks, tooltip, cursor and pie slice borders reference the --app-*
custom properties, so the .dark block restyles them and the navbar
toggle repaints an open chart without a rerender. The heatmap mixes its
diverging red/blue scale toward the surface token with color-mix(), so
0 sits on the panel colour in both themes and strong cells keep white
text. Null cells, the diagonal, dividers, the hover ring and the
highlight values use the existing theme classes. The SVG export cannot
resolve var(), so it reads the token values off the document element at
click time and bakes them in.

Adds a ChartRenderer test that mounts a chart under a dark ThemeProvider
and extends the heatmap tests to cover the mixed fills, the highlight
classes and the exported palette.

Fixes c2siorg#210
@ivantha
ivantha force-pushed the fix/210-chart-dark-mode branch from 7b733c4 to 3778581 Compare October 2, 2026 22:08
@ivantha
ivantha merged commit 0e1b184 into c2siorg:main Oct 2, 2026
8 checks passed
@rishiraj38

rishiraj38 commented Oct 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@ivantha the Slack invite on the GSoC page has expired. Can you share a new one? Thanks.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement New feature or request ready-for-maintainer Reviewed; looks mergeable, awaiting maintainer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature] Theme charts and the correlation heatmap for dark mode

2 participants

@rishiraj38 @ivantha