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

_ignoreDefaultFontLinks boolean private config option #1595

Closed
wants to merge 1 commit into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Aug 21, 2024

This seems more reasonable than introducing a new extraHead option; it doesn't change the default font stack that loads Source Serif (née Source Serif Pro, now Source Serif 4) from Google Fonts.

supersedes #1589
supersedes #1592

@Fil Fil requested a review from mbostock August 21, 2024 12:15
@Fil Fil changed the base branch from main to fil/googlefonts August 21, 2024 12:16
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn’t actually feel simpler to me; it feels like the same complexity but less powerful. (And I also quibble with the name ignoreDefaultFontLinks which I would probably invert to includeDefaultFonts, but that’s easily fixed.) I’m going to make some small tweaks to the extraHead branch which I think is still the way to go.

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After more thought, I prefer #1597. 🙏

@Fil Fil closed this Aug 22, 2024
@Fil Fil deleted the fil/font-headers branch August 22, 2024 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants