-
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
Show button & tooltip to complete GA4 setup in Settings > Connected Services #5621
Comments
@techanvil Shouldn't the URL used be It looks like maybe this URL was used before the issue to consolidate the help links was merged. Otherwise this looks good; if that's the only change needed feel free to update the URL required to just be |
Thanks @tofumatt, good shout. I've updated the AC to mention the selector 👍 |
@nfmohit, we can use
This will be done in #5620, no need to re-post it here. What we need to mention here is that we need to change the module name from
We have changed the the button works in 5620, now it sends users to the setup form instead of opening the settings form. We need to update IB to reflect this change. @techanvil I think we also need to update AC for this ticket, right? If so, can you do it since you wrote it originally? |
Thanks for pointing this out, @eugene-manuilov. I had originally anticipated that we'd still be opening the edit view here but having read through the comments on #5620 I can see that we do need to update this issue too. As such, I have updated the AC, making the changes as follows:
@nfmohit apologies for not making this change earlier, but please could you go ahead and update the IB accordingly? Bear in mind the behaviour for navigating to the Setup page will be implemented in #5620, we just need to change the button text here and show the callout on the Setup page. |
Thank you, @eugene-manuilov and @techanvil! I have updated the IB addressing the suggestions and the updates. Please take a look now. |
@nfmohit the
Perhaps it would be better to use a dedicated property to determine whether we need to render a tooltip or not? We can add something like Also we need to make sure that we display the tooltip only once per page view. In other words, the user should see the tooltip only for the first time when they lend on the setup page and shouldn't see it again when they change the select account. Last but not least, we need to make sure that we display the tooltip only when the Analytics module is already connected and not show it during the standard setup flow. |
I've updated the IB according to the suggestions. Thank you, @eugene-manuilov! |
Thanks, @nfmohit. IB ✔️ |
QA Update: ❌@techanvil @aaemnnosttv afraid I have found two issues while running through this:
ga-b1.mp4
ga-b2.mp4 |
QA Update:
|
Hi @wpdarren, good observations there. I do think we should fix these in the next release as they are fairly benign yet may require a bit of digging to fix, and it doesn't seem worth delaying this release further to fix them. Please can you go ahead and create tickets for them? |
QA Update: ✅Verified: This latest round of testing was completed with the
Note: there are two observations which I highlighted above. These will have new tickets created for them. Screencastsga4-qa2.mp4ga4-qa1.mp4 |
Great, thanks @techanvil and @wpdarren ! |
Feature Description
Following on from #5620, when the Analytics module is connected but GA4 is not setup, the “Analytics is connected” text should be replaced with the CTA button as implemented in #5620, but the button text will be “Connect Google Analytics 4” and clicking on the button will open the settings in edit mode.
Additionally, with the Analytics settings open in edit mode, the GA4 Property dropdown should have a blue callout displayed next to it containing explanatory text and a link to documentation.
Acceptance criteria
{proxy}/support/?doc=ga4
select( CORE_SITE ).getDocumentationLinkURL( 'ga4' )
.Implementation Brief
assets/js/components/settings/SettingsActiveModule/Header.js
:isGA4Connected
which uses theuseSelect
hook to run theisModuleConnected()
selector withanalytics-4
passed as the property from thecore/modules
store (CORE_MODULES
constant).<p>
tag responsible for displaying theConnected
text (being updated in Update module headers in Settings > Connected Services #5620).<p>
tag if:slug
of the module isanalytics
. ANDisGA4Connected
is falsey.<Button />
component with the textComplete setup for { module_name }
(being added in Update module headers in Settings > Connected Services #5620).! connected
) (Already implemented in Update module headers in Settings > Connected Services #5620). ORconnected
). ANDslug
of the module isanalytics
. ANDisGA4Connected
is falsey.analytics
module isconnected
butisGA4Connected
is falsey), update the contents of the button with the stringConnect Google Analytics 4
(translatable).assets/js/modules/analytics-4/components/common/PropertySelect.js
:<PropertySelect />
component to accept a newclassName
prop.select.googlesitekit-analytics__select-property
element to have theclassName
prop as an additional class if available, using theclassnames
library.assets/js/components/JoyrideTooltip.js
:<JoyrideTooltip />
component to accept two new props:className
.cta
with the default asfalse
.onShow
with the default as an empty function.className
andcta
props as properties to the object in thesteps
array.site-kit-wp/assets/js/components/JoyrideTooltip.js
Lines 38 to 47 in 856bf8b
callback
prop of the<Joyride />
component, and call theonShow()
function if thetype
(current state) isEVENTS.TOUR_START
.content
anddismissLabel
as not required so that the tooltip component works without them if necessary.assets/js/components/TourTooltip.js
:div.googlesitekit-tour-tooltip
element to havestep.className
as an additional class if available, using theclassnames
library.div.googlesitekit-tooltip-buttons
:step.cta
.primaryProps.title
is truthy.assets/js/modules/analytics/datastore/constants.js
:HIDE_GA4_PROPERTY_SELECT_TOOLTIP
with the valuehideGA4PropertySelectTooltip
(or something more relevant).assets/js/modules/analytics/components/setup/SetupFormGA4Transitional.js
:useSelect
hook to run thegetDocumentationLinkURL()
selector from thecore/site
store withga4
string passed as the selector parameter, and assign it to a variable nameddocumentationURL
.useCallback
hook namedmarkTooltipAsDisplayed
which uses theuseDispatch
hook to dispatch thesetValue
action with theHIDE_GA4_PROPERTY_SELECT_TOOLTIP
constant andtrue
(boolean) as the properties.useSelect
hook to run thegetValue()
selector from thecore/ui
store with theHIDE_GA4_PROPERTY_SELECT_TOOLTIP
constant passed as the selector parameter, and assign it to a variable namedhideGA4Tooltip
.useSelect
hook to run theisModueConnected()
selector from thecore/modules
store withanalytics
string passed as the selector parameter, and assign it to a variable namedisAnalyticsConnected
.<GA4PropertySelect />
component and pass a unique CSS class so that our callout/tooltip can target them.<GA4PropertySelect />
component, render a<JoyrideTooltip />
component:Learn more
string (translatable) as the content.title
:Set up a new GA4 property from here.
string (translatable).target
: The CSS selector (class) that we passed onto the property selector above.cta
: A<Button />
component with:googlesitekit-tooltip-button
asclassName
prop.documentationURL
ashref
prop._blank
astarget
prop.text
prop.className
: A relevant CSS class. Add styles for this class to style the tooltip according to the Figma designs.onShow
:markTooltipAsDisplayed
function.isAnalyticsConnected
is truthy andhideGA4Tooltip
is falsey.Test Coverage
QA Brief
This change is not affected by the
ga4ActivationBanner
feature flaggooglesitekit_analytics-4_settings
option in WordPress. This can be done in a number of ways, for example:wp option delete googlesitekit_analytics-4_settings
.DELETE FROM wp_options WHERE option_name = 'googlesitekit_analytics-4_settings'
;delete_option( 'googlesitekit_analytics-4_settings' );
in the console.Site Kit -> Settings -> Connected Services
.Connect Google Analytics 4
.Learn More
opens the appropriate documentation page in a new tab.Changelog entry
The text was updated successfully, but these errors were encountered: