CYS - Core: fix font load when user opts out of tracking #45185
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-cys-core-font-preview-and-site-preview-fail-to-load-fonts-when-user-opts-out-of-tracking
gigitux
added
type: bug
|
Hi @albarin, @nefeline, @woocommerce/woo-fse Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
…ad-fonts-when-user-opts-out-of-tracking' of github.com:woocommerce/woocommerce into 45138-cys-core-font-preview-and-site-preview-fail-to-load-fonts-when-user-opts-out-of-tracking
github-actions
bot
added
the
plugin: woocommerce
gigitux
commented
Feb 28, 2024
| @@ -558,14 +523,14 @@ export const FONT_PAIRINGS_WHEN_USER_DID_NOT_ALLOW_TRACKING = [ | |||
| elements: { | |||
| heading: { | |||
| typography: { | |||
| fontFamily: 'var(--wp--preset--font-family--inter)', | |||
| fontFamily: 'var(--wp--preset--font-family--body)', | |||
There was a problem hiding this comment.
We have to use the variable defined by the theme: https://github.com/WordPress/twentytwentyfour/blob/trunk/theme.json#L215
| fontStyle: 'normal', | ||
| fontWeight: '300', | ||
| }, | ||
| }, | ||
| }, | ||
| typography: { | ||
| fontFamily: 'var(--wp--preset--font-family--cardo)', | ||
| fontFamily: 'var(--wp--preset--font-family--heading)', |
There was a problem hiding this comment.
Test Results SummaryCommit SHA: 8b21b9f
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
albarin
approved these changes
Feb 28, 2024
gigitux
deleted the
45138-cys-core-font-preview-and-site-preview-fail-to-load-fonts-when-user-opts-out-of-tracking
branch
github-actions
bot
added
the
needs: analysis
alopezari
added
needs: external testing
Konamiman
pushed a commit
that referenced
this pull request
Mar 13, 2024
* CYS - Core: fix font load when user opts out of tracking * remove comment * Add changefile(s) from automation for the following project(s): woocommerce * fix array to pass to FontFamiliesLoader * fix crash * fix font pair selected after the setup --------- Co-authored-by: github-actions <github-actions@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Submission Review Guidelines:
As a spec, we had to add the "Instrument Sans + Jost" font pair. The issue is that these fonts are loaded in a different variation of the TT4. Currently, we are using the default one. With the default variation, only Inter and Cargo fonts are loaded..
Adding the "Instrument Sans + Jost" font pair could be complex. Should we work on it?
cc @verofasulo @nefeline @albarin
EDIT: @verofasulo confirmed that we can avoid to work to implement the "Instrument Sans + Jost" font pair. p1709138640335519/1709039848.913899-slack-C053716F2H2
Changes proposed in this Pull Request:
Closes #45138.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
customize-store/wp-admin/admin.php?page=wc-settings&tab=advanced§ion=woocommerce_comand ensure that theAllow usage of WooCommerce to be trackedisn't enabled./wp-admin/admin.php?page=wc-admin&path=/customize-store.Choose your Fonts.Cardo/System sans-serifandInter/Cardoare visible./wp-admin/admin.php?page=wc-settings&tab=advanced§ion=woocommerce_comand enable the tracking./wp-admin/admin.php?page=wc-admin&path=/customize-store.Choose your Fonts.Changelog entry
Significance
Type
Message
CYS - Core: fix font load when user opts out of tracking.
Comment