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

docs: Possibly fix table display issue #18187

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rikhuijzer
Copy link

I wasn't able to reproduce #11106, but I'm quite sure this PR will fix the issue.

From the screenshots that people posted in the issue, it looks like the table is displayed in a variable width (non-monospace) font. When setting the font to a non-monospace, namely serif, in the Firefox developer tools, then the output looks indeed wrong (see "serif" in the bottom right):

image

And if I directly set

code {
  font-family: "Source Code Pro";
}

Then Firefox (and Chrome; tested also) cannot find that font and falls back to a non-monospace font:

image

But this doesn't explain the issue, because normally devices should fallback to a monospace font due to

/* From extra.css */
:root {
  --md-code-font: "Source Code Pro" !important;
}

/* From some mkdocs CSS file */
code, kdb, pre {
  font-family: var(--md-code-font-family) !important;
}

body {
  --md-code-font-family: var(--md-code-font), SFMono-Regular, Consolas, Menlo, monospace;
}

So essentially when substituting the variables, this should mean that

code, kdb, pre {
  font-family: "Source Code Pro", SFMono-Regular, Consolas, Menlo, monospace !important;
}

which should work fine everywhere.

What could be the root-cause though is the !important part. Maybe certain browsers substitute that as:

code, kdb, pre {
  font-family: "Source Code Pro" !important, SFMono-Regular, Consolas, Menlo, monospace !important;
}

which could then make the line invalid and then the browser falls back to a non-monspace font.

A solution to this could be to remove the !important.

This PR suggests an even more robust solution, namely to override the font-family and thus sidestep the var:

code, kdb, pre {
  font-family: monospace !important;
}

All devices should have a monospace font available, so although this might be slightly less pretty, it should work everywhere. Also, since this is less dependent on which fonts are available system-wide, issues should occur for everyone instead of in specific situations only.

@github-actions github-actions bot added documentation Improvements or additions to documentation python Related to Python Polars rust Related to Rust Polars labels Aug 14, 2024
@c-peters
Copy link
Collaborator

Would this change the font for all code elements from "Source Code Pro" to monospace?

@rikhuijzer
Copy link
Author

rikhuijzer commented Aug 15, 2024

Would this change the font for all code elements from "Source Code Pro" to monospace?

Yes. It would do that for everyone who doesn't by default have "Source Code Pro" available (the website doesn't distribute the font to clients as far as I can see).

The alternative could be to drop the !important as a first attempt and then verify the fix with someone who has a device that can reproduce the bug.

@hadley
Copy link

hadley commented Aug 21, 2024

Before merging this, I think you need to be really sure that this isn't a firefox bug.

When I look at this in my firefox, I see the same problem but devtools reports that a using font-face: Source Code Pro, so I don't think this is a font-fallback problem.

Also pkgdown should be including the font file, and definitely does on https://duckplyr.tidyverse.org/#usage-for-individual-data-frames, so it's not even using the users font.

...

Maybe it's a glyph substitution problem? When I look my locally installed Source Code Pro, it does contain the box drawing characters, but when I use the type tester (https://fonts.google.com/specimen/Source+Code+Pro?query=source+code+pro) it shows the missing glyph symbol.

....

https://fonts.googleapis.com/css2?family=Source+Code+Pro:ital,wght@0,200..900;1,200..900&display=swap lists a bunch of unicode ranges. I wonder if there's some tool to help debug which one is used?

....

Related issue: google/fonts#2465

So maybe the root cause is that firefox picks a much worse substitute than safari/chrome.

...

This is the fix that @t-kalinowski used https://github.com/rstudio/keras3/blob/6ff11d22946960439fee830338ffd29422a5b294/pkgdown/_pkgdown.yml#L26-L32

...

This appears to be a long-standing issue with google fonts: google/fonts#4235

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants