Fix handling of Infinity data in all charts

envex commented 1 month ago

What does this implement/fix?

We've discovered that sometimes our charts are being passed Infinity now and this fixes the visual issues. When Infinity is passed to the charts, we instead render the existing error state. We previously fixed this in the vertical bar chart - this PR fixes it across all the other charts.

It looks like a lot of code changes, but mostly it's added Stories to ensure the fixes work. The actual code changes are contained in the first commit.

The code changes:

when possible, take advantage of existing min/max values to perform checks, so we aren't adding performance bottlenecks
the check in useYScale required us to reconsider our use of Infinity is as fallback value when we create scales in some charts, so you'll see that in a few places (including the changes to clamp)
useYScale takes care of several charts but others need the checks internally
some charts were not being passed the skeleton type so those are added

Does this close any currently open issues?

Followup to https://github.com/Shopify/web/issues/144056 Closes https://github.com/Shopify/polaris-viz/issues/1736

What do the changes look like?

Infinity states

Before	After

Data series with empty array of data (providing this because it is related to the changes and is something we should watch carefully)

Before	After

Data prop that is an empty array (providing this because it is related to the changes and is something we should watch carefully, although technically this doesn't satisfy types so maybe it's not something we should be too concerned about)

Before	After

Storybook link

You can check out added stories under the Chromatic section after running yarn storybook:all

Before merging

[x] Check your changes on a variety of browsers and devices.
[x] Update the Changelog's Unreleased section with your changes.
[x] Update relevant documentation, tests, and Storybook.
[ ] Make sure you're exporting any new shared Components, Types and Utilities from the top level index file of the package

github-actions[bot] commented 1 month ago

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
polaris-viz-core-cjs	61.51 KB (+0.16% 🔺)	1.3 s (+0.16% 🔺)	1.3 s (+58.83% 🔺)	2.5 s
polaris-viz-cjs	216.16 KB (+0.14% 🔺)	4.4 s (+0.14% 🔺)	2 s (+2.69% 🔺)	6.4 s
polaris-viz-esm	176.94 KB (+0.15% 🔺)	3.6 s (+0.15% 🔺)	1.3 s (-27.43% 🔽)	4.8 s
polaris-viz-css	4.73 KB (+0.09% 🔺)	95 ms (+0.09% 🔺)	278 ms (-11.77% 🔽)	373 ms
polaris-viz-esnext	182.6 KB (+0.15% 🔺)	3.7 s (+0.15% 🔺)	1.4 s (+4.26% 🔺)	5.1 s

carysmills commented 1 month ago

@susiekims hi! tagging you ont his as @envex and I both worked on the fixes. Let me know if you need any more context :)

Shopify / polaris-viz