site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Implement the Audience Creation Notice happy path

Open techanvil opened this issue 1 year ago • 8 comments

Feature Description

Implement the happy path for creating an audience via the Audience Creation Notice. This includes adding the notice to the Selection Panel, creating the audiences and showing the success state. It does not include redirecting to OAuth if necessary, this will be implemented separately via https://github.com/google/site-kit-wp/issues/8165.

See audience creation in the design doc.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Main notice

  • If Site Kit hasn’t created the “new visitors” and “returning visitors” audiences, the Selection Panel should show a dismissible Audience Creation Notice with CTAs for creating them.
    • Specifically, the notice should only be shown if neither of the audiences exist. If one of the audiences exists the implication is that they were both created and the user has archived one of them, in which case we shouldn't show the notice.
    • It's OK to show the notice in what should be an unlikely edge case of the audiences having been created and then the user archiving both of them. In this scenario the user can simply dismiss the notice (or of course may choose to recreate the audiences).
  • The notice should follow the Figma design.
  • It should have a title with the following copy: Create groups suggested by Site Kit
  • It should have an "X" icon in the top right corner. Clicking it should permanently dismiss the notice for the current user (in fact, the dismissal state will be clearable by a via a change of the connected property, but this will be handled separately via issue #8180).
  • It should initially show a row for each of the "new visitors" and "returning visitors" audiences, displaying their names and descriptions as follows (in line with the values specified in audience configuration in the design doc):
    • Name: New visitors
      • Description: People who visited the site for the first time
    • Name: Returning visitors
      • Description: People who have visited your site at least once before
  • Each audience should have a Create CTA button.
  • Clicking Create should create the audience using the appropriate audience configuration.
  • While the creation is in progress, the Create button should show a spinner to the left of the button label.
  • Upon successful creation of an audience, the audience row should be removed from the Audience Creation Notice, and the audience should appear in the Selection Panel.
  • When both audiences have been created the notice should be dismissed.

Success notice

  • Upon successful creation of an audience, a success notice will appear above the footer of the Selection Panel, following the Figma design.
  • This notice should have the following copy: Visitor group created successfully!
  • It should have a Got it button which will dismiss the notice.
  • The notice should reappear in response to a subsequent audience creation.

  • Note that all copy should be verified with Figma as the source of truth.

Implementation Brief

In assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/constants.js:

  • Add a new constant AUDIENCE_CREATION_NOTICE_SLUG with the value audience-segmentation-creation-notice (or an appropriate key value).
  • Add a new constant AUDIENCE_CREATION_SUCCESS_NOTICE_SLUG with the value audience-segmentation-creation-success-notice (or an appropriate key value).

AudienceCreationNotice component

In assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel directory, create a new file AudienceCreationNotice.js:

  • Create a new functional component, AudienceCreationNotice, that will be used to render the Audience Creation Notice.
  • Retrieve the notice dismissal state from the isItemDismissed selector by passing the AUDIENCE_CREATION_NOTICE_SLUG constant.
  • If the notice is dismissed, return null.
  • Retrieve the Site Kit created audiences from the getConfigurableAudiences selector. For example, see how this is being filtered AudienceItems component: https://github.com/google/site-kit-wp/blob/8240ccf423e84ab53dc7fcea36833feae1736235/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/AudienceItems.js#L60-L70
  • If the Site Kit created audiences are available, return null.
  • Descriptions for the audiences will be available in the retrieved configurable audiences.
  • Create a handleCreateAudience function with the following:
    • Accepts the audience slug as an argument.
    • Sets the isCreatingAudience state to true.
    • Dispatches the createAudience action with the audience slug.
    • Syncs the available audiences with the syncAvailableAudiences action.
    • Sets the isCreatingAudience state to false.
    • If no error occurs, dispatch the setValue action of the CORE_UI store with the AUDIENCE_CREATION_SUCCESS_NOTICE_SLUG constant and true as the value.
  • Check if one of the Site Kit created audiences just created by the user by retrieving the success notice state from the getValue selector by passing the AUDIENCE_CREATION_SUCCESS_NOTICE_SLUG constant.
  • Create a variable to store whether the success notice flag is true and exactly one Site Kit-created audience exists. This checks if the user has just created one of the audiences.
  • Define the condition under which the Audience Creation Notice should be displayed. The notice should be shown if it has not been dismissed and either:
    • The Site Kit created audiences are not available, or
    • One audience has been created, allowing the user to create the second audience.
   const hasOneAudienceBeenCreated = hasSuccessNotice && siteKitCreatedAudiences.length === 1;
   const shouldShowNotice = ! isItemDismissed( AUDIENCE_CREATION_NOTICE_SLUG ) && ( ! siteKitCreatedAudiences || hasOneAudienceBeenCreated );
  • If the condition to show the notice is met, render the notice with the following structure:
    • Reuse the container styles from the AddGroupNotice component.
    • Reuse the styles to align the audience name and description from the SelectionPanel component.
    • Render the close button with the [X](https://github.com/google/site-kit-wp/blob/001d6128da62b59ad599b834a440ebc3b9f5d205/assets/svg/icons/close.svg) icon. See how it's being implemented in the SelectionPanelHeader component: https://github.com/google/site-kit-wp/blob/7ea3bb35dd70112f679f6336b6d0f5d0b327aa3e/assets/js/components/SelectionPanel/SelectionPanelHeader.js#L39-L45
    • Pass a callback function that dispatches the dismissItem action with the AUDIENCE_CREATION_NOTICE_SLUG constant to the onClick event of the close button.
    • Render the SpinnerButton component with the following props:
      • children: Create
      • onClick: Pass the handleCreateAudience function with the audience slug as an argument.
      • isSaving: Create a new component state isCreatingAudience and set it to true when the createAudience action is dispatched.
      • spinnerPosition: SPINNER_POSITION.BEFORE (before).
  • The notice and the CTA buttons should be styled according to the Figma design.
  • Add the custom styles in assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-selection-panel.scss.

AudienceCreationSuccessNotice component

  • It should be similar to the ErrorNotice component.
  • Retrieve the notice global state from the getValue selector by passing the AUDIENCE_CREATION_SUCCESS_NOTICE_SLUG constant.
  • If the notice is not available, return null.
  • If the notice is available, render the notice with the following structure:
    • Render the CheckFill icon on the left side of the notice.
    • The notice should have the following copy: Visitor group created successfully!.
    • The background color should be #B8E5CA.
    • Render the Got it button on the right with the following props:
    • onClick: Dispatch the setValue action of the CORE_UI store with the AUDIENCE_CREATION_SUCCESS_NOTICE_SLUG constant and false as the value.
    • children: Got it.
    • tertiary: true.
    • The color of the button should be #265C3B.
    • The notice should be styled according to the Figma design.

Rendering the Notice components

In assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/index.js:

  • Render the AudienceCreationNotice component right after the AddGroupNotice component.
  • Render the AudienceCreationSuccessNotice component right after the ErrorNotice component.
  • In the closePanel function, dispatch the setValue action of the CORE_UI store with the AUDIENCE_CREATION_NOTICE_SLUG constant and false as the value.

Test Coverage

  • Add unit test coverage for the new components in assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel directory.
  • Add test coverage for the new components in assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/index.test.js.
  • Add storybook stories in assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/index.stories.js.

QA Brief

Changelog entry

techanvil avatar Jan 25 '24 11:01 techanvil

I've moved this back to Backlog as the final in-progress changes to the design doc, relating to audience caching, will probably affect the AC for this one

techanvil avatar Mar 28 '24 14:03 techanvil

The audience caching aspect of the design doc has been sufficiently finalised, and I've moved this back to AC.

techanvil avatar Apr 05 '24 14:04 techanvil

Hi @techanvil, If a user creates one of the Site Kit audiences (either "new visitors" or "returning visitors") from the notice, and the available audiences are synced after the creation, should we continue to show the notice for the other audience that hasn't been created yet? Or should the notice not be displayed when one of the audiences is created? I want to ensure we handle this scenario correctly according to the AC.

Thanks for your guidance!

hussain-t avatar Jul 01 '24 09:07 hussain-t

Hey @hussain-t, if the available audiences are synced after the user creates one of the audiences, the notice for the other should still remain while the panel is still open so they have the opportunity to create the second audience.

Sending it back to you in IB in case this needs an update, please let me know if anything needs further clarification!

techanvil avatar Jul 01 '24 11:07 techanvil

Thanks for the clarification, @techanvil. I've updated the IB.

hussain-t avatar Jul 01 '24 14:07 hussain-t

Thanks @hussain-t. I'm unassigning this for now as I hadn't actually picked this up for review, although I'm happy to do so if nobody else gets it first!

techanvil avatar Jul 01 '24 14:07 techanvil

Sure, thanks @techanvil 👍

hussain-t avatar Jul 01 '24 14:07 hussain-t

Thanks @hussain-t. I have ended up picking this one up for IBR :)

Before I get into the detail, a high level consideration I have when reviewing this is that, with this being another 30 point issue, I wonder if we should break this down into smaller issues. This should help us improve the estimation and reduce the risk of another issue running over. I'm thinking we could create the two components presentationally in Storybook with an issue for each of them, with a third issue to integrate them with the defined logic. What do you think?


This aside, I have a few relatively minor points to consider:

  • If the notice is dismissed, return null.
    ...
  • If the Site Kit created audiences are available, return null.
    ...
  • Define the condition under which the Audience Creation Notice should be displayed. The notice should be shown if it has not been dismissed and either:
    • The Site Kit created audiences are not available, or
    • One audience has been created, allowing the user to create the second audience.

Can the first two points be removed? They seem a bit confusing in isolation (particularly the second point), when the condition for showing the notice should include both of these sub-conditions in combination as per the third point.

Also, maybe we should be explicit and say "both of the Site Kit created audiences are not available".

  • Descriptions for the audiences will be available in the retrieved configurable audiences.

This seems incorrect, because the audiences won't exist in the list of configurable audiences yet as we haven't created them. Presumably we should be retrieving the values from SITE_KIT_AUDIENCE_DEFINITIONS.

Lastly, a small formatting consideration - the IB is missing the checkboxed points that we have defined as the format to use for them.

techanvil avatar Jul 05 '24 10:07 techanvil

Thanks for the thoughtful review, @techanvil. Breaking this into smaller, more manageable issues makes a lot of sense. However, given that we’ve already come this far and the IB has been defined, I’m flexible and open to either approach.

If you think proceeding with the current plan would be more efficient, we can do that. Otherwise, as you mentioned above, we can break it into multiple issues. Let me know how you’d like to proceed!

Apart from that, I have updated the IB.

hussain-t avatar Jul 08 '24 09:07 hussain-t

Thanks @hussain-t. I think splitting the issue is the way to go here.

I realise the IB is already defined - but, it's worth remembering that the guidance is for us to prefer to split an issue if it's estimated at a 30, which can obviously only happen once the initial IB is drafted anyway. So, reorganising into smaller issues at this stage is the expected thing to do with our process anyway. And of course the main thing is it should allow us to be a bit more precise and accurate with the estimates - if when we evaluate these individually we find they add to more than 30 points then we might have saved ourselves from going over the estimate on this as a single issue.

So, I've gone ahead and split this into three issues. As you've already drafted the IB for this one, I've assigned the other two issues to you in IB, I hope that's OK. These issues being https://github.com/google/site-kit-wp/issues/8986 and https://github.com/google/site-kit-wp/issues/8987.

techanvil avatar Jul 08 '24 15:07 techanvil

Thanks, @techanvil. That makes sense. I appreciate you splitting the issue and assigning the new ones. I'll take care of the IB for the other two issues.

hussain-t avatar Jul 08 '24 16:07 hussain-t

Thanks @hussain-t! This, and the IBs for the other issues are LGTM.

IB :white_check_mark:

techanvil avatar Jul 09 '24 08:07 techanvil

Noting that the AC has been tweaked to remove the requirement for neither of the SK audiences to exist in order to show the notice.

As discussed with @hussain-t during execution, this would be problematic in the case where the user created one audience but ran into some technical issue (e.g. browser crashing) before they created the second. If we were to no longer show the notice because one audience exists, the user would be blocked from setting up the second audience.

We will instead show the notice if either of the audiences is still available to create, and the notice hasn't been dismissed.

techanvil avatar Jul 29 '24 10:07 techanvil

QA Update: ⚠️

I have one observation and it might not be directly under the scope of this ticket. Let me know if this needs fixing and thus, a new ticket:

  • If I dismiss the audience creation notice, the 'All Visitors' audience is there and we will have the text 'Additional groups' below it but it's essentially blank. Should we be hiding that text?
    Screenshot 2024-08-01 at 15 20 36

These were tested successfully ✅

  • After archiving the Site Kit created audiences and opening Audience Selection panel, the Audience Creation Notice was present.
  • The Audience Creation Notice is as per figma
  • Clicking Create for an audience functions well. While the creation is in progress, the Create button shows a spinner to the left of the button label.
  • Upon successful creation of an audience, the audience row is removed from the Audience Creation Notice, and the audience appears in the Selection Panel. The success notice is dismissed when clicked on 'Got it'
  • Meanwhile, the audiences re-sync and Audience Creation Success Banner appears at the bottom of the panel. Also, the Audience Creation Notice updates so that only the uncreated audience is there.
  • When both audiences have been created the Audience Creation Notice is dismissed.
  • We can also dismiss the notice without creating the audiences.

https://github.com/user-attachments/assets/74ec8982-fd89-403d-87a6-9bbae430f57c

kelvinballoo avatar Aug 01 '24 12:08 kelvinballoo

I have one observation and it might not be directly under the scope of this ticket. Let me know if this needs fixing and thus, a new ticket:

  • If I dismiss the audience creation notice, the 'All Visitors' audience is there and we will have the text 'Additional groups' below it but it's essentially blank. Should we be hiding that text?

    Screenshot 2024-08-01 at 15 20 36

Thanks for flagging this, @kelvinballoo. It is indeed outside the scope of this issue, but it's something we should address.

The "Additional groups" label should not appear when there are no additional groups to select. Please can you create an issue for this?

techanvil avatar Aug 01 '24 14:08 techanvil

QA Update ✅

Thanks for checking @techanvil . I've raised a new ticket here : https://github.com/google/site-kit-wp/issues/9109

As for this ticket, I'm moving it to Approval as these were tested successfully:

  • After archiving the Site Kit created audiences and opening Audience Selection panel, the Audience Creation Notice was present. ✅
  • The Audience Creation Notice is as per Figma. ✅
  • Clicking Create for an audience functions well. While the creation is in progress, the Create button shows a spinner to the left of the button label. ✅
  • Upon successful creation of an audience, the audience row is removed from the Audience Creation Notice, and the audience appears in the Selection Panel. The success notice is dismissed when clicked on 'Got it'. ✅
  • Meanwhile, the audiences re-sync and Audience Creation Success Banner appears at the bottom of the panel. Also, the Audience Creation Notice updates so that only the uncreated audience is there. ✅
  • When both audiences have been created the Audience Creation Notice is dismissed. ✅
  • We can also dismiss the notice without creating the audiences. ✅

https://github.com/user-attachments/assets/74ec8982-fd89-403d-87a6-9bbae430f57c

kelvinballoo avatar Aug 01 '24 14:08 kelvinballoo