-
Notifications
You must be signed in to change notification settings - Fork 295
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
Add First-Party Mode site health and value helpers. #9794
Conversation
Build files for f82d2af have been deleted. |
5cbafdc
to
fab5ce0
Compare
|
||
return array( | ||
'first_party_mode_is_enabled' => array( | ||
'label' => __( 'First Party Mode: Enabled', 'google-site-kit' ), |
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.
@techanvil, should it be First-Party Mode: Enabled
with a hyphen?
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.
Thanks for spotting that @hussain-t, you are quite right, "First-Party" should be hyphenated.
I've amended the AC/IB for this issue, and for #9668 accordingly.
Cc @benbowler
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.
Thanks @techanvil. Since I'm reviewing the PR I can amend the changes.
cc: @benbowler
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.
Sounds good, thanks Hussain!
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.
Nice work, @benbowler! LGTM 👍
Note: I've verified that the CI build failures are unrelated to this PR.
Summary
Addresses issue:
Relevant technical choices
During execution I realised that because First-Party Mode is not a module it's debug fields won't show up. First I updated DebugData before settling on adding the fields in the Analytics and Ads module. The code is in both so that the debug field appears if either module is connected. As they have the same key, only one field shows even if both modules are connected.
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist