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
[CYS on Core] Conditionally change the default fonts available based on user consent #44532
[CYS on Core] Conditionally change the default fonts available based on user consent #44532
Conversation
…the user did not grant us consent.
Test Results SummaryCommit SHA: d14a3fe
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. |
…rade-notice class.
…ing wasn't allowed.
…y is unavailable.
…ever the user opts in.
…default-fonts-available-based-on-user-consent
…Update the link to download the latest version of the core of WordPress.
…default-fonts-available-based-on-user-consent
Hi @albarin, @gigitux, @kmanijak, 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: |
…edirect is not triggered before the option is saved to the database.
…default-fonts-available-based-on-user-consent
…default-fonts-available-based-on-user-consent
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.
Great job!
Good question, @nefeline. Right now, we are using TT4 as the default theme for patter assembler, but we could use Creatio, Assembler 2 or whatever other theme. The logic you proposed should be in place once we implement the pattern assembler experience for all the block themes IMO. Right now, it's detached from a specific theme and its style, so I would default to what we have already implemented. Thanks! |
…on user consent (#44532) * Add upgrade notice for the fonts feature whenever WP is outdateed or the user did not grant us consent. * Add styles for the woocommerce-customize-store_sidebar-typography-upgrade-notice class. * Update the upgrade conditionals. * Introduce the new modal for the user to opt in to usage tracking. * Add the CSS for the woocommerce-customize-store__opt-in-usage-tracking-modal * Implement the sendEvent to OptInDataSharing * Fix typo. * Make sure the opt in button is disabled if the opt in checkbox is unchecked. * Update the styles for the woocommerce-customize-store__opt-in-usage-tracking-modal * Update the styles to allow changes to the link * Set Cardo + System Sans-serif as the default fonts if the usage tracking wasn't allowed. * Set Cardo + System Sans-serif as the default fonts if the Font Library is unavailable. * Add the Jost + Instrument Sans font pairing. * Update copy for the upgrade notice to remove mentions to Gutenberg. * Replace the Link component with the Button component. * Ensure the woocommerce_allow_tracking option is properly updated whenever the user opts in. * Add the new Inter + Cardo font pairing. * Redirect user to the loading screen so all relevant fonts can be installed for them. * Render the FontPairing component only if the isFontLibraryAvailable. Update the link to download the latest version of the core of WordPress. * update the fontPairings constant. * Update styles for buttons and links. * Add changefile(s) from automation for the following project(s): woocommerce * Make sure the dispatch for updating the option is async so the page redirect is not triggered before the option is saved to the database. * Fix lint error --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Alba Rincón <alba.rincon@automattic.com>
Submission Review Guidelines:
Changes proposed in this Pull Request:
Introduce the feature for conditionally changing the default font pairings available based on user consent and the WordPress version.
Closes #44187
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Initial Setup
customize-store
1. The store owner gave consent (
woocommerce_allow_tracking
is true) but has an outdated version of WordPress/Gutenberg or Gutenberg is not installed'woocommerce_allow_tracking'
option is set toyes
:select * from wp_options where option_name = 'woocommerce_allow_tracking';
Screen.Recording.2024-02-23.at.10.50.58.mov
2. User did not give us consent (
woocommerce_allow_tracking
is false) and also has an outdated version of WordPress (earlier than 6.5).'woocommerce_allow_tracking'
option is set tono
:select * from wp_options where option_name = 'woocommerce_allow_tracking';
Copy on the sidebar:
Copy for the newly introduced Opt-in modal:
The checkbox is unchecked, the Opt-in button is disabled:
Screen.Recording.2024-02-23.at.10.57.26.mov
3. User did not give us consent (
woocommerce_allow_tracking
is false) and has the latest version of WP/GB installed.'woocommerce_allow_tracking'
option is set tono
:select * from wp_options where option_name = 'woocommerce_allow_tracking';
Copy on the sidebar and available font pairings:
The two font pairings are being inherited from the TT4 theme:
4. User gave consent and has the latest version of WordPress and/or GB plugin installed
'woocommerce_allow_tracking'
option is set toyes
:select * from wp_options where option_name = 'woocommerce_allow_tracking';
Question to @verofasulo: I believe the 2 font pairings from the TT4 theme should be preserved in the list of options in this case instead of replacing them with our default ones, WDYT?
Changelog entry
Significance
Type
Message
Customize Your Store: Introduce the feature for conditionally changing the default font pairings available based on user consent and the WordPress version.
Comment