-
Notifications
You must be signed in to change notification settings - Fork 53
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
fix(content-gating): account for block visibility in GA4 params #3805
base: release
Are you sure you want to change the base?
Conversation
@@ -334,13 +334,8 @@ private static function get_block_names_recursive( $blocks ) { | |||
* } | |||
*/ | |||
public static function get_gate_metadata() { |
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.
I believe this is also used by back-end events... maybe we need to keep things here and override on the front-end... but for the back-end events we would actually need to see how that conditional blocks are rendered to find out what values to set
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.
Ah, I didn't see that the back-end events were actually using this. Maybe we can avoid recreating the Memberships blocks' logic by checking their render output.
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.
2f117da adds a check in the back-end method, too. We don't get innerBlocks if a member or non-member block won't render any content.
@leogermani this could use another review |
Tomorrow, I promise |
I'm afraid this didn't work for backend events and it might be trickier than we thought: The front-end event sent from a "non members" block that had only a Newsletter Subscription block looked right. It was not considering the "Checkout button" element that was behind the "Members only" block. But the respective back-end events were different:
What it seems to me is that by the time the backend event is fired, the user is already logged in (it logs in as part of the registration process), and then the calculation there takes this into account. This might need a deeper conversation
|
All Submissions:
Changes proposed in this Pull Request:
Takes visibility of important blocks into account when setting relevant GA4 params related to membership content gates. Previously, a content gate was said to "have" Donate, Registration, or Checkout Button blocks if its post content contained those blocks. Now, it will "have" those blocks only if the blocks exist in the DOM inside the content gate container, AND they're visible in the DOM (not hidden by CSS).
Closes
1200550061930446/as/1209574433088705
.How to test the changes in this Pull Request:
collect
XHR request that contains a payload that looks like this:Note the highlighted part of the payload, and confirm that the
gate_has_donation_block
,gate_has_registration_block
, andgate_has_checkout_button
params are accurately set toyes
orno
based on which of those blocks are visible within the content gate.collect
XHR request and confirm that its payload is accurate based on which blocks are visible in the content gate.window.newspack_memberships_gate.metadata
in the JS console. The parameters here are calculated in the back-end, and should match the values of the front-end parameters most of the time.Other information: