Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Charts - update font-family #11116

Open
mcoker opened this issue Oct 17, 2024 · 4 comments
Open

Charts - update font-family #11116

mcoker opened this issue Oct 17, 2024 · 4 comments
Labels
Milestone

Comments

@mcoker
Copy link
Contributor

mcoker commented Oct 17, 2024

Just noticed that axis labels and tooltips look like they're not using the correct font-family. Specifically it looks like these places:

cc @dlabrecq

@mcoker
Copy link
Contributor Author

mcoker commented Oct 21, 2024

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")

Image

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)

Image

@dlabrecq
Copy link
Member

dlabrecq commented Oct 21, 2024

@mcoker Why do we have both "Red Hat Text" and "RedHatText"?
I ask because we updated Victory to support "RedHatText" via this PR #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 added a commit to dlabrecq/patternfly-react that referenced this issue Oct 21, 2024
@tlabaj tlabaj added the Post v6 label Oct 21, 2024
@tlabaj tlabaj added this to the 2025.Q4 milestone Oct 21, 2024
@mcoker
Copy link
Contributor Author

mcoker commented Oct 21, 2024

@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.

@dlabrecq
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

No branches or pull requests

4 participants