Skip to content
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

Add the Selection Panel happy path view (Storybook) #8157

Closed
32 of 47 tasks
techanvil opened this issue Jan 25, 2024 · 27 comments
Closed
32 of 47 tasks

Add the Selection Panel happy path view (Storybook) #8157

techanvil opened this issue Jan 25, 2024 · 27 comments
Labels
Module: Analytics Google Analytics module related issues P1 Medium priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@techanvil
Copy link
Collaborator

techanvil commented Jan 25, 2024

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 #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 #8162, #8164 and #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 Refactor the Key Metrics Selection Panel, extracting components for reuse in the Audience Segmentation Selection Panel. #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 Ensure that audiences are listed in the correct order. #8519.

Implementation Brief

Please refer POC PR: #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

  • Open Storybook.
    • Open the Modules -> Analytics4 -> Components -> AudienceSegmentation -> Dashboard -> AudienceSelectionPanel` stories.
    • Verify the audience selection panel looks and behaves according to the Figma designs and the ACs.

Changelog entry

  • Add the Audience Selection Panel as a component which is primarily visible in Storybook pending full integration.
@techanvil techanvil added Module: Analytics Google Analytics module related issues P1 Medium priority Type: Enhancement Improvement of an existing feature labels Jan 25, 2024
@hussain-t hussain-t self-assigned this Feb 1, 2024
@techanvil
Copy link
Collaborator Author

techanvil commented Feb 5, 2024

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 #8141 to the dependencies for this issue. Cc @ivonac4

@techanvil
Copy link
Collaborator Author

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.

@ivonac4 ivonac4 added the Team M Issues for Squad 2 label Apr 9, 2024
@techanvil
Copy link
Collaborator Author

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.

@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Apr 16, 2024
@nfmohit nfmohit assigned nfmohit and unassigned hussain-t and nfmohit Apr 19, 2024
@ankitrox ankitrox self-assigned this Apr 25, 2024
@techanvil techanvil reopened this Apr 25, 2024
@ankitrox ankitrox removed their assignment Apr 26, 2024
@hussain-t
Copy link
Collaborator

@ankitrox, could you set an estimate?

@ankitrox
Copy link
Collaborator

@hussain-t Added the estimate.
Thank you.

@techanvil techanvil self-assigned this Apr 29, 2024
@techanvil
Copy link
Collaborator Author

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 Add and hook up the Change groups CTA, integrating the Selection Panel and its core happy path functionality #8158 to be merged in order to retrieve the list of selected widgets. However, Add and hook up the Change groups CTA, integrating the Selection Panel and its core happy path functionality #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 techanvil assigned ankitrox and unassigned techanvil May 1, 2024
@nfmohit nfmohit assigned techanvil and unassigned nfmohit Jun 5, 2024
@techanvil
Copy link
Collaborator Author

Thanks @nfmohit, that looks great. Over to QA for testing :)

@techanvil techanvil removed their assignment Jun 5, 2024
@kelvinballoo
Copy link
Collaborator

kelvinballoo commented Jun 6, 2024

QA Update ⚠️

Tested and I have a few observations and queries, documented as follows:

ITEM 1:
All visitors' should be 'Google Sans Text' instead of 'Google Sans Display'

Storybook: ![Uploading Screenshot 2024-06-06 at 16.11.57.png…]()

Figma:
Screenshot 2024-06-06 at 16 12 54

ITEM 2:
People who visited the site for the first time font weight should be 400 instead of 500.

Storybook: Screenshot 2024-06-06 at 16 14 20

Figma:
Screenshot 2024-06-06 at 16 15 31

ITEM 3:
Font weight for 'Save selection' button should be 500 instead of 400.
That said, I did see that the KM panel would be 400. So if this is to keep things consistent with KM, then we can ignore this.

Storybook: Screenshot 2024-06-06 at 16 17 41

Figma:

Screenshot 2024-06-06 at 16 17 57

ITEM 4:
There are a few points in the AC that I don't think can be tested in this Storybook ticket as the Storybook data are not real data linked to a real account. I've listed them below.
Can you confirm that indeed these are items not to be tested in this ticket

  • 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.

  • 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."

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 7, 2024

Thank you for sharing your observations, @kelvinballoo !

ITEM 1: All visitors' should be 'Google Sans Text' instead of 'Google Sans Display'

Storybook: Uploading Screenshot 2024-06-06 at 16.11.57.png…
Figma: Screenshot 2024-06-06 at 16 12 54

This is consistent with what we have in the Key Metrics selection panel. If we should change the font here, I believe we should change it in the KM panel as well for consistency. @sigal-teller Could you confirm if the font should be changed here to Google Sans Text for both KM and audience selection panels?

ITEM 2:
People who visited the site for the first time font weight should be 400 instead of 500.

I'm opening a follow-up PR for this.

ITEM 3:
Font weight for 'Save selection' button should be 500 instead of 400.
That said, I did see that the KM panel would be 400. So if this is to keep things consistent with KM, then we can ignore this.

As you mentioned, this uses the re-usable component that both KM and audience selection panels use, so if we want to change it here, we should change it everywhere else for consistency.. @sigal-teller We need your confirmation here as well to see if this change should be applied for all selection panels. Thanks!

  • 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.

As this is a Storybook-only issue, unfortunately, these cannot be tested here, but can be done in #8158. I'll add them in the QAB for #8158.

  • The user count for an audience should be displayed on the right hand side of it.

You should be able to see the user count from the mock report data in Storybook.

image

You'll be able to see actual data in #8158.

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 link is correct, however, unfortunately, you'll not be able to see it in Storybook. It should be verifiable in #8158.

The link doesn't scroll down to the respective section though. @techanvil mentioned he is going to open an issue for it.

Thanks!

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 7, 2024

I have opened a follow-up PR #8829 to address some of the concerns above. For this to move forward, we have two questions for @sigal-teller that we need for confirmation:

ITEM 1: All visitors' should be 'Google Sans Text' instead of 'Google Sans Display'

Storybook:
image
Figma:
image

This is consistent with what we have in the Key Metrics selection panel. If we should change the font here, I believe we should change it in the KM panel as well for consistency. @sigal-teller Could you confirm if the font should be changed here to Google Sans Text for both KM and audience selection panels?

ITEM 3:
Font weight for 'Save selection' button should be 500 instead of 400.
That said, I did see that the KM panel would be 400. So if this is to keep things consistent with KM, then we can ignore this.

As you mentioned, this uses the re-usable component that both KM and audience selection panels use, so if we want to change it here, we should change it everywhere else for consistency.. @sigal-teller We need your confirmation here as well to see if this change should be applied for all selection panels. Thanks!

Thank you!

@sigal-teller
Copy link
Collaborator

@nfmohit The links you added appears broken for me so I can see exactly what you're referring to. In any case, I'm using the styles that we have in the design system, so I'm not sure why the font is changing for you. Maybe there are missing fonts or styles for you and this is why replaces them? Anyway, group names (if that was what you were referring to) are label medium. LMK if you were referring to something else.

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 10, 2024

Hi @sigal-teller. I have updated the links for you, thank you for pointing that out. In summary, we have two questions:

1st:
Should we use "Google Sans Text" for the checkbox labels in the selection panel? The KM panel, when implemented, used the "Google Sans Display" font, but in the current Figma designs for Audience Segmentation, it uses "Google Sans Text".

Screenshots

In the current KM selection panel:
image

In Audience Segmentation Figma designs:
image

2nd:
Should the font-weight for the "Save selection" CTA be 400 or 500. In the KM selection panel, it is 400, but in the Audience Segmentation Figma designs, it is 500.

Screenshots

In the current KM selection panel:
image

In the Audience Segmentation Figma designs:
image

@sigal-teller
Copy link
Collaborator

Hi @nfmohit

  1. Checkboxes labels should be "Google sans text" (this what is used for Label/medium" in Figma).
  2. font weight for buttons should be 500 (buttons text are Label/medium as well). you can find them in the design system here. Also attaching a screenshot.
Screenshot 2024-06-11 at 16 57 35

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 11, 2024

Thank you for confirming, @sigal-teller. I'll make these updates accordingly.

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 11, 2024

@kelvinballoo This is now in CR with PR #8829 that addresses your observations: ITEM 1 & ITEM 2.

For ITEM 3, I have opened a new issue #8856.

The other questions have been addressed in my previous comments.

Thanks!

@kuasha420 kuasha420 self-assigned this Jun 11, 2024
kuasha420 added a commit that referenced this issue Jun 11, 2024
Audience selection panel style improvements
@kuasha420
Copy link
Contributor

@kelvinballoo Back to you for another pass, the ITEM 1 and 2 should now be fixed. As noted by @nfmohit above, the 3rd item is a much bigger change, so it will be fixed in a follow up issue.

@kuasha420 kuasha420 assigned kelvinballoo and unassigned kuasha420 Jun 11, 2024
@kelvinballoo
Copy link
Collaborator

kelvinballoo commented Jun 12, 2024

QA Update ✅

All the issues are resolved or have an action in another ticket. Moving this ticket to 'Approval'.

ITEM 1:
All visitors' is now 'Google Sans Text'.

Screenshot 2024-06-12 at 17 20 20
________________________________________________________________

ITEM 2:
People who visited the site for the first time font weight is now 400.

Screenshot 2024-06-12 at 17 22 12
________________________________________________________________

ITEM 3:
Noted that this will be handled under #8856.


ITEM 4:
Noted that this is out of scope for this test.

@kelvinballoo kelvinballoo removed their assignment Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues P1 Medium priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

9 participants