Add contextual banners for WooCommerce Services (now WooCommerce Tax) #2849
Conversation
Enhance the NUX (New User Experience) system by adding contextual banners that show different content based on: - Store country (US vs non-US) - Whether the WooCommerce Shipping plugin is active This creates a more personalized onboarding flow by: 1. Adding a new option flag for tracking contextual banner state 2. Improving the banner display logic with prioritized checks 3. Creating three different contextual banner types 4. Ensuring proper transition between traditional and contextual banners The change maintains backward compatibility while providing more targeted guidance.
…ismissal control - Extend WC_Connect_Nux to support new service settings and schemas stores. - Add contextual after-connection banner for US users without WooCommerce Shipping plugin. - Enqueue and localize migration modal scripts and styles; render React modal container in admin notices. - Add dismissal logic with a shared option (`wcs_nux_any_banner_shown`) to prevent multiple simultaneous banners. - Enhance feature announcement React component to support forced open state. - Improve migration runner state handling for robustness. - Add safety checks in admin notice dismiss button event listeners. - Make `WC_Connect_Loader::get_wc_connect_base_url` public static for broader usage. - Instantiate `WC_Connect_Nux` with new dependencies in loader. - Add placeholder admin banner and modal manager classes.
Ferdev
force-pushed
the
add/tax-migration-banners
branch
from
ffdecf4 to
8b3ea8d
Compare
Ferdev
requested review from
a team,
allie500 and
Copilot
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR personalizes the NUX flow by introducing contextual banners based on store country and WooCommerce Shipping plugin status, managed via a new option flag.
- Added a
SHOULD_SHOW_CONTEXTUAL_BANNERflag and updated NUX constructor. - Refined
get_banner_type_to_displayto set and prioritize contextual banners. - Created dedicated display methods for three contextual banner types and adjusted tests.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| woocommerce-services.php | Made get_wc_connect_base_url public and updated NUX constructor |
| tests/php/test-class_wc-connect-nux.php | Updated tests to expect new contextual banner output |
| client/wcshipping-migration-admin-notice.js | Wrapped migration notice events in null checks |
| client/components/migration/migration-runner.js | Added validation for previous state key lookup |
| client/components/migration/feature-announcement.jsx | Introduced forceOpen prop for announcement modal |
| classes/class-wc-connect-options.php | Registered new option name for contextual banners |
| classes/class-wc-connect-nux.php | Implemented contextual banner logic, display, and dismissal |
| tests/docker/* | Added Docker setup for integration testing |
There was a problem hiding this comment.
Thanks for working this, Fer! The code looks good. I left a couple of non-blocking comments.
Unfortunately, I have been unable to run through the manual tests outlined in the PR. I think I did something wrong when I connected Jetpack. I clicked the connect button in the banner on the plugin page. That set up all the things as far as I could tell and I could never get the after connection button to show.
I destroyed my wp-env and started it fresh. This time I went to the Jetpack page and connected from there. The banner still was not showing up. I think I must be missing a setup step.
|
hey @allie500, I just went ahead and performed all the requested changes, as they all were very on point 🎯 I've also fixed the issue not allowing you seeing the migration banner. Please, give it another try whenever you can! You're gonna need to either start with a brand new site, or deleting the wp_option |
There was a problem hiding this comment.
Thanks for addressing those requested changes. It took me a little while to get comfortable testing this PR as it is my first in this code base. That said, most everything tested well. The testing for all the tests after the 'after connection banner test', I had issues related to the banners not changing or displaying when conditions were changed. In each of these tests, I also needed to delete the wcs_nux_any_banner_shown option to get the different banner to show. Is this expected behavior?
See below for further details of my testing.
Before Connection Banner:
Here's the before connection banner I saw:
Test After Connection Banner:
- Connect Jetpack for the first time.
- Verify the "after_jetpack_connection" banner appears as expected.
- Dismiss the banner and ensure the flag for showing the contextual banner is set.
This worked as expected. Here's the after banner I saw:
wc_connect_options option array had the flag set: s:36:"should_display_nux_contextual_banner";b:1;
Test Terms of Service (TOS) Banner:
- With Jetpack connected but TOS not yet accepted, verify that the TOS acceptance banner appears.
- Accept the TOS and confirm that the contextual banner flag is set.
For this one, I manually updated the tos_accepted value from 1 to 0 in wc_connect_options. Then I deleted the wcs_nux_any_banner_shown option and refreshed the page. I got the before connection banner again which is, I assume, also the TOS banner.
Test Contextual Banners:
- Set store country to "US" and deactivate the WooCommerce Shipping plugin.
- Verify that the 'after_cxn_us_no_wcs_plugin' contextual banner is shown on the Plugins page.
This worked as expected. I saw this banner:
- Activate the WooCommerce Shipping plugin while still having a US store.
- Confirm that the 'after_cxn_us_with_wcs_plugin' contextual banner appears on the Plugins page.
This worked as expected. I saw this banner:
- Change the store country to a non-US locale.
- Verify that the 'after_cxn_non_us' contextual banner is shown appropriately.
This worked as expected. I saw this banner:
Dismissal Behavior:
- On each banner, click the dismiss action.
- Confirm the corresponding option flags are updated or deleted.
- Ensure the banners no longer appear after dismissal until conditions change.
I checked this for each and this worked as expected. That said, most of this was controlled by the presence of the wcs_nux_any_banner_shown which would prevent any banners showing up, even if conditions changed. I would need to delete that option as well to get a different banner to show for the updated conditions.
Edge Cases:
- Attempt to trigger banners on admin screens other than the Plugins page and confirm they do not display.
- Verify that users who have accepted TOS and completed after-connection flow but without the contextual banner flag see the contextual banner once the flag is set.
I could not trigger banners on other admin screens. That said, "Verify that users who have accepted TOS and completed after-connection flow but without the contextual banner flag see the contextual banner once the flag is set." this doesn't work as expected. I would have to also delete the wcs_nux_any_banner_shown value as well.
General Verification:
- Confirm that styles load correctly for all banners.
- Check that banners update dynamically based on plugin activation and store country changes.
Styles were consistent and appeared to load correctly for me, for all banners. That said, the banners wouldn't update dynamically based on plugin activation or store country. If I made any changes to those, I'd also need to delete the wcs_nux_any_banner_shown option to trigger the banner display behavior.
Yes, that's correct, apologies for not giving a heads up on that, Allie! Yeah, since these banners are only expected to be displayed once, for the test process we need to reset that option and I forgot to mention it. My bad! Thanks for the thorough testing, though! All what you described is the expected behaviour, so I think we can go ahead and approve this. ⚠ WARNING: LET'S NOT MERGE THIS YET.We've changed planes and we're not going to release this banners as they are right now. There will probably be updated with new copy for new communication efforts for the migration project. |
There was a problem hiding this comment.
Thanks for the follow up to confirm that the behavior is what we expect. I'll go ahead and approve this as it is now, even though we won't be merging this as is.
If we get updated banner copy and this PR changes, I'll be happy to re-review the new banner copy.
⚠ WARNING: LET'S NOT MERGE THIS YET.
We've changed planes and we're not going to release this banners as they are right now. There will probably be updated with new copy for new communication efforts for the migration project.
Description
This update enhances the NUX (New User Experience) by introducing contextual banners tailored to store location and plugin status, providing a more personalized onboarding flow. Key improvements include:
Closes WOOSHIP-1360
Testing Instructions
Prepare Environment:
Test After Connection Banner:
Test Terms of Service (TOS) Banner:
Test Contextual Banners:
Dismissal Behavior:
Edge Cases:
General Verification: