CYS - Core: install font when user clicks opt-in #45580

gigitux · 2024-03-14T10:33:25Z

Warning

This PR is blocked by #45586

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Closes #45476 .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Make sure you have the WooCommerce Beta Tester plugin installed and activated
Under Tools > WCA Test Helper > Features, make sure customize-store is enabled
Under Tools > WCA Test Helper > Tools, click on Reset Customize Your Store-
Visit wp-admin/admin.php?page=wc-settings&tab=advanced&section=woocommerce_com
Ensure that the Allow usage of WooCommerce to be tracked is disabled.
Visit wp-admin/admin.php?page=wc-admin&path=%2Fcustomize-store.
Click on start Start designing.
On the sidebar, click on Change homepage.
Select a new homepage.
Save the change.
On the sidebar, click on Choose Fonts.
Check Opt in to usage tracking to get access to more fonts.
Click on usage tracking.
Click on Opt-in.
Ensure that the pre-load screen appears
Ensure that the Assembler is loaded
Click on the "Choose Fonts"
Ensure that multiple fonts are available.
Ensure that the homepage hasn't been restored.
Install https://wordpress.org/plugins/wp-data-access/
Visit wp-admin
On the sidebar WP Data Access > Query Builder
Copy paste:

SELECT * FROM `wp_posts` WHERE `post_type` LIKE 'wp_font_family' ```

25. Click Execute
26. Ensure that a table with some results is visible
27. Copy paste:

SELECT * FROM wp_posts WHERE post_type LIKE 'wp_font_face'

28. Ensure that a table with some results is visible
29. Remove them via:

DELETE FROM wp_posts WHERE post_type = 'wp_font_face';
DELETE FROM wp_posts WHERE post_type = 'wp_font_family';

30. Ensure that the `woocommerce_allow_tracking` option (`wp_option` table) is set to `yes`.
31. Update the option to `no`.
32. Refresh the assembler page.
33. Repeat 9-20.


<!-- End testing instructions -->

### Changelog entry

<!-- You can optionally choose to enter a changelog entry by checking the box and supplying data. -->

-   [x] Automatically create a changelog entry from the details below.

<details>

#### Significance

<!-- Choose only one -->

-   [ ] Patch
-   [x] Minor
-   [ ] Major

#### Type

<!-- Choose only one -->

-   [x] Fix - Fixes an existing bug
-   [ ] Add - Adds functionality
-   [ ] Update - Update existing functionality
-   [ ] Dev - Development related task
-   [ ] Tweak - A minor adjustment to the codebase
-   [ ] Performance - Address performance issues
-   [ ] Enhancement - Improvement to existing functionality

#### Message <!-- Add a changelog message here -->
CYS - Core: install font when user clicks opt-in

#### Comment <!-- If the changes in this pull request don't warrant a changelog entry, you can alternatively supply a comment here. Note that comments are only accepted with a significance of "Patch" -->


</details>

github-actions · 2024-03-14T10:54:13Z

Test Results Summary

Commit SHA: cdb7799

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 38s
E2E Tests	335	0	0	18	0	353	7m 57s

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.

…er-clicking-start-designing-1' of github.com:woocommerce/woocommerce into 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

…mmerce

github-actions · 2024-03-14T14:02:42Z

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

…mmerce

…er-clicking-start-designing-1' of github.com:woocommerce/woocommerce into 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

…-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

gigitux · 2024-03-15T16:40:24Z

@albarin, @nefeline and @verofasulo. This PR introduces this UX:
https://github.com/woocommerce/woocommerce/assets/4463174/21d85b46-8d54-4e04-8106-e6d08c54fb89

Please note that after installation, clicking on "Choose Fonts" will not select any font pair. This is because the theme continues to use the previously chosen font by default:

Should we consider updating the theme to utilize the newly installed font (Inter -Inter) during the installation process?

danielwrobert · 2024-03-15T20:48:45Z

...-admin/client/customize-store/assembler-hub/sidebar/sidebar-navigation-screen-typography.tsx

@@ -201,18 +202,16 @@ export const SidebarNavigationScreenTypography = () => {
 										</Button>
 										<Button
 											onClick={ async () => {


Is the async still needed here or can this be a regular arrow fn now that we removed the await portion below?

Nice catch! Fixed!

danielwrobert · 2024-03-15T21:14:32Z

Nit: I'm not sure if it's appropriate to address in this PR but it seems like the Opt in to portion of the Opt in to usage tracking to get access to more fonts. should also be included in the link so it's a more clear CTA.

Having usage tracking be the linked text alone makes me think I will be directed to information about the usage tracking.

danielwrobert

Ensure that the pre-load screen appears

I've tried to run the testing steps 4 times and each time it freezes at the Turning on the lights step.

I can move forward in the process by refreshing the page but if I don't refresh, it will just hang.

CleanShot.2024-03-15.at.21.59.45.mp4

nefeline · 2024-03-16T07:38:09Z

Nit: I'm not sure if it's appropriate to address in this PR but it seems like the Opt in to portion of the Opt in to usage tracking to get access to more fonts. should also be included in the link so it's a more clear CTA. Having usage tracking be the linked text alone makes me think I will be directed to information about the usage tracking.

@danielwrobert could you please direct any questions related to design and/or copy to @verofasulo and @lauroraa ? Thanks!

nefeline

Thanks for your work here, @gigitux! I've requested a change in the code: let's keep both async and sync fonts installation in place in case one of them happen to fail 👍

Regarding:

and each time it freezes at the Turning on the lights step.

I can reproduce this as well: unfortunately, the console has no particular errors that could help with debugging.

...-admin/client/customize-store/assembler-hub/sidebar/sidebar-navigation-screen-typography.tsx

albarin · 2024-03-16T09:19:23Z

I've tried to run the testing steps 4 times and each time it freezes at the Turning on the lights step.

I can reproduce this as well.

gigitux · 2024-03-18T10:28:01Z

Sorry for the confusion, I introduced the issue with 00eb4a3.

I fixed it! Also, I noticed that the font installation wasn't triggered when the assembler wasn't loaded in the iframe. I fixed that issue as well! 🤞

albarin

Great effort 🙇
Tests as expected now!

nefeline

Thanks for the amazing work! Confirmed things are working as expected now 🚀

gigitux linked an issue Mar 14, 2024 that may be closed by this pull request

[CYS on Core] Make sure the pre-loader is triggered right after the user clicks on the opt in modal. #45476

Closed

CYS - Core: install font when user clicks opt-in

4439716

gigitux force-pushed the 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal branch from 71d8240 to 4439716 Compare March 14, 2024 11:10

gigitux and others added 5 commits March 14, 2024 12:12

remove debugger

33e7819

CYS - Core: fix wp-admin page visible when click on start designing

0aae2b5

Merge branch '45542-cys-core-wp-admin-screen-visible-for-a-moment-aft…

0d4aded

…er-clicking-start-designing-1' of github.com:woocommerce/woocommerce into 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

Add changefile(s) from automation for the following project(s): wooco…

cb2ff93

…mmerce

use sendEvent instead of redirect

94b6082

gigitux marked this pull request as ready for review March 14, 2024 13:58

gigitux requested review from nefeline and albarin March 14, 2024 13:59

woocommercebot requested a review from a team March 14, 2024 14:02

github-actions and others added 5 commits March 14, 2024 14:04

Add changefile(s) from automation for the following project(s): wooco…

9884342

…mmerce

not show wp-admin after the click on the dialog button

6a547e2

Merge branch '45542-cys-core-wp-admin-screen-visible-for-a-moment-aft…

9799f9c

…er-clicking-start-designing-1' of github.com:woocommerce/woocommerce into 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

Merge branch 'trunk' of github.com:woocommerce/woocommerce into 45476…

d656983

…-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

use sendEventToIntroMachine

69f63e5

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 15, 2024

gigitux added 3 commits March 15, 2024 11:03

Merge branch 'trunk' of github.com:woocommerce/woocommerce into 45476…

a228c1e

…-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

Merge branch 'trunk' of github.com:woocommerce/woocommerce into 45476…

7271ec3

…-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal

fix font installation

06c19fa

gigitux force-pushed the 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal branch from e7cdfba to 06c19fa Compare March 15, 2024 16:14

gigitux self-assigned this Mar 15, 2024

gigitux added focus: customize-your-store Issues related to the Customize Your Store onboarding flow. type: bug The issue is a confirmed bug. labels Mar 15, 2024

fix eslint error

00eb4a3

danielwrobert reviewed Mar 15, 2024

View reviewed changes

danielwrobert reviewed Mar 16, 2024

View reviewed changes

nefeline requested changes Mar 16, 2024

View reviewed changes

...-admin/client/customize-store/assembler-hub/sidebar/sidebar-navigation-screen-typography.tsx Show resolved Hide resolved

gigitux added 2 commits March 18, 2024 11:15

fix install font phase when the iframe is not loaded

9b2505e

remove not necessary async

cdb7799

albarin approved these changes Mar 18, 2024

View reviewed changes

nefeline approved these changes Mar 18, 2024

View reviewed changes

gigitux merged commit f9c0d40 into trunk Mar 18, 2024
31 checks passed

gigitux deleted the 45476-cys-on-core-make-sure-the-pre-loader-is-triggered-right-after-the-user-clicks-on-the-opt-in-modal branch March 18, 2024 15:49

github-actions bot added this to the 8.8.0 milestone Mar 18, 2024

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CYS - Core: install font when user clicks opt-in #45580

CYS - Core: install font when user clicks opt-in #45580

gigitux commented Mar 14, 2024 •

edited

github-actions bot commented Mar 14, 2024 •

edited

github-actions bot commented Mar 14, 2024 •

edited

gigitux commented Mar 15, 2024

danielwrobert Mar 15, 2024 •

edited

gigitux Mar 18, 2024

danielwrobert commented Mar 15, 2024

danielwrobert left a comment

nefeline commented Mar 16, 2024

nefeline left a comment •

edited

albarin commented Mar 16, 2024

gigitux commented Mar 18, 2024

albarin left a comment

nefeline left a comment

CYS - Core: install font when user clicks opt-in #45580

CYS - Core: install font when user clicks opt-in #45580

Conversation

gigitux commented Mar 14, 2024 • edited

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

github-actions bot commented Mar 14, 2024 • edited

Test Results Summary

github-actions bot commented Mar 14, 2024 • edited

gigitux commented Mar 15, 2024

danielwrobert Mar 15, 2024 • edited

Choose a reason for hiding this comment

gigitux Mar 18, 2024

Choose a reason for hiding this comment

danielwrobert commented Mar 15, 2024

danielwrobert left a comment

Choose a reason for hiding this comment

nefeline commented Mar 16, 2024

nefeline left a comment • edited

Choose a reason for hiding this comment

albarin commented Mar 16, 2024

gigitux commented Mar 18, 2024

albarin left a comment

Choose a reason for hiding this comment

nefeline left a comment

Choose a reason for hiding this comment

gigitux commented Mar 14, 2024 •

edited

github-actions bot commented Mar 14, 2024 •

edited

github-actions bot commented Mar 14, 2024 •

edited

danielwrobert Mar 15, 2024 •

edited

nefeline left a comment •

edited