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

Published 20 hours ago • verified publisher icon google

Add the Selection Panel happy path view (Storybook)

Open techanvil opened this issue 1 year ago • 14 comments

Feature Description

Create the Selection Panel component, implementing the happy path view, and add it to Storybook.

This should include the CTAs but not the behaviour for saving the selection, this will be handled separately via https://github.com/google/site-kit-wp/issues/8158.

Additionally it should not include the loading state, the Audience Creation Notice or the Selection Panel Info Notice, these will be also handled separately via https://github.com/google/site-kit-wp/issues/8162, https://github.com/google/site-kit-wp/issues/8164 and https://github.com/google/site-kit-wp/issues/8159.

It should include the logic/behaviour described in the selection panel > available audiences section.

See selection panel in the design doc.

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

Acceptance criteria

  • The Audience Segmentation Selection Panel component should be created and stories for it added to Storybook.
  • It should follow the Figma design, and should also be based on the Key Metrics selection panel.
  • The generic components extracted from the KM Selection Panel should be reused (see google/site-kit-wp#8652), with changes to the design for Audience Segmentation applied back to the KM panel (notably the footer layout in the mobile view as mentioned in the design doc).
  • The Selection Panel should display the list of available audiences that is cached in the DB (i.e. stored in the availableAudiences setting). Their display names and descriptions should be displayed directly, with the exception of the "All users" default audience which should be shown as "All visitors".
  • The “Purchasers” default audience should only be listed if it’s had any users in it since the associated Analytics property was created.
  • Audiences should be selectable via checkboxes and, when there is a currently saved selection, grouped into "Current selection" (the currently saved selection) and "Additional groups" groups.
  • The user count for an audience should be displayed on the right hand side of it.
  • The Selection Panel should display the header text including the link to the Admin Settings tab on the Settings screen.
    • If the Audience Segmentation Settings section is available at implementation time this link should ensure the section is scrolled into view when navigating to the Settings screen. Otherwise, a small followup issue should be created to update the link when the section is available.
  • The bottom part of the Selection Panel should include:
  • Note that the special case for retrieving reports using the newVsReturning dimension to avoid the “partial data” state as discussed in the design doc's selection panel > available audiences section should not be implemented here, this will be handled separately via google/site-kit-wp#8160.
  • Note too that the specific ordering of audiences within the selection panel will be handled via the subsequent issue #8519.

Implementation Brief

Please refer POC PR: https://github.com/google/site-kit-wp/pull/8676. Also, this implementation will be done once #8652 is merged.

  • [ ] Create directory AudiencesSelectionPanel inside assets/js/modules/analytics-4/components/AudienceSegmentation/
    • [ ] Create constants.js inside new directory. Add the following constants in it similar to key metrics component.
      • [ ] Add AUDIENCES_SELECTION_PANEL_OPENED_KEY, AUDIENCES_SELECTION_FORM, AUDIENCES_SELECTED, MIN_SELECTED_AUDIENCES_COUNT and MAX_SELECTED_AUDIENCES_COUNT.
    • [ ] Create index.js which will have AudiencesSelectionPanel component in it.
      • [ ] It should use the SelectionPanel component and pass the relevant props to it.
      • [ ] Make use of AUDIENCE_SELECTION_PANEL_OPENED_KEY constant to pass isOpen prop to SelectionPanel.
      • [ ] onOpen prop should accept a callback function where the callback should add the list of saved audiences to AUDIENCES_SELECTED. List of saved audiences can be retrieved using getConfiguredAudiences selector.
      • [ ] closePanel prop should be a callback to set AUDIENCES_SELECTION_PANEL_OPENED_KEY value to false. This callback will also be passed to header component to be triggered on close button's click added in header.
      • [ ] Within the SelectionPanel component. Following components needs to be rendered, which would be passed as children to the SelectionPanel component.
        • Header
        • AudienceItems
        • Footer
    • [ ] Create Header component inside AudiencesSelectionPanel directory.
      • [ ] It should make use of SelectionPanelHeader and pass following props to it.
        • [ ] title - Heading text for the selection panel.
        • [ ] onCloseClick - Callback to trigger when selection panel is closed. This can be same callback passed to SelectionPanel.
        • [ ] children This should display the text under the main heading with the settings link. This is a component created with createInterpolateElement. Refer the figma design for the copy.
    • [ ] Create AudienceItems component inside AudiencesSelectionPanel directory.
      • [ ] It should leverage the SelectionPanelItems component and pass the necessary props to it.
      • [ ] Pass current selection title and available selection title according to figma designs.
      • [ ] availableSavedItems and availableUnsavedItems should be calculated by fetching available audiences using getAvailableAudiences selector which returns the available audiences in property and getConfiguredAudiences which returns the saved audiences in DB.
      • [ ] getAvailableAudiences should be modified to check if "Purchasers" audience has had any data. This can be checked with getResourceDataAvailabilityDate from the partial data infra. If there is any data available for the "Purchasers" i.e. getResourceDataAvailabilityDate does not return 0 or undefined, we should keep "Purchasers" in getAvailableAudiences response.
      • [ ] If there has never been any data for "Purchasers", filter out the "Purchasers" from getAvailableAudiences response as we do not want to show "Purchasers" audience in this case.
      • [ ] While passing the availableSavedItems and availableUnsavedItems props, they must be mapped to include title, description, subtitle and userCount properties.
        • [ ] title would be displayName from audiences response.
        • [ ] description belongs to description property.
        • [ ] subtitle is the text which appears below description. As seen in the figma designs, it will be Created by .... text.
          • [ ] If the audience is "All users" or "Purchasers", subtitle would be "Created by default by Google Analytics"
          • [ ] If the audience is "New visitors" or "Returning visitors", subtitle would be "Created by Site Kit".
          • [ ] For all other audience types, subtitle should be "Already exists in your Analytics property".
        • [ ] userCount would be the total number of users in audience for the selected period. This should use the getReport selector from analytics module where the metric would be totalUsers and dimension is audienceResourceName.
        const options = {
  startDate: "2024-05-09",
  endDate: "2024-05-15",
  metrics: "totalUsers",
  dimensions: [ 'audienceResourceName' ]
};

const result = googlesitekit.data.select('modules/analytics-4').getReport( options );
      • [ ] Purchasers audience should only be listed if it’s had any users in it since the associated Analytics property was created. For the same, we can use the isResourcePartialData selector to check if there "Purchasers" has had any audience.
      • [ ] For savedItemSlugs pass the value returned by getConfiguredAudiences which is array of name in audience response.
      • [ ] ItemComponent should be AudiencesItem which will be responsible to render individual selection item in the panel.
        • [ ] This component will receive title, description, subtitle and userCount props as mentioned above.
        • [ ] Note that we need to pass userCount as suffix into (maybe wrapped into some JSX) SelectionPanelItem component. suffix prop is described in below point.
    • [ ] Add an additional prop suffix to the component SelectionPanelItem. This should be a react component which should be rendered after the SelectionBox component.
      • [ ] Default value of suffix can be a function which returns false ( suffix = () => false ), so that if nothing is passed, it should not render anything. This is particularly useful in case of KM panel.
    • [ ] Create AudiencesItem component inside AudiencesSelectionPanel directory.
      • [ ] It should leverage the SelectionPanelItem component and pass the necessary props to it. All these props should already be passed by SelectionPanelItems component down to this component.
      • [ ] We can pass description as react component which encompasses of description of the audience and created by subtitle.
    • [ ] Create Footer component inside AudiencesSelectionPanel directory.
      • [ ] This component should use SelectionPanelFooter component to return the new composed component for audience segmentation selection panel footer.
      • [ ] savedItemSlugs - These are the items returned from getConfiguredAudiences.
      • [ ] selectedItemSlugs - This can be retrieved from AUDIENCES_SELECTED inside AUDIENCES_SELECTION_FORM. Please refer this example here.
      • [ ] Refer other props in SelectionPanelFooter and pass them accordingly. A reference of MetricsFooter component can be taken to build other props.
    • [ ] It's worth noting that generic components extracted from the KM Selection Panel should be reused in audience selection panel. Note that footer changes in audience selection panel should also be applied to the KM panel.
      • [ ] In assets/sass/components/global/_googlesitekit-selection-panel.scss, target .googlesitekit-selection-panel-footer__content and make it display block.
      • [ ] .googlesitekit-selection-panel-footer__item-count should be text aligned left and .googlesitekit-selection-panel-footer__actions should be text aligned right with display block.
      • [ ] It's essential that both audience selection panel and KM panel should reflect this change.

Test Coverage

  • Add the AudiencesSelectionPanel in storybook with the following scenarios.

    • Default - Where it should show the flat list of all the audiences.
    • SavedSelection - Where it should show the Current selection and Additional groups. Current selection will show the saved items.
    • View-only user - With view only context.

  • Write the test for AudienceSegmentation/AudienceSelectionPanel component. Take reference of KM component tests. Not all of the tests in the MetricsSelectionPanel suite will apply to the AudienceSelectionPanel, so copy/modify the tests which are applicable and add any additional tests as needed.

QA Brief

Changelog entry

techanvil avatar Jan 25 '24 11:01 techanvil

Hey @hussain-t, just a heads up that I've amended the Feature Description here to make it clear that we do want to include the logic described in available audiences as part of this issue.

I've also tweaked that section in the design doc to point out that the implementation can leverage the "partial data" infrastructure, and as a result I've added https://github.com/google/site-kit-wp/issues/8141 to the dependencies for this issue. Cc @ivonac4

techanvil avatar Feb 05 '24 14:02 techanvil

Hi @hussain-t, just a heads up that I've made a small tweak to the AC to make it clear that we should be leveraging the audience cache here, as opposed to retrieving the audiences directly from the audiences API.

techanvil avatar Apr 05 '24 11:04 techanvil

Hi @hussain-t, I've also added a point to the AC to make it clear we don't need to address the specific details of audience ordering in this issue, this will be handled via #8519. This shouldn't actually have any impact on this issue, just want to give a heads up for clarity.

techanvil avatar Apr 10 '24 16:04 techanvil

@ankitrox, could you set an estimate?

hussain-t avatar Apr 29 '24 06:04 hussain-t

@hussain-t Added the estimate. Thank you.

ankitrox avatar Apr 29 '24 08:04 ankitrox

Hi @ankitrox, thanks for drafting this IB. Here are some points to consider:

  • Using the single SELECTION_PANEL_OPENED_KEY key to control the panel's open state is likely to cause problems when there are more than one version of the panel. I can imagine the shared SELECTION_PANEL_HEADING and SELECTION_PANEL_SELECTED_ITEMS potentially being problematic too. I think we should keep separate state for each version of the panel. This could be achieved by passing different key values into the component via props, or potentially via context to facilitate their use in child components. We might want to simplify things a bit by keeping all these values within their own form slice, so we effectively just need to pass a unique value for what's currently SELECTION_PANEL_FORM.
  • Additionally the MIN_SELECTED_COUNT and MAX_SELECTED_COUNT values will differ for the audiences selection panel which has a min of 1 and max 3 selected items, so these values will also need to be passed via props/context.
  • The passing of a children property in selectedItems and availableItems doesn't look right. Rather we should pass a prop like ItemComponent={Label} which would be the component function rather than a JSX rendered instance. Then we can render the component for each item, passing the item to it via props e.g. <ItemComponent item={item} />.
  • Additionally, the shape of the selectable items will be different for each instance - i.e. Key Metrics objects have a different shape to cached audience objects. We'll probably want to map these to a common shape so we'd want to effectively define an interface for a selectable item and map the specific items to it. I also think selectedItems should be a simple array of slugs, while availableItems should most likely be an array of mapped, common objects. I imagine these objects should have a shape along the lines of { slug, title, subtitle, description } (considering the commonalities between the KM selection panel and the audiences selection panel):
    • KM panel:
      image
    • Audiences panel:
      image
  • In fact, taking these commonalities into consideration maybe we don't even need ItemComponent and it should rather be an optional component for rendering a "suffix", e.g. ItemSuffixComponent, or ItemDetailComponent or something, which can then be used just to show the user count for the audiences panel.
  • There are a number of Key Metric specific aspects that have been glossed over/missed and will need extracting/abstracting from the appropriate components, for example the saving state, subheader text and behaviour in the Header component are all KM-specific, and there are similar things across all the child components. It's worth considering that by lifting this logic out of the child components we might find ourselves with an overly complex parent so it could be appropriate to extract some reusable "base" child components which we then create instance-specific wrappers in which can encapsulate the logic for those components. E.g. we extract a common Header but also create, say, a MetricsSelectionPanel/Header which contains the KM-specific logic for the header and renders the common Header within it. We don't necessarily need to get too detailed about these aspects in the IB, and we could make a general point about them for awareness rather than defining each and every prop.
  • CSS classnames should be renamed to remove/replace the km substrings so as to be generic. We should avoid passing className in unless we actually need it, a key thing we want to get out of this refactor is to keep the styling common as much as possible.
  • The IB implies that we'd need to wait for #8158 to be merged in order to retrieve the list of selected widgets. However, #8158 is a dependant of this issue so this issue needs to be merged first. We can simply specify that the list of selected widgets can be retrieved via the configuredAudiences settings for the purpose of this issue.
  • There are some minor formatting issues where backticks haven't been applied to variable names, for example selectedItems at the start of point A.3.i and similar. Also it looks like the code snippet in point C.3.i should be using the triple-backtick formatting. There's also a typo that has crept into the IB and POC, "metricss", should obviously be "metrics".
  • As a general point it's worth bearing in mind this is quite a complex issue for you to have picked up for IB quite early into your time on the project. It's commendable to be keen to get stuck in, but it might be worth ramping up a bit more slowly, going for some easier issues and building up progressively, if possible. I know we are a bit limited in terms of what's available to be picked up so it might be difficult to apply this so well in practice.

techanvil avatar May 01 '24 16:05 techanvil

Note that since my previous comment we've decided to split this issue into two, with https://github.com/google/site-kit-wp/issues/8652 being created for the initial KM Selection Panel refactor.

techanvil avatar May 02 '24 15:05 techanvil

Thank you @techanvil for breaking this task into refactoring and audience segmentation panel. This is helpful to better handle the overall requirement.

I have updated the IB considering the fact that we have a separate task for refactoring and key metrics panel. This IB take reference of your POC PR, but I think that should be fine because we are composing the components using extracted components.

Assigning this to you for review.

Thanks again!

ankitrox avatar May 09 '24 14:05 ankitrox