Ensure ECE button load events are triggered for multiple buttons on the same page #9845
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
rafaelzaleski
changed the title
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +55 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
rafaelzaleski
requested review from
a team and
asumaran
and removed request for
a team
asumaran
approved these changes
Nov 29, 2024
There was a problem hiding this comment.
Code looks great! I just left a non-blocking comment for you, @rafaelzaleski.
I tested that the tracking events/requests are correctly triggered on the following pages:
- Product Page.
- Cart Page (Block, Shortcode)
- Checkout Page (Block, Shortcode)
- WooPay enabled/disabled.
However, I noticed an unintended behavior with WooPay: the tracking event is triggered multiple times (at least twice) on the block-based Cart page. We should probably apply the same fix here to ensure the event is called at least once every second or only once. While this isn’t related to this PR, I wanted to bring up the issue.
| @@ -17,20 +17,29 @@ export const trackExpressCheckoutButtonClick = ( paymentMethod, source ) => { | |||
| recordUserEvent( event, { source } ); | |||
| }; | |||
|
|
|||
| const debouncedTrackApplePayButtonLoad = debounce( ( { source } ) => { | |||
There was a problem hiding this comment.
@rafaelzaleski Is there a scenario where debounce is truly necessary? It seems we only want to register the tracking event once per page load, regardless of how many times the Express Checkout buttons are rendered. Wouldn’t it be simpler to use lodash’s once function (didn't test), or even avoid lodash entirely by using an internal flag to prevent multiple calls?
In my tests, I didn’t observe the tracking event being triggered multiple times, but I also didn’t render the Express Checkout buttons multiple times. I still think it’s important to ensure the tracking event runs only once per page load, so I’m unsure if allowing it to be called multiple times after 1 second is the desired behavior.
jaclync
added a commit
that referenced
this pull request
Dec 2, 2024
* develop: Ensure ECE button load events are triggered for multiple buttons on the same page (#9845) Payouts: Add payout bank reference key to payout reports and CSV (#9832) Fix WooPay component spacing (#9748) Use paragraph selector instead of label for pmme appearance (#9840) Allow redirect to the settings page from WCPay connect (#9827) Update references to woocommerce_payments_server (#9824) Fix return types in DocBlocks (#9815) Fix WooPay trial subscriptions purchases (#9778)
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.
Fixes #9795
Changes proposed in this Pull Request
This PR resolves an issue where Apple Pay and Google Pay button load events were not triggered when both buttons were present on the same page. The root cause was the debouncing of trackExpressCheckoutButtonLoad, which suppressed subsequent calls with different arguments. The implementation now debounces load events independently for each payment method, ensuring all events are correctly tracked.
Testing instructions
Prerequisites:
Test the fix:
gpay_button_loadandapplepay_button_loadevents are sent when the page loads. (search foradmin-ajaxcalls)class-woopay-tracker.php.$event_objvariable.gpay_button_loadand one forapplepay_button_load.Expected Outcome:
gpay_button_loadandapplepay_button_loadevents should be triggered when the respective buttons load.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge