Add the Settings Section setup CTA variant including happy path setup behavior
Feature Description
Add the Settings Section setup CTA variant including happy path setup behavior.
This includes the setup logic (which may be extracted for reuse from the Setup CTA Banner if not already reusable) including OAuth redirection, the in progress state and the success state.
Note that the success state can reuse the component introduced in https://github.com/google/site-kit-wp/issues/8238.
See Settings section > setup CTA in the design doc.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- If the Audience Segmentation feature has not been set up (i.e. the Setup CTA Banner has not been actioned), the Settings section should display a setup CTA instead of the switch to toggle the audiences widget area.
- The setup CTA should follow the Figma design.
- It should have a description with the following copy: To set up new visitor groups for your site, Site Kit needs to update your Google Analytics property.
- It should have an Enable groups link button.
- Upon clicking Enable groups, the feature will be set up following the setup logic described for the Setup CTA Banner (including the OAuth flow, if the edit scope is not granted. See #8131 and #8132), while showing a progress bar for the in progress state (see Figma).
- Upon success, a dismissible notice will be displayed alongside the switch for toggling the audiences widget area (see Figma).
- It should have the following copy: We've added the audiences section to your dashboard!
- It should have Show me and Got it primary and secondary CTAs.
- Clicking Show me should navigate to the dashboard and scroll down to the newly added widget area.
- Clicking Got it should dismiss the notice.
- Note that all copy should be verified with Figma as the source of truth.
Implementation Brief
This should be implemented once #8132 is implemented and merged.
-
[ ] Create a hook
useAudienceSegmentationSetupinsideassets/js/modules/analytics-4/hooks/.- [ ]
useAudienceSegmentationSetupshould return an object consisting of the following properties.- [ ]
isSaving- This is a boolean flag indicating whether the setup operation is in progress. This can be constructed using theuseStatehook and will be set insideonEnableGroupscallback as it is currently inAudienceSegmentationSetupCTAWidgetcomponent. - [ ]
onEnableGroups- Callback forEnable groupsCTA. This should be taken as is fromAudienceSegmentationSetupCTAWidgetcomponent from here with all the dependecies for theuseCallback. - [ ]
canRender- Returnnullfrom the component if this isfalse, else render the component. Refer this condition under which this can be evaluated.- [ ] Hook should also call this useEffect as this will be applicable to both the components and any other similar component.
- [ ]
- [ ]
-
[ ] Create a new component
SettingsVisitorGroupsCTAinsideassets/js/modules/analytics-4/components/audience-segmentation/settings/.- [ ] We can take a reference of
SettingsCardVisitorGroupscomponent to create the UI forSettingsVisitorGroupsCTAcomponent, its similar. - [ ] Component should use
useAudienceSegmentationSetuphoook which will return the object as mentioned above.- [ ] If
isSavingisfalserender theEnable groupsCTA, else render theProgressBarcomponent. We may need to add a fix height when displaying theProgressBarto avoid the layout shift (~142px). - [ ] onClick callback of CTA should be
onEnableGroupscallback returned by the hook.
- [ ] If
- [ ] We can take a reference of
-
[ ] Create a new component
AudienceSetupSuccessSettingsNoticeinside/assets/js/modules/analytics-4/components/audience-segmentation/settings/which should be responsible for displaying the success notice withinSettingsCardVisitorGroupscomponent.- [ ] Item key for component to display the notice can be
audience_segmentation_setup_settings_notification.
- [ ] Item key for component to display the notice can be
-
[ ] Update
SettingsCardVisitorGroupscomponent to display the success message as per following.- [ ] Check if the success banner has already been dismissed. Refer
AudienceSegmentationSetupSuccessSubtleNotificationcomponent for the item key and how it handles the banner here. Note that the item key needs to be different as mentioned above so that relevant notice is being displayed. - [ ] Use dismiss item infra from
core/userdata store to check if the item is dismissed and to also dismiss the item. - [ ] Success banner should be displayed in
SettingsCardVisitorGroupscomponent if it has not been dismissed earlier. - [ ] Clicking the
Got itshould dismiss the notice. - [ ] Clicking show me should do the following.
- [ ] Dismiss the notice.
- [ ] Use
navigateTofunction to navigate to the audience widget area.
- [ ] Check if the success banner has already been dismissed. Refer
-
[ ] Update the
AudienceSegmentationSetupCTAWidgetto useuseAudienceSegmentationSetuphook and remove the code which has been ported to hook.
Test coverage
- Add the stories for
SettingsVisitorGroupsCTAcomponent with all the states. - Add the test for the
SettingsVisitorGroupsCTAcomponent with different states and props. - Fix the story for
AudienceSegmentationSetupCTAWidgetcomponent to accommodate changes. - Fix any failing tests and VRTs.
- Add tests for the
useAudienceSegmentationSetuphook. 6 . Update theSettingsCardVisitorGroupscomponent tests to add tests for the success notice behaviour. - Add tests for
AudienceSetupSuccessSettingsNoticecomponent.
QA Brief
Changelog entry
Thanks @ankitrox for drafting the IB. I've given an initial pass over the IB and the approach looks solid. A few questions and suggestions:
- The
useAudienceSegmentationSetupshould probably reside inassets/js/modules/analytics-4/hooksas the feature is analytics module specific and it will be used here. - The hook also should have basic test coverage if possible.
- It would be good to detail a little bit more about the hook and what it should do, or how the suggested values should be obtained before being returned.
Let me know what you think, cheers!
Thank you so much @kuasha420 for reviewing the IB.
I have made some amendments in IB as per your suggestions. Assigning this to you for re-review.
Thanks again!
Thanks @ankitrox, Looking good now. However, I have missed a point in my previous review. The second to last point of the AC states "Upon success, a dismissible notice will be displayed alongside the switch for toggling the audiences widget area", this is not covered in current IB. Can you add that,
Besides that, it would be nice to add a pointer/note about trying to avoid layout shift while the progressbar is showing by using a appropriate height for the Visitor Group area and other relevant component. What do you think?
Cheers!
@kuasha420 Thanks for the spotting the missing point about the success notice.
I have addressed the feedback. assigning this to you for re-review.
Thanks
Thanks @ankitrox for the additions to the IB.
It is worth pointing out that the success notice for settings should use a different dismiss item key, Otherwise, if someone activates the Audience Group in Dashboard, and visits settings without dismissing the success notification, they'll see it above the toggle here as well which may not be desirable behavior.
Also, this should be a new component that renders inside the SettingsCardVisitorGroups, ie. AudienceSetupSuccessSettingsNotice which is similar in looks to the AudienceSegmentationSetupSuccessSubtleNotification.
Cheers.
Thank you @kuasha420 .
Updated the IB to include new component AudienceSetupSuccessSettingsNotice to display success notice and also included mention about the different item key.
Thanks @ankitrox. LGTM. I've bumped up the estimate one level, considering this kind of issues are generally more involved that initially feels, and the extensive test coverage suggestion.
IB :white_check_mark:
Hi @techanvil . The ACs for this issue include a requirement to scroll down to the Audience Tiles widget area. However, this functionality is planned to be built as part of #8874. Should the AC for #8874 be updated to include updating the functionality for the settings setup success notification as well? Thanks!
Hi @nfmohit, personally I would prefer to keep the AC as they stand, as the IB for #8874 has already been approved and it would need a rewrite; the common code between this and the behaviour in #8874 is just a call to getContextScrollTop() and scrollTo(), the triggers will be completely different.
It will also be natural to test the scrolling as part of this issue, as it's part and parcel of the Settings flow that is being implemented. The main reason #8874 was created separately is that the widget area wasn't available to scroll to when the setup happy path was first implemented, which isn't the case here. If this issue was larger then it might be worth splitting it out, but as it's currently scoped and estimated I think it's fine to keep it here. Of course, if the issue was underestimated and looks like it's going to go much over the estimate it could be worth reconsidering.
@techanvil Co-assigned you on this as we request your kind feedback here, thank you!
QA Update ⚠️
I only 2 feedback:
ITEM 1: First item is regarding the fonts size and weight used. While it's reflecting as per figma, it's not consistent with other text on the page.
- The 'Enable groups' is at font weight 500 but other similar CTA on the page are at 400.
- The text 'To set up new visitor groups for your site, Site Kit needs to update your Google Analytics property.' is currently at 12px, similar to Figma but other similar text on the page are at 14px.
Refer to the video for more details.
ITEM 2: I can still see the tiles on the SK dashboard even if I turn them off in settings. Is that going to be fixed in a different ticket?
https://github.com/user-attachments/assets/9ed52786-3fc0-4714-baa1-e7ba1ad400e3
Other than that, the following were tested good.
- The setup CTA follows the Figma design. ✅
- The setup CTA has a description with the following copy: To set up new visitor groups for your site, Site Kit needs to update your Google Analytics property. It has an Enable groups link button.✅
- It should have an
- Clicking on the "Enable groups" CTA link, the "in progress state" appears according to the ACs and Figma designs. We are then taken to the OAuth flow to grant necessary scopes. ✅
- Once OAuth granted, the "in progress state" persists until Audience Segmentation is set up, after which, the success notification appears with a switch. Copy and figma are good. ✅
- Upon success, a dismissible notice is displayed alongside the switch for toggling the audiences widget area.
- It has the following copy: We've added the audiences section to your dashboard!
- It has Show me and Got it primary and secondary CTAs.
- Clicking Show me navigates to the dashboard and scroll sdown to the newly added widget area.
- Clicking Got it dismisses the notice.
- Verified on mobile and tablet breakpoints and while there are no figma for those, there is no broken experience.
https://github.com/user-attachments/assets/6a6fc1d7-2f87-4076-a5e6-19d7c7c408db
@kelvinballoo, regarding your observations:
ITEM 1: First item is regarding the fonts size and weight used. While it's reflecting as per figma, it's not consistent with other text on the page.
We should keep the CTA and text consistent with other admin settings. Please send it back to execution and assign it to @benbowler, as he agreed to work on it during the AS sync.
ITEM 2: I can still see the tiles on the SK dashboard even if I turn them off in settings. Is that going to be fixed in a different ticket?
That has not been implemented yet. @techanvil, could you create a new issue for that?
Thanks @hussain-t! As mentioned on Slack, I've created #9065.
Back to you for another pass, @kelvinballoo.
QA Update ✅
Tested ITEM 1 and it's looking much better.
- The text 'To set up new visitor groups for your site, Site Kit needs to update your Google Analytics property.' is now at 14px, similar to other text on the page.
- The 'Enable groups' is now at font weight 400
ITEM 2 will be tackled under https://github.com/google/site-kit-wp/issues/9065
Other than that, the following were tested good.
- The setup CTA follows the Figma design. ✅
- The setup CTA has a description with the following copy: To set up new visitor groups for your site, Site Kit needs to update your Google Analytics property. It has an Enable groups link button.✅
- It should have an
- Clicking on the "Enable groups" CTA link, the "in progress state" appears according to the ACs and Figma designs. We are then taken to the OAuth flow to grant necessary scopes. ✅
- Once OAuth granted, the "in progress state" persists until Audience Segmentation is set up, after which, the success notification appears with a switch. Copy and figma are good. ✅
- Upon success, a dismissible notice is displayed alongside the switch for toggling the audiences widget area. ✅
- It has the following copy: We've added the audiences section to your dashboard!
- It has Show me and Got it primary and secondary CTAs.
- Clicking Show me navigates to the dashboard and scroll sdown to the newly added widget area.
- Clicking Got it dismisses the notice.
- Verified on mobile and tablet breakpoints and while there are no figma for those, there is no broken experience.
- Did a smoke test on the main dashboard for enabling groups and it is functioning as expected. Also tested the unhappy paths introduced in #8134. ✅
https://github.com/user-attachments/assets/eb5a1b09-3fa3-430a-847f-5e4f2d2320b9
https://github.com/user-attachments/assets/6a6fc1d7-2f87-4076-a5e6-19d7c7c408db
Moving ticket to Approval.