Shopify / polaris-viz

A collection of React and React native components that compose Shopify's data visualization system
https://polaris-viz.shopify.dev
Other
335 stars 25 forks source link

Change font size for touch devices #1749

Closed envex closed 2 weeks ago

envex commented 3 weeks ago

What does this implement/fix?

Resolves https://github.com/Shopify/core-issues/issues/80051

Change all the font-sizes to 14px when on a mobile/touch device. This applies to legends, labels and values for all charts.

What do the changes look like?

Desktop

image

Mobile/Touch

image

Storybook link

https://6062ad4a2d14cd0021539c1b-xwoqqwlwwf.chromatic.com/

Before merging

github-actions[bot] commented 3 weeks ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
polaris-viz-core-cjs 61.63 KB (+0.04% 🔺) 1.3 s (+0.04% 🔺) 766 ms (-21.97% 🔽) 2 s
polaris-viz-cjs 222.3 KB (+0.13% 🔺) 4.5 s (+0.13% 🔺) 1.8 s (-3.42% 🔽) 6.3 s
polaris-viz-esm 180.33 KB (+0.11% 🔺) 3.7 s (+0.11% 🔺) 1.8 s (+30.79% 🔺) 5.4 s
polaris-viz-css 5.47 KB (+0.04% 🔺) 110 ms (+0.04% 🔺) 255 ms (+31.06% 🔺) 364 ms
polaris-viz-esnext 186.99 KB (+0.1% 🔺) 3.8 s (+0.1% 🔺) 1.4 s (-12.19% 🔽) 5.1 s
envex commented 2 weeks ago

@carysmills Removed the annotation font-size and fixed the + 3 more font size on legends.

envex commented 2 weeks ago

@carysmills I'll dig in, but it's likely the new sizes are being too aggressive on the "Drop labels that touch" logic.

I vaguely remember fixing an issue here as part of this PR as well, so maybe we just need to be less aggressive .

carysmills commented 2 weeks ago

@envex sounds good. I am seeing this on desktop though, when the font sizes shouldn't have changes, so perhaps it's to do with how some of the math has changed with the character widths etc