patternfly
Charts - update font-family
Just noticed that axis labels and tooltips look like they're not using the correct font-family. Specifically it looks like these places:
- I didn't dig into this one but I assume it's removing spaces from the var value? If so it's worth noting that we went from our font being defined as "RedHatText" in v5 to "Red Hat Text" in v6, so we'll just want to make sure those spaces are retained https://github.com/patternfly/patternfly-react/blob/a3ffb39a0cc2c9130f7db86ad3186787ba12648e/packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts#L129
- This look like an example style, but it should probably be updated to be more accurate? https://github.com/patternfly/patternfly-react/blob/a3ffb39a0cc2c9130f7db86ad3186787ba12648e/packages/react-charts/src/victory/components/ChartBoxPlot/examples/ChartBoxPlot.md?plain=1#L227
- Samesies for this one https://github.com/patternfly/patternfly-react/blob/a3ffb39a0cc2c9130f7db86ad3186787ba12648e/packages/react-charts/src/victory/components/ChartTooltip/examples/ChartTooltip.md?plain=1#L306
cc @dlabrecq
As far as I can tell, all of the axis labels, legends, and tooltips that I see are using the wrong font ("RedHatText" instead of "Red Hat Text")
Here's an example of text that's using the correct font-family. And FWIW wherever the correct family is specified, it looks like the font famnily names are a fallback for the var(--pf-v6-chart-global--FontFamily) var, like:
font-family: var(--pf-v6-chart-global--FontFamily, "Red Hat Text", "RedHatText", "Noto Sans Arabic", "Noto Sans Hebrew", "Noto Sans JP", "Noto Sans KR", "Noto Sans Malayalam", "Noto Sans SC", "Noto Sans TC", "Noto Sans Thai", Helvetica, Arial, sans-serif)
@mcoker Why do we have both "Red Hat Text" and "RedHatText"? I ask because we updated Victory to support "RedHatText" via this PR https://github.com/patternfly/patternfly-react/pull/5301
Note that we don't use a CSS variable for fontFamily because Victory's approximateTextSize function relies on specific character widths and that doesn't work with CSS variables. Victory uses "RedHatText"'s character spacing to calculate padding, etc. -- it doesn't know about "Red Hat Text"
@dlabrecq gotcha! @lboehling do you remember anything around why we're using "Red Hat Text" vs "RedHatText" as the font family name on the page? Curious if we're aligning with any other groups at RH on that, and if we should consider declaring the font-family in PF as both "Red Hat Text" and "RedHatText" for anyone that may not be using our font-family token and might have specified "RedHatText" as their font family somewhere.
Turns out that Victory implemented a reliable way to calculate character widths dynamically. The existing character map is now a backup (e.g., when there is no window object) -- likely for use with React native (i.e., iOS, Android, etc.).
The "Red Hat Text" (avg: 0.5413177633285524) character width is comparable to the previous font. Considering "RedHatText" (avg: 0.5341940789473685) is already supported, we don't need to update Victory upstream.
