Charts: add HeatmapChart (HTML/CSS implementation)#50065
Conversation
Define heatmapChart settings on ChartTheme/CompleteChartTheme plus a default entry. Only values with no other home live here: the scale color (primaryColor, kept for parity with the other HTML charts) and the compact contribution-graph sizing. Cell gap, radius, in-cell value size, the empty-cell color and the selection ring resolve from WPDS tokens in CSS instead, so they track the active design system theme.
Render the heatmap as a CSS Grid of cells shaded with CSS color-mix against the resolved primary color (prop > theme > palette colors[0]), making the scale CSS-first and adaptive to the chart background. Empty cells default to the WPDS track color; compact mode renders fixed squares. Full ARIA grid with 2D arrow-key navigation, and tooltips on hover and keyboard focus. Includes the calendar/contribution-data transform and the color-scale legend, and wires the package exports. The keyboard-tooltip effect reads containerBounds from a ref rather than depending on it: useTooltipInPortal returns a fresh bounds object each render, so depending on it would loop (showTooltip -> render -> new bounds -> effect -> showTooltip).
Render/empty-state, ARIA grid + 2D keyboard navigation, hover and keyboard tooltips, compact sizing, primaryColor resolution (prop, theme, palette fallback), and the value-extent/normalize and calendar-transform helpers (including invalid-date and duplicate-day edge cases).
Stories (default, compact, calendar with weekStartsOn control, fixed dimensions, aspect ratio, composition legend, error state), the docs/API MDX pages, shared sample datasets (activity matrix + calendar series), the charts-library index entry, and the changelog.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
github-actions
Bot
added
the
[Status] Needs Author Reply
|
@copilot review this PR. Leave your feedback as review comments only — do NOT push commits, apply suggestions, or modify the branch in any way. I will make all code changes myself. |
I reviewed the PR changes and didn’t find any blocking functional, accessibility, or API-compatibility issues to call out in review comments. |
Code Coverage SummaryCoverage changed in 3 files.
7 files are newly checked for coverage. Only the first 5 are listed here.
|
Drop comments that restate the code or the package's given conventions (WPDS theming, cross-references to the other HTML charts), keeping only the non-obvious why: the containerBounds-ref loop guard, the repeat()/var() constraint, the 15% color-mix floor, and the aria-activedescendant focus model. Docs keep WPDS theming notes but drop the other-HTML-chart references.
The HTML heatmap has no SVG <title> element, so the no-native-title check can't fail; the aria-label assertion in the same test is the real check.
Use getByRole('gridcell', { name }) for the accessible-name check and
within(row).getAllByRole('gridcell') for the ARIA-hierarchy check, so the
testing-library/no-node-access rule no longer needs disabling.
|
@copilot review this PR. Leave your feedback as review comments only — do NOT push commits, apply suggestions, or modify the branch in any way. I will make all code changes myself. |
Large values (e.g. 748,500) overflowed the cell when rendered in full. Use formatNumberCompact for the in-cell value (748.5K) so it fits; the tooltip and aria-label keep the full, precise value. Adds a LargeValues story and sample dataset.
The in-cell value used white text whenever the data value was above the midpoint, but with a light palette (e.g. colors[0] #98C8DF) even the fullest cell is light, so white numbers failed contrast. Decide the text color from the cell's actual blended-fill luminance instead: light text only where it out-contrasts dark text (W3C 0.179 threshold). Adds reusable relativeLuminance and prefersLightText to color-utils (composed with the existing lightenHexColor for the over-white blend) rather than bespoke color math in the chart.
|
@copilot review this PR. Leave your feedback as review comments only — do NOT push commits, apply suggestions, or modify the branch in any way. I will make all code changes myself. |
adamwoodnz
removed
the
[Status] Needs Author Reply
Replace the hardcoded #fff with --wpds-color-fg-interactive-neutral-strong, the WPDS light foreground for content on a strong/colored surface (content tokens have no strong/inverse variant). Pairs with the neutral dark default.
The heatmap pinned the responsive wrapper's debounced, measured pixel width and height onto its container, so on resize the grid snapped to stale dimensions 300ms behind the cursor instead of reflowing with its parent. CSS Grid can do the reflow for free, but the inline pixel sizes were overriding it. Size the chart purely in CSS instead, matching the conversion-funnel chart: width/height flow at 100%, the grid uses flex to fill remaining space, and a min-height floor keeps it usable when the parent has no explicit height. Aspect-ratio mode now expresses the ratio as a CSS aspect-ratio on the responsive wrapper so its height tracks width fluidly rather than snapping to a debounced measurement; this also makes aspect-ratio sizing fluid for the other charts that share the wrapper. Both axes now reflow continuously in the same layout pass, with no JavaScript in the sizing path.
The Accessibility section listed value labels and color cues but omitted that in-cell text picks light or dark from the blended fill luminance, which is what keeps numbers legible across the scale and under any primary color. Document it alongside the other a11y guarantees.
🤖 Codex ReviewAutomated review generated locally with SummaryThe heatmap addition largely typechecks and tests pass, but the responsive aspect-ratio change mis-sizes wrappers when Findings[P2] Keep aspect-ratio sizing aligned with When a responsive chart uses [P2] Honor fixed dimensions in the unresponsive heatmap — When consumers import the newly exported Posted on behalf of @adamwoodnz — this is an automated Codex review and has not yet been triaged or addressed. |
The CSS aspect-ratio was applied to a wrapper that stays width: 100%, while the wrapped chart is sized from the maxWidth-capped measured width. In a container wider than maxWidth (default 1200) with an aspectRatio set, the wrapper reserved height for the full container width while the chart only filled the capped width — e.g. a 2000px parent with aspectRatio 0.4 gave a 2000x800 wrapper around 1200-wide content, leaving a block of empty space below the chart. Cap the wrapper's width at maxWidth in the aspect-ratio branch so the CSS-derived height matches the capped content. Scoped to that branch, so non-aspect-ratio charts keep their existing full-width wrapper. Reported by a Codex review of the HeatmapChart PR.
HeatmapChartInternal stopped reading width/height when sizing moved to CSS, so consumers of the exported HeatmapChartUnresponsive that pass width or height got them silently ignored — the chart fell back to 100% plus the grid min-height and could not render the documented fixed size without a wrapper of their own. Apply width/height inline again, and have the responsive export drop the wrapper's measured pixels (passing width/height undefined) so its grid still fills the container via CSS and reflows fluidly. Net: the unresponsive export pins explicit dimensions; the responsive export stays unpinned. Cover both with tests. Reported by a Codex review of the HeatmapChart PR.
|
@claude — requesting a final review. This is the HTML/CSS-first HeatmapChart (alternative to the SVG/visx branch in #49855). Copilot's rounds are complete; a local Codex review was run and its two findings are addressed. Please focus on:
Gates are green locally: type-check, ESLint, Stylelint, and the full charts suite (963 tests). Thanks! |
|
Claude finished @adamwoodnz's task in 3m 59s —— View job Final review — HTML/CSS HeatmapChartReviewed the implementation against the three focus areas. This is a clean, well-tested PR — both Codex findings are correctly addressed and the a11y story is strong. Notes below, nothing blocking.
✅ Responsive sizing — correctThe two-export split does what the description claims:
✅
|
handleCellMouseMove closed over containerBounds, a fresh object from useTooltipInPortal on every render, so the callback was rebuilt each render and reattached to every cell. Read the bounds from the existing containerBoundsRef — the same pattern the keyboard-tooltip effect uses — so the callback memoizes and the two tooltip paths stay consistent. Raised as a nit in the @claude review.
The shared HOC now expresses aspectRatio as a CSS aspect-ratio and caps the wrapper at maxWidth, but nothing asserted it. Add tests for the CSS aspect-ratio value and the maxWidth cap, plus its omission when an explicit width is passed, so this shared behavior is locked in. Raised as a nit in the @claude review.
The `<Source>` example blocks authored their JSX at the template's base column, so Storybook's dedent stripped the props' indentation and the examples rendered with every prop flush against the component tag. Indent the JSX body one level (imports stay flush), matching the convention the other chart docs already use, so the dedent preserves the prop indentation. Kept to tabs throughout — no tab/space mix.
adamwoodnz
added
[Status] Needs Review
kangzj
left a comment
kangzj
left a comment
There was a problem hiding this comment.
Independent review — solid, well-tested implementation. CSS-first is the right call for a discrete grid, the public API matches the SVG branch as claimed, and coverage (calendar builder, keyboard nav, theming resolution, contrast utils) is genuinely thorough. No blocking bugs.
Left 5 inline notes: (1) the shared withResponsive change affects all aspect-ratio charts, not just the heatmap — worth verifying no regression or splitting out; (2) a circular import between the chart and its legend; (3) a minor ARIA grid → row structural nit; (4) the in-cell contrast calc implicitly assumes a light background; (5) one vacuous test assertion.
Nice touches worth calling out: the max = 0 → undefined slice trap is avoided, getNormalizedValue handles the flat-extent case, buildCalendarHeatmapData filters unparseable dates + documents last-write-wins for duplicate days (both tested), and the keyboard-tooltip effect reads containerBounds from a ref to avoid a render loop.
| hasAspectRatio && aspectRatio | ||
| ? { | ||
| aspectRatio: `${ 1 / aspectRatio }`, | ||
| maxWidth: width === undefined ? maxWidth : undefined, |
There was a problem hiding this comment.
This isn't heatmap-specific — it applies a CSS aspect-ratio and a maxWidth cap to the wrapper <div> for every chart that uses aspectRatio (line, bar, funnel, …). Previously the wrapper was width: 100% with no cap (only the inner chart width was capped via Math.min(parentWidth, maxWidth)); now the wrapper itself caps at maxWidth in wide containers.
That's arguably more correct (removes dead space to the right of the chart), but it's a visible layout change to shared infra bundled into a new-chart PR. The two added unit tests cover the emitted style, not visual layout — worth confirming no regression on existing aspect-ratio stories, or splitting this out from the heatmap change. Not a blocker.
| import { Stack, Text } from '@wordpress/ui'; | ||
| import { useContext } from 'react'; | ||
| import { useGlobalChartsTheme } from '../../../providers'; | ||
| import { HeatmapContext } from '../heatmap-chart'; |
There was a problem hiding this comment.
Circular import: heatmap-chart.tsx imports HeatmapLegend from ./private, and this file imports HeatmapContext back from ../heatmap-chart. It works today because the context is only referenced at render time (ES live bindings resolve by then), but it's fragile — a future eager use of HeatmapContext at module scope here would break. Cheap fix: move HeatmapContext into its own small module under private/ that both import. Not a blocker.
| style={ gridStyle as CSSProperties } | ||
| > | ||
| { /* Corner gutter + column labels; aria-hidden, since each cell's label carries the text. */ } | ||
| <span aria-hidden="true" /> |
There was a problem hiding this comment.
The corner gutter and column-label <span>s are direct children of role="grid" rather than inside a role="row". They're aria-hidden so functionally fine and AT-invisible, but a strict validator expects grid → row → gridcell only. Wrapping the header cells in a role="row" (or role="presentation") would make it airtight. Low priority.
| const cellHasLightText = ( intensity: number ): boolean => | ||
| isValidHexColor( primaryHex ) && | ||
| prefersLightText( | ||
| lightenHexColor( primaryHex, 1 - ( CELL_MIX_FLOOR + ( 1 - CELL_MIX_FLOOR ) * intensity ) ) |
There was a problem hiding this comment.
cellHasLightText derives luminance from lightenHexColor(primary, …), which blends toward white, while the actual CSS fill is color-mix(…, transparent) — i.e. primary over whatever background sits behind it. These agree only on a light background; on a dark chart background the light/dark text choice can invert. The package background is light today so this is acceptable, but it's an implicit coupling — worth a one-line comment near CELL_MIX_FLOOR noting the light-background assumption.
| renderChart(); | ||
| const grid = screen.getByRole( 'grid', { name: /heatmap/i } ); | ||
| // Non-compact sets no inline gap/radius — the SCSS falls back to the WPDS tokens. | ||
| expect( grid.style.getPropertyValue( '--heatmap-cell-gap' ) ).toBe( '' ); |
There was a problem hiding this comment.
Vacuous assertion: nothing in the component ever sets --heatmap-cell-radius (the SCSS reads --wpds-border-radius-sm directly), so this can never fail regardless of behavior. Either wire radius through that var or drop the line — the --heatmap-cell-gap assertion above it is the one that carries weight.
kangzj
left a comment
kangzj
left a comment
There was a problem hiding this comment.
Approving — solid, well-tested HTML/CSS implementation with a clean public API matching the SVG branch. The inline notes are non-blocking; happy to see them handled as follow-ups. 🚀
Fixes CHARTS-218
Why
A heatmap is a discrete grid of solid-filled cells — the canonical CSS Grid case. The original implementation (PR #49855, unmerged) renders it as an
<svg>of<rect>s via@visx/heatmap, which means manual coordinate math (xScale/yScale,binWidth/binHeight, gap fudging) and JS-driven color theming. This PR is an alternative, HTML/CSS-first implementation of the same public API, posted as a second option to compare against the SVG branch before either lands.The win is CSS-first theming (the package's preferred direction): per-cell shading, the light→full scale, the empty color, the gap/radius/value-size and the selection ring all move into CSS, driven by WPDS tokens and
color-mix. JS computes only a 0–1 intensity per cell. The geometry math disappears into native CSS Grid +gap, sizing is handled by the layout engine, and text/focus get native rendering.Proposed changes
display: contents).color-mix(in srgb, var(--heatmap-primary) …, transparent); intensity floored at 15% so the lowest value stays visibly tinted. Empty cells default to the WPDS track color and a value overrides it.748.5K,1.2M) so they fit the cell; the tooltip and each cell's accessible label keep full precision.min-heightfloor keeps it usable when the parent has no explicit height, and aspect-ratio mode derives height from width in CSS.primaryColorprop >heatmapChart.primaryColor(theme) > palettecolors[0]viagetElementStyles.heatmapCharttheme section is justprimaryColor+ the compact sizing (compactCellSize,compactCellGap).buildCalendarHeatmapData, a composition color-scale legend, full ARIA grid + 2D keyboard navigation, and tooltips on hover and keyboard focus.The public API (
HeatmapChartProps,<HeatmapChart.Legend />,buildCalendarHeatmapData) stays compatible with the SVG branch, so it's a drop-in alternative.Screenshots
Default matrix
Compact mode
Large values (compact notation)
Calendar / contribution layout
Composition legend
Testing instructions
pnpm installand run Storybook for the charts package.weekStartsOncontrol), Fixed Dimensions, Aspect Ratio, Composition Legend, and Error State.withTooltipsits tooltip follows), and press Escape to clear.pnpm run typecheckandpnpm test— the heatmap suite (36 tests) and the full charts suite (961) pass.Does this pull request change what data or activity we track or use?
No.