-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: use plex as per family #17683
fix: use plex as per family #17683
Conversation
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17683 +/- ##
==========================================
+ Coverage 81.81% 82.19% +0.37%
==========================================
Files 404 404
Lines 14067 14151 +84
Branches 4413 4418 +5
==========================================
+ Hits 11509 11631 +122
+ Misses 2395 2359 -36
+ Partials 163 161 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good!🥳
It would be nice if the existing documentation could be updated to reflect this change and surface the new config option. I'm specifically thinking about the .md we have in the repo that covers how to self-host plex and write your own font resolver, etc.
Probably need some new docs as well covering stuff like:
- What happens if both
$use-akamai-cdn
and$use-per-family-plex
are set to true? - How would someone migrate from
$use-akamai-cdn
? - How would someone migrate from basic/existing
$font-path
usage? - What will be the default in the next major
If you'd like to put the documentation work in a different issue/PR, I'd be fine approving and merging this 👍 |
Co-authored-by: Taylor Jones <[email protected]>
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple things I saw - I'll work on a small refactor and push the changes to this PR
Co-authored-by: Taylor Jones <[email protected]>
Co-authored-by: Taylor Jones <[email protected]>
Co-authored-by: Taylor Jones <[email protected]>
I pushed an update here, let me know what you think! |
Oh I see what you have done here !! These details will be helpful. |
bda9d1e
Closes #11661
Updates
@carbon/styles
to have flexibility of using both individual@ibm/plex-${font-style}
packages and@ibm/plex
by using a variableuse-per-family-plex
which is false by default.Changelog
New
@ibm/plex
familiesfont-feature-settings: 'liga' 1;
added to_reset.scss
to ensure ligatures render even withletter-spacing
set$package-name
field added to font src resolverChanged
url
field in font src resolver to use new package name format whenuse-per-family-plex
is trueRemoved
.woff
, so they have been removed from the individual font formatsTesting / Reviewing
Ensure the fonts are loaded correctly, either using the Akamai CDN or not. Ensure the
fi
ligature renders properly.