CES exit prompt for product editing screens #35728

louwie17 · 2022-11-25T10:17:18Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This add's initial logic to make it easy to track if users leave pages with un-saved changes, for showing a feedback notice.
This would only be enabled if the user has enabled tracking.

Partially addresses #35126 .

Kapture.2022-11-25.at.11.16.31.mp4

How to test the changes in this Pull Request:

Load this branch, build it, and make sure you have the new-product-management-experience feature flag enabled (you can do so using the latest version of the Beta tester within the mono repo).
Make sure you have tracks enabled and also outputted in the console: localStorage.setItem( 'debug', 'wc-admin:*' );
Go to Product > Add New and make some changes
Now go to another page like WooCommerce > Home and select Leave when it warns you about unsaved changes.
A notice should show on the new page that allows you to share feedback, something to the affect of We noticed you started editing a product, then left....
Click the share feedback and submit feedback, notice how the wcadmin_ces_snackbar_view, wcadmin_ces_view, and wcadmin_ces_feedback include the ces_location prop with the outside value.
You may also want to try dismissing the notice and see if the prop has been added to wcadmin_ces_snackbar_dismiss
Repeat steps 4-6 for all these pages: Product edit page, New product MVP page, edit product MVP page.
Now delete the woocommerce_ces_shown_for_actions option and disable tracking under WooCommerce > Settings > Advanced > Woocommerce.com
Make sure the notice doesn't show up for the above anymore and that customer-effort-score-exit-page in local storage does not get updated, but stays empty.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

github-actions · 2022-11-25T10:40:25Z

Test Results Summary

Commit SHA: 104a1e3

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 50s
E2E Tests	187	0	0	6	0	193	13m 18s

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.

louwie17 · 2022-11-25T11:08:36Z

plugins/woocommerce-admin/client/wp-admin-scripts/product-tracking/shared.ts

+	}
+	window.addEventListener( 'beforeunload', function ( event ) {
+		// Check if button disabled or triggered delete to see if user saved or deleted the product instead.
+		if ( checkIfSubmitButtonsDisabled() || triggeredDelete ) {


As the comment implies, it was hard to differentiate if the beforeunload was happening because a user was saving or deleting a product or if they were exiting.
These two checks were the main differentiator I could add to check against this.

octaedro · 2022-12-02T17:51:56Z

plugins/woocommerce-admin/client/customer-effort-score-tracks/constants.ts

+export const SHOWN_FOR_ACTIONS_OPTION_NAME =
+	'woocommerce_ces_shown_for_actions';
+export const ADMIN_INSTALL_TIMESTAMP_OPTION_NAME =
+	'woocommerce_admin_install_timestamp';


I couldn't find the place where these two const are being used.

Aaah yeah, I kinda copy pasted them from another PR, that one is merged now, so all of them should be used in a couple other files now.

octaedro · 2022-12-02T17:54:11Z

...ins/woocommerce-admin/client/customer-effort-score-tracks/customer-effort-score-exit-page.ts

+
+	let items = getExitPageData();
+
+	items = items.filter( ( item ) => item !== pageId );


I'm not clear about this part. Why do we remove the item from the array and add it again?
Maybe we can check if the item is present in the array and add it if not.
It would look like this:

if ( ! items.find( ( pageExitedId ) => pageExitedId === pageId ) ) { items.push( pageId ); }

Simplicity mostly, but I can use your approach as well.

Also to make sure we never have duplicates.

octaedro · 2022-12-02T17:54:26Z

...ins/woocommerce-admin/client/customer-effort-score-tracks/customer-effort-score-exit-page.ts

+
+	let items = getExitPageData();
+
+	items = items.filter( ( item ) => item !== pageId );


What do you think about renaming item to pageExitedId or something less generic?

octaedro · 2022-12-02T17:54:35Z

...ins/woocommerce-admin/client/customer-effort-score-tracks/customer-effort-score-exit-page.ts

+}
+
+/**
+ * checks the exit page list and triggers a CES survey for the first item in the list.


Really nitpick: it should start with a capital letter.

octaedro

Fantastic work @louwie17! The code looks good and it's testing pretty well here. I only left a couple of small comments and I saw something unexpected when I was saving the product.

louwie17 · 2022-12-08T09:51:13Z

Thanks for the review @octaedro, I addressed your PR feedback and fixed the issue you found. This should be ready for re-review.

louwie17 · 2022-12-12T10:39:19Z

plugins/woocommerce-admin/client/customer-effort-score-tracks/customer-effort-score-tracks.js

-			isResolving( 'getOption', [ ALLOW_TRACKING_OPTION_NAME ] );
+			! hasFinishedResolution( 'getOption', [
+				ALLOW_TRACKING_OPTION_NAME,
+			] );


Made use of hasFinishedResolution to fix a bug where if one of the above options was being retrieved again isResolving would be set to true again, which in turn set resolving to true and in turn caused this component to return null.
Once the resolving finished it would render the CustomerEffortScore again and in turn trigger a new notice, something we don't want.

Nice catch!

octaedro

Thank you for adding the changes! LGTM 🚀
Good job @louwie17

* Add exit page tracker logic and implement it for product pages * Add changelog * Fix lint errors and add comments * Add ces_location prop * Add mock to fix broken test * Add CES exit page survey tests * Fix a bug with React pages redirects and update actions * Fix test * Fix lint * Add default inside location prop * Remove exit prefix within action * Address PR feedback and make sure its not triggered on save * Update copy of exit feedback notice * Add changelog * Update name of param * Fix lint error * Use hasFinishedResolution vs isResolved in customerEffortScoreTracks

github-actions bot added focus: react admin plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Nov 25, 2022

louwie17 commented Nov 25, 2022

View reviewed changes

louwie17 force-pushed the add/35126_ces_exit_prompt branch from 4b85bcd to d919d68 Compare November 28, 2022 09:27

louwie17 marked this pull request as ready for review November 28, 2022 09:28

louwie17 self-assigned this Nov 29, 2022

octaedro self-requested a review December 2, 2022 13:02

octaedro reviewed Dec 2, 2022

View reviewed changes

louwie17 force-pushed the add/35126_ces_exit_prompt branch from 049815d to a8ee37c Compare December 8, 2022 09:50

louwie17 requested a review from octaedro December 8, 2022 09:51

github-actions bot added the package: @woocommerce/customer-effort-score issues related to @woocommerce/customer-effort-score label Dec 9, 2022

louwie17 added 13 commits December 12, 2022 10:07

Add exit page tracker logic and implement it for product pages

9a81aa7

Add changelog

20f62b1

Fix lint errors and add comments

0ade393

Add ces_location prop

e4e7d76

Add mock to fix broken test

bcd6ad3

Add CES exit page survey tests

7c0729f

Fix a bug with React pages redirects and update actions

c1b3351

Fix test

efb7f1e

Fix lint

420f3ac

Add default inside location prop

9112a7c

Remove exit prefix within action

d675e5d

Address PR feedback and make sure its not triggered on save

451cc52

Update copy of exit feedback notice

530281f

louwie17 added 4 commits December 12, 2022 10:07

Add changelog

721b47e

Update name of param

74f56a9

Fix lint error

0f0a9d0

Use hasFinishedResolution vs isResolved in customerEffortScoreTracks

104a1e3

louwie17 force-pushed the add/35126_ces_exit_prompt branch from 9ebe306 to 104a1e3 Compare December 12, 2022 10:27

louwie17 commented Dec 12, 2022

View reviewed changes

louwie17 force-pushed the add/35126_ces_exit_prompt branch from 96eb9af to 104a1e3 Compare December 12, 2022 13:16

octaedro approved these changes Dec 12, 2022

View reviewed changes

louwie17 merged commit 613e58c into trunk Dec 12, 2022

louwie17 deleted the add/35126_ces_exit_prompt branch December 12, 2022 13:56

github-actions bot added this to the 7.3.0 milestone Dec 12, 2022

github-actions bot mentioned this pull request Jan 13, 2023

Test WooCommerce 7.3 UVLabs/location-picker-at-checkout-for-woocommerce#59

Closed

louwie17 mentioned this pull request Jan 12, 2024

Show feedback prompt only once #43164

Merged

11 tasks


		let items = getExitPageData();

		items = items.filter( ( item ) => item !== pageId );

CES exit prompt for product editing screens #35728

CES exit prompt for product editing screens #35728

Uh oh!

Conversation

louwie17 commented Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

github-actions bot commented Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octaedro left a comment

Choose a reason for hiding this comment

Uh oh!

louwie17 commented Dec 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octaedro left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

louwie17 commented Nov 25, 2022 •

edited

Loading

github-actions bot commented Nov 25, 2022 •

edited

Loading