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

feat(@ibm/plex): Update plex dependency to use new packages #17025

Merged
merged 9 commits into from
Aug 9, 2024

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Jul 23, 2024

Updates @carbon/styles to use individual @ibm/plex-${font-style} packages rather than importing all from @ibm/plex

Changelog

New

  • 8 new packages for each of the @ibm/plex families
Screenshot 2024-07-25 at 9 17 15 AM
  • font-feature-settings: 'liga' 1; added to _reset.scss to ensure ligatures render even with letter-spacing set
  • $package-name field added to font src resolver

Changed

  • Updated the url field in font src resolver to use new package name format

Removed

Testing / Reviewing

Ensure the fonts are loaded correctly, either using the Akamai CDN or not. Ensure the fi ligature renders properly.

Copy link

netlify bot commented Jul 23, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit be9cdff
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/66b639ff9220c600086d47ac
😎 Deploy Preview https://deploy-preview-17025--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 23, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit be9cdff
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/66b639ffa1dd690008421aee
😎 Deploy Preview https://deploy-preview-17025--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tay1orjones
Copy link
Member

One thing to verify here is the ligature fixes from #11175 IBM/plex#438 are coming through. The storybook doesn't turn on the $use-akamai-cdn flag, so the fi ligature is still broken. For example in the notification component:

This should be fixed by this PR where we consume the new packages that contain these updates to the font files. I know this is still in draft, but right now I don't see the ligatures in the deploy preview.

@tw15egan
Copy link
Member Author

tw15egan commented Jul 24, 2024

@tay1orjones it seems like a component-level issue for some reason. I will take a deeper look. I see the ligatures when just putting text outside of the notification Screenshot 2024-07-24 at 2 38 04 PM

Edit: It seems to be caused due to the letter-spacing property coming from body-compact-01

@tay1orjones
Copy link
Member

tay1orjones commented Jul 24, 2024

@tw15egan wow, yeah it's part of the current css-fonts-3 spec and the working draft css-fonts-4 spec

setting a non-default value for the ‘letter-spacing’ property disables common ligatures.

Can we get around it with font-feature-settings?

@tw15egan tw15egan marked this pull request as ready for review July 25, 2024 13:26
@tw15egan tw15egan requested a review from a team as a code owner July 25, 2024 13:26
Copy link
Member

@alisonjoseph alisonjoseph left a comment

Choose a reason for hiding this comment

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

🔥

@kennylam kennylam enabled auto-merge August 9, 2024 15:47
@kennylam kennylam added this pull request to the merge queue Aug 9, 2024
Merged via the queue into carbon-design-system:main with commit efd08bc Aug 9, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants