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

Use local fonts for font-variant-alternates #27206

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Jun 7, 2023

Update https://developer.mozilla.org/en-US/docs/web/css/font-variant-alternates to use a local font, so we can make a live sample work.

This should be possible now mdn/yari#7605 has landed.

Updates #21597.

@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Jun 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Preview URLs

(comment last updated: 2023-06-08 15:26:24)

@caugner
Copy link
Contributor

caugner commented Jun 8, 2023

@wbamberg We only support woff2, not woff. Can we use the woff2 version of MonteCarlo?

@wbamberg wbamberg changed the title Try to use local fonts Use local fonts for font-variant-alternates Jun 8, 2023
@wbamberg wbamberg marked this pull request as ready for review June 8, 2023 15:20
@wbamberg wbamberg requested a review from a team as a code owner June 8, 2023 15:20
@wbamberg wbamberg requested review from dipikabh and removed request for a team June 8, 2023 15:20
@wbamberg
Copy link
Collaborator Author

wbamberg commented Jun 8, 2023

It works! Thank you @caugner ! @Elchi3 , we have a live sample for font-variant-alternates now!

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@dipikabh dipikabh merged commit 47807a3 into mdn:main Jun 8, 2023
@drott
Copy link

drott commented Jun 9, 2023

@wbamberg , thanks very much for following up, great to see this working in the preview link! https://pr27206.content.dev.mdn.mozit.cloud/en-US/docs/Web/CSS/font-variant-alternates - Out of curiosity: when does this roll to public MDN?

@caugner
Copy link
Contributor

caugner commented Jun 9, 2023

when does this roll to public MDN?

MDN is deployed every night between 0am and 1am UTC.

@wbamberg
Copy link
Collaborator Author

wbamberg commented Jun 9, 2023

It looks like the page is live, but the font loading is not working:

Screen Shot 2023-06-09 at 8 06 56 AM Screen Shot 2023-06-09 at 8 07 53 AM

@caugner
Copy link
Contributor

caugner commented Jun 12, 2023

@wbamberg I looked into it and the reason it doesn't work is that the filename is not lowercase. The filecheck command catches this, but content's pr-test.yml workflow hasn't been updated yet to account for the newly supported file extensions:

GIT_DIFF_FILES=$(echo "${DIFF_DOCUMENTS}" | egrep -i "^files/.*\.(png|jpeg|jpg|gif|svg|webp)$" | xargs)

Rather than adding extensions, I would suggest to run the filecheck on all non-Markdown files. The command supports this since mdn/yari#7716.

@wbamberg
Copy link
Collaborator Author

Ah, thank you for the investigation, @caugner ! -> #27277

@wbamberg
Copy link
Collaborator Author

wbamberg commented Jun 12, 2023

@drott , am I right in thinking that in the CSS for the example:

.variant {
  font-feature-settings: "swsh" 1;
  font-variant-alternates: swash(fancy);
}

...the first part font-feature-settings: "swsh" 1; is not needed: that in fact the two lines are alternative ways of doing the same thing, and the point of font-variant-alternates is that it offers a less low-level way to do it, and should be preferred (as described in the first paragraph of https://developer.mozilla.org/en-US/docs/Web/CSS/font-feature-settings#syntax, after the example)?

So we should remove the first line from this example?

@wbamberg
Copy link
Collaborator Author

Or even, include both versions, but in different rules:

.variant1 {
  font-feature-settings: "swsh" 1;
}

.variant2 {
  font-variant-alternates: swash(fancy);
}

@drott
Copy link

drott commented Jun 13, 2023

...the first part font-feature-settings: "swsh" 1; is not needed:

Yes, these are two syntaxes for the same thing. font-variant-alternates inherits more in line with CSS (font-feature-settings gets overriden by child rules) and is perhaps more ergonomic to use. But the result is the same.

Showing both, describing font-feature-settings as a lower level alternative would be an option, but that's up to what you as authors feel should be in MDN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants