Settings: Add new dropdown for choosing analytics behavior #13567
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
…e tracking handler
… selection This dropdown will only be visible when SiteKit is installed & active.
AnuragVasanwala
added
Type: Enhancement
swissspidy
reviewed
Jan 17, 2024
packages/wp-dashboard/src/components/editorSettings/googleAnalytics/index.js
Outdated
Show resolved
Hide resolved
swissspidy
reviewed
Jan 17, 2024
packages/wp-dashboard/src/components/editorSettings/googleAnalytics/index.js
Outdated
Show resolved
Hide resolved
…rameter to the constructor
AnuragVasanwala
commented
Jan 31, 2024
….com/GoogleForCreators/web-stories-wp into feat/13506-flexible-tracking-support
1. Check if dropdown has rendered 2. Check if SiteKit specific message is displayed 3. Check if `handleUpdateGoogleAnalyticsHandler` is called with the appropriate values
swissspidy
reviewed
Feb 12, 2024
Comment on lines
337
to
343
| {analyticsHandler === GOOGLE_ANALYTICS_HANDLER_TYPE.SITE_KIT && ( | ||
| <TextInputHelperText size={TextSize.Small}> | ||
| <TranslateWithMarkup> | ||
| {TEXT.SITE_KIT_IN_USE} | ||
| </TranslateWithMarkup> | ||
| </TextInputHelperText> | ||
| )} |
There was a problem hiding this comment.
In our last meeting we discussed always displaying this text.
However, after looking at it again, I think we can simply remove it. It doesn't add any value in this new layout.
There was a problem hiding this comment.
I think current behaviour seems more clear from user's point of view. Because, note is necessary to convey to the user that Site Kit has enabled Google Analytics and will be used to track analytics.
WDYT 🤔
| Option | Note |
|---|---|
| Use Site Kit for analytics (default) | Note: Site Kit by Google has already enabled Google Analytics for your Web Stories, all changes to your analytics tracking should occur there. |
| Use only Web Stories for analytics | - |
| Use both | - |
There was a problem hiding this comment.
I think current behaviour seems more clear from user's point of view. Because, note is necessary to convey to the user that
Site Kithas enabled Google Analytics and will be used to track analytics.
The problem is that this note does not convey this at all. The option in the dropdown does.
That's why the text needs updating and why I don't think it makes sense to show this Note only when the first option is chosen. Either always show it (if Site Kit is active), or never show it.
If we want to rephrase it still, it would make more sense to use something like:
If Site Kit is active, it will be used to set up Google Analytics by default. However, you can customize the behavior in case you need more flexibility.
There was a problem hiding this comment.
If Site Kit is active, it will be used to set up Google Analytics by default. However, you can customize the behavior in case you need more flexibility.
Seems to make more sense 👍🏻
There was a problem hiding this comment.
@swissspidy I have updated Site Kit note to be displayed regardless of option chosen in the analytics dropdown.
Commit Ref: b86ae3c
Please let me know if anything else needs to be taken care of. Otherwise, the PR is ready for final review.
swissspidy
changed the title
…ve and `analyticsHandler` is `site-kit`
|
Marking as ready for review so that I can test this on the QA environment |
|
Plugin builds for 8b792a2 are ready 🛎️!
|
|
Size Change: +385 B (0%) Total Size: 2.77 MB
ℹ️ View Unchanged
|
…d on changing the dropdown option
|
Minor request, otherwise this looks good: Right now the measurement ID field is disabled when the "Use Site Kit" option is chosen. I don't think we need this any longer. Let's remove that restriction. 
|
swissspidy
reviewed
Feb 15, 2024
packages/wp-dashboard/src/components/editorSettings/googleAnalytics/index.js
Outdated
Show resolved
Hide resolved
packages/wp-dashboard/src/components/editorSettings/googleAnalytics/index.js
Outdated
Show resolved
Hide resolved
….com/GoogleForCreators/web-stories-wp into feat/13506-flexible-tracking-support
swissspidy
approved these changes
Feb 16, 2024
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.
Summary
This PR add support for flexible Tracking Handler. When SiteKit is installed and active, in WebStories Settings - it will provide a dropdown with options to select:
User-facing changes
A dropdown has been added to select the analytics tracking handler. The dropdown is only shown if Site Kit analytics module is active.
Dropdown options:
Testing Instructions
Scenario 1: Selecting "Use Site Kit for analytics (default)":
web_stories_ga_tracking_handleroption changes tosite-kitupon selecting this option.Scenario 2: Selecting "Use only Web Stories for analytics":
web_stories_ga_tracking_handleroption changes toweb-stories.Scenario 3: Selecting "Use both":
web_stories_ga_tracking_handleroption changes toboth.Reviews
Does this PR have a security-related impact?
No.
Does this PR change what data or activity we track or use?
🤔
Does this PR have a legal-related impact?
No.
Checklist
Type: XYZlabel to the PRFixes #13506