Charts: Tick a small whole-number value axis only on whole numbers - #52588
adamwoodnz merged 18 commits into
Conversation
|
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Social plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Videopress plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Premium Analytics plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
f2013ce to
1377104
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
This comment was marked as resolved.
This comment was marked as resolved.
Code Coverage SummaryCoverage changed in 3 files.
1 file is newly checked for coverage.
|
6cb9473 to
d1018a8
Compare
32479e1 to
8a6381d
Compare
8a6381d to
ef4bcca
Compare
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for fixing this! @adamwoodnz I ran the three stories, and the small ranges read 0 and 1 now. 👍
I left a few inline comments, mainly about a percentage axis the whole-number check also catches.
| * @param series - The series visx renders. | ||
| * @return True when at least one reading exists and none has a fraction. | ||
| */ | ||
| export const hasOnlyWholeNumbers = ( series: SeriesData[] ): boolean => { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Good catch, thanks. Fixed in f684ea9: the filter now stands aside when the caller pins the value domain (yScale.domain, or xScale.domain on a horizontal bar chart). A pinned domain means the caller chose the range the ticks divide, and a percent formatter labels 20% steps without repeating. The chart's own fallback domains, such as the [ 0, 1 ] a flat series of ones gets, are not the caller's, so they still get whole-number ticks. No new prop. Tests on line, area and both bar orientations reproduce your case (all zeros, [ 0, 1 ], percent format) and expect the six 20% labels. They fail without the fix.
There was a problem hiding this comment.
Could you verify this is fixed for you now please @chihsuan?
ef4bcca to
45ea992
Compare
d3 steps a linear scale's ticks by 1, 2 or 5 times a power of ten, so a [0, 1] domain ticks at 0, 0.2, 0.4, 0.6, 0.8, 1. The chart's compact number formatter rounds those to one decimal place, so several ticks land on the same label. These helpers let later tasks drop the fractional ticks and keep only the whole ones when every reading in the series is a whole number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
A range that only ever hits whole numbers (say 0 and 1) still asked d3 for 4 ticks, so the axis printed 0, 0.25, 0.5, 0.75, 1 even though no reading was ever a quarter. Route the y Grid and Axis through WholeNumberTicks so they drop the fractional ticks when every visible series is whole, keep them when any reading has a fraction, and defer to a caller's own tickValues. Also passes the caller's y numTicks to Grid, which was hard-coded to 4 regardless of the axis config. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
Same fix as the line chart's y axis, applied to the bar chart's value axis (y when vertical, x when horizontal). A whole-number-only range like 0 and 1 still asked d3 for 4 ticks and printed 0.25, 0.5 and 0.75 alongside them. Route the value axis's Grid and Axis through WholeNumberTicks, which drops the fractional ticks once every rendered series is whole and defers to a caller's own tickValues. The date/band axis on the other side is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
Same fractional-tick problem as the line and bar charts, applied here only when the plotted values are the readings themselves. A normalized stack (expand/wiggle/silhouette) reshapes whole-number data into fractions, so the whole-number filter is gated on `! stacked || stackOffset === 'none'` and left untouched otherwise. Also passes the caller's y numTicks to Grid, which was hard-coded to 4 regardless of the axis config. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
Line and bar chart consumers need a way to see and reproduce the new whole-number tick behavior in Storybook, and to know how to opt out with options.axis.y.tickValues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The whole-number-ticks sentence named axis.y unconditionally, but a horizontal bar chart puts values on axis.x, so the doc pointed callers of that orientation at the wrong prop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
A pinned domain like [0, 0.8], or a hidden fractional series with
rescaleYOnVisibilityChange={ false }, leaves exactly one whole tick.
Filtering to that single tick showed only 0 on the axis, so fall back
to visx's own ticks unless at least two whole ones survive.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The same note on Visx's tickValues gap was copied into the line and area charts as well as the bar chart. Keep it only where bar-chart.tsx already had it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The whole-number check already filters to visible series, but nothing covered it. Seed a fractional series hidden via defaultHiddenSeries alongside a whole one and assert the axis reads 0 and 1 once each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The tickValues fix landed without its own changelog entry alongside the value-axis whole-number one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The docs already say axis.y.tickValues moves to axis.x on a horizontal bar chart; the SmallWholeNumberRange story description did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The package changelog only reaches the plugins listed in its own CHANGELOG. Premium Analytics, Jetpack, wpcomsh, Social and VideoPress each surface the fix to their users and need their own entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The bar chart's category gridlines now follow the caller's x numTicks instead of a fixed 4. The Podcast daily downloads chart sets up to 15 x ticks with gridVisibility "y", so its visible gridlines change. It runs on WordPress.com and self-hosted Jetpack, so it needs its own package entry and a Jetpack plugin entry, and the charts entry now names the bar chart too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The grids chose between the whole-number ticks and the caller's with `( horizontal && valueTicks ) || ...`, which leans on `false` falling through. A ternary with `??` says which axis gets the whole-number ticks, and reads like the Axis block below it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
Nothing tested that the category grid now follows the x axis numTicks, the change the Podcast changelog entries describe: visx filters a band domain by numTicks, so 15 draws more column lines than 4. Also cover a caller's value tickValues winning on the bar chart, where the orientation branch lives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
45ea992 to
809d2d3
Compare
readings.ts already owns what counts as a reading. Checking it again here would drift the moment that rule changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
visx's Axis and Grid fall back to the scale's own default when numTicks is undefined. Defaulting to 4 here meant a caller passing numTicks: undefined got a filtered tick set built from a different count than the axis drew. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
A percentage axis pinned to [ 0, 1 ] with all-zero data (a store with no orders) is whole numbers, so the filter cut its 20% steps down to 0% and 100%. A caller who pins the domain has chosen the range the ticks divide, and a custom formatter labels those steps without repeating, so the filter now stands aside. The chart's own fallback domains, like the [ 0, 1 ] a flat series of ones gets, are not the caller's and still get whole-number ticks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs
The fix itself landed in #52588 while this PR was open. What is left is the coverage that PR did not add — grid positions asserted equal to the axis tick positions, on both axes and both orientations — and the API doc sentence, which still named only tickValues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vAVZ6qLAJ7AR2UujogThA


Fixes CHARTS-283
Why
A site with one subscriber saw its chart's value axis read
1, 1, 1, 0, 0, 0, so no one could tell which gridline was which. Now, when every value on a chart is a whole number, the axis and its gridlines sit only on whole numbers, and the same chart reads0and1. This applies to every small count: subscribers, posts, views, visits.Builds on CHARTS-281 (#52522) and CHARTS-284 (#52580), both merged. CHARTS-281 anchors a flat series of 1s at
[ 0, 1 ], which is exactly the range that showed this bug. #52523 (WOOA7S-2181) is stacked on this PR.Proposed changes
whole-number-ticks.tsxreads the value scale visx built (throughDataContext), asks it for the usual ticks, and drops the fractional ones. d3 steps by 1, 2 or 5 times a power of ten, so every whole number in the domain is already one of its ticks. Nothing changes when the ticks are already whole, when any visible value has a fraction, or when fewer than two whole ticks would remain.Axisand to the matchingGrid, so the gridlines move with the labels. The area chart applies it only when unstacked or withstackOffset: 'none'; the other offsets plot transformed values.tickValuesor pinned value domain always wins. A pinned domain (yScale.domain, orxScale.domainon a horizontal bar chart) keeps every tick, so a percentage axis on[ 0, 1 ]still steps by 20% when its data is all zeros. The chart's own fallback domains are not the caller's and still get whole-number ticks. A caller'snumTicksis still the count asked of the scale, and the whole-number filter applies to it too.numTicksandtickValues. They were hard-coded to 4, so a caller's settings moved the labels but not the gridlines. This includes the bar chart's category gridlines.SmallWholeNumberRangestories and docs for the line and bar charts, and changelog entries.Screenshots
The
SmallWholeNumberRangestory, values 0 and 1 only.Line chart: the y axis reads 0 and 1 once each
Bar chart: the y axis reads 0 and 1 once each
Horizontal bar chart: the value axis is x and reads 0 and 1
Plugin changelog entries
Chosen by tracing every consumer of
LineChart,BarChartandAreaChart:premium-analytics,jetpack,wpcomsh): subscriber and other count widgets show this.social,jetpack): the Overview traffic chart passes no tick options.videopress): the views trends chart passes no tick options for views.jetpack): the My Jetpack stats chart already hides non-integer labels with its owntickFormat, so only its extra gridlines go away. That workaround does nothing after this change and can be removed in a follow-up.packages/podcast,jetpack): its value axis pins its owntickValues, but its daily downloads chart draws category gridlines, which now follow its xnumTicks(up to 15) instead of a fixed 4.tickValues. Jetpack's At a Glance backup chart and the Boost score graph do not use@automattic/charts.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
In Storybook, open JS Packages / Charts Library / Charts / Line Chart / Small Whole Number Range.
0and1, once each, with no gridlines between them.orientationtohorizontal: the x axis reads0and1.Verified:
pnpm run testinprojects/js-packages/charts: 104 suites, 1722 tests passing.pnpm run typecheck: clean.pnpm run lint-changed: clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01T6rWwVGBVEcNmuR8kkLphs