Add the Audience Tile happy path view (Storybook)
Feature Description
Create the individual audience tile component, implementing the happy path view (i.e. the main view showing the metrics for an audience), and add it to Storybook.
Note this does not include the loading state or any other state variants which will be implemented separately.
See audience tiles > individual tiles in the design doc.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- A new component along with its Storybook story should be added for audience tile (happy path).
- Appropriate props should be added as per the design doc.
- The UI should be similar to the Figma designs.
- The component should follow the relevant Figma designs at mobile breakpoints (see the phone and tablet designs).
- On phones and tablets, the title is part of a tabbed component.
Implementation Brief
- [X] Add
assets/js/modules/analytics-4/components/widgets/AudienceSegmentation/AudienceTile.js- Include the following props:
titleinfoTooltipvisitorsvisitsPerVisitorspagesPerVisitpageviewstopCitiestopContentWidget
- Include the following props:
- [x]
visitors,visitsPerVisitors,pagesPerVisit, andpageviewsprops, should receive value as an object. They will be extracted from report response in parent component, and will represent the metric values. The object should look like this{previousValue: x, currentValue: x, metricValue: x}. Mimic their usage to what we have inMetricTileNumericcomponent. - [x]
topCitiesandtopContentwill receive an object in format{dimensionValues: [], metricValues: [], total: x }. It will represent the data that is used inTopCitiesWidgetcomponent for example, and then you can borrow the logic fromMetricTileTablecomponent - [x] Wrap the component output with
<Widget noPadding>, and a main childdivwith class, saygooglesitekit-audience-segmentation-tile - [x] Borrow the logic for
infoTooltipcomponents from one of the key metrics widgets - [x] Tile title and info tooltip should be grouped under same
divwrapper - [x] Include additional badge in Top content section - currently rendered unconditionally as it is not in scope of this issue, and match the styling and content
Partial datawith figma design.- Also handle the case if
topContentortopCitiesprop are null, a messageNo data to show yetshould be displayed instead of their data
- Also handle the case if
- [x] Use
useBreakpointhook to extract the current device size, so that tile title withinfoTooltipare conditionally rendered in following manner:- in case of
BREAKPOINT_TABLETorBREAKPOINT_SMALLtitle should not be rendered - Otherwise show title
- in case of
- [x] Add
assets/sass/components/analytics-4/audience-segmentation/audience-tile.scssand apply the styles to match the figma designs linked in AC for desktop and mobile
Test Coverage
- Add
AudienceTiletests and stories
QA Brief
- View the new component in storybook confirming it matches the design doc and mockups at all breakpoints.
Changelog entry
- Add the Audience Tile in its "happy path" state as a component which is viewable in Storybook.
@techanvil, why do we have a separate task for this storybook story? It should be part of the #8131 ticket, shouldn't it? We usually create new stories when we work on actual components. Or I miss something?
@eugene-manuilov, #8131 only relates to the UI and logic encapsulated in the Setup CTA Banner component, whereas this issue is about introducing the individual Audience Tile component, initially in Storybook.
@eugene-manuilov, #8131 only relates to the UI and logic encapsulated in the Setup CTA Banner component, whereas this issue is about introducing the individual Audience Tile component, initially in Storybook.
Thanks, @techanvil, I was confused by what it means (Storybook) in the title. So, basically we need to implement the corresponding component, but not connect it to the plugin UI yet.
- A new Storybook story should be added for the individual audience tile component as per the design doc.
@asvinb, let's probably rephrase AC a little bit because currently it is confusing as it feels like the main purpose of this task is to create a storybook story. AC should say that first of all we need to create the new component Audience Tile and corresponding storybook story(ies) for it.
Thanks, @techanvil, I was confused by what it means
(Storybook)in the title. So, basically we need to implement the corresponding component, but not connect it to the plugin UI yet.
That's right @eugene-manuilov. Thanks for pointing out the confusion, I can see what you mean. I've updated the Feature Descriptions for all of the issues that involve creating a component and adding it to Storybook to make them clearer.
Thanks @eugene-manuilov . Let me know if it's clearer now. Thanks!
Yes, thanks, Asvin. AC ✔️
IB ✔️
QA Update ❌
Tests were done on Dev environment and Chrome Desktop on MacOS Ventura. Since there is quite a number of feedback on desktop, I prefer to get those ironed out first before moving to cross browser and cross device checks. Tested URL: https://google.github.io/site-kit-wp/storybook/develop/?path=/story/modules-analytics4-components-audiencesegmentation-audiencetile--ready Figma reference: https://www.figma.com/file/7pSrkEy8t00BcYRAi9LjjH/Audience-Segmentation?node-id=1046%3A42107&mode=dev
Issues and screenshots are detailed below:
ISSUE 1 The overall tile margin on the side should be 24px and not 16px.
Screenshots
Figma: 24px
Storybook implementation: 16 px
ISSUE 2 Title of tile and tooltip are missing along with the line below it.
Screenshots
Figma: Highlighted all the needed elements
Storybook implementation: Elements missing
ISSUE 3 Overall height of a section should be 76px and not 80px
Screenshots
Figma: 76 px in height
Storybook implementation: 80 px (56+12+12)
ISSUE 4 Right margin to the right of the icon should be 20px. It's currently 19px which is rather odd
Screenshots
Figma: 20px
Storybook implementation: 19px
ISSUE 5 Icon is not centrally aligned vertically
Screenshots
Figma: Both arrows are the same size and icon is vertically centred
Storybook implementation: The arrows are of the same size but there is a white gap, meaning the icon is not centrally aligned on the verical
ISSUE 6 There should be a margin gap before the start of the icon.
Screenshots
Figma: 16px gap with the start of the grey line
Storybook implementation: No gap implemented
ISSUE 7 For the page view stats, it's in the K format on storybook but in figma, it's the full number e.g. 1,565 And for the %, should we remove the decimal place?"
Screenshots
Figma: Full number format: 1565 and percentage is without decimal place
Storybook implementation: No gap implemented
ISSUE 8 The space below 'Cities with the most visitors' is 14 px but it should be 12px Column gap should be 10px instead of 16px. There should be a 2px gap between the city and the %
Screenshots
Storybook implementation:
ISSUE 9 There are multiple items to highlight under the last section. Better refer to the screenshot
Screenshot
Storybook implementation:
Hey @kelvinballoo, thanks for your QA.
RE Issue 1, 2 and 6, the total width or your storybook window must be >1200px to load the desktop breakpoint, in these issues you were viewing the mobile or tablet breakpoint where these elements should be missing or padding/layout is different.
RE Issue 3, I've manually set the pixel line-heights of the text to match the height for each metric section.
RE Issue 4 and 5, fixed.
RE Issue 7, is consistent with other usage of digits of there range in the Key Metrics widgets and other dashboard widgets.
RE Issue 8 and 9, fixed.
QA Update ❌
Thanks @benbowler , great work.
Retested as follows and I just need you to look into Issues 2, 9, 10.
ISSUE 1 : ✅
Looking good with a 24px margin after enlarging the viewport.
Screenshot
24px margin on the sides
ISSUE 2 : ❌
We'll still need to look into issue 2 as the tooltip is not present.
Also, the 'V' in 'Visitors' should be lower case.
Screenshots
Implementation
Figma
ISSUE 3 ✅ The sections are now 76px.
Screenshot
52+12+12 = 76 px
ISSUE 4 ✅ The margin is now 20px, not 19.
Screenshot
20px on the right of the icon
ISSUE 5 ✅ The icon is not vertically centred.
Screenshot
ISSUE 6 ✅ Retesting on desktop viewport, the margin now appears accordingly and is 16px.
Screenshot
ISSUE 7 ✅ Noted on the approach of making this consistent with other sections.
ISSUE 8 ✅ All the mini-issues have been resolved.
ISSUE 9 ❌ All the mini-issues have been resolved except for the bottom gap. While we do have a 12 px gap, we have an extra 16px gap on top. Refer to the screenshots below:
Screenshots
Figma: 12+16
Implementation: only 12 (+4 from the text)
ISSUE 10 🆕
This is linked issue 2 but documenting it here so that it's more clear-cut.
While we don't have figma for mobile/tablet, I do want to double check on why we have omitted the Title on those breakpoints.
I think the title is an important piece of information that should not get hidden on smaller screen.
Happy to be overruled though.
Image are attached
Screenshots
Currently on mobile/tablet viewport, the Title 'New visitors' and the tooltip get hidden
Reference for desktop view
Thanks @kelvinballoo, updates below:
ISSUE 2 : ❌: You can see the tooltip version of the story here.
ISSUE 9 ❌: I made the margin bottom 16px in total rather than adding an additional 16px. PR connected to this ticket with the change and will move to CR once checks pass.
QA Update ⚠️
ISSUE 2 is fine ✅ . Spotted the tooltip on the Storybook URL given.
Screenshot
Size (16x16) and margin of 5 px are as expected
ISSUE 9 is fine ✅ . The extra 16px margin has been implemented.
Screenshot
ISSUE 10 :warning: Need further review on this.
While we don't have figma for mobile/tablet, I do want to double check on why we have omitted the Title on those breakpoints. I think the title is an important piece of information that should not get hidden on smaller screen. Happy to be overruled though.
@benbowler , this might have been overlooked. Let me know your thoughts. If there is no concern, we can mark this as pass. Else, if this needs further discussion with UX/UI team, happy for this to be moved to another ticket/thread.
Hey @kelvinballoo, the reason the title is hidden on smaller breakpoints is that, in these breakpoints, the AudienceTile component will be loaded in a tabbed layout by the parent AudienceTiles component. These tabs will show the title of each tile in the tabs themselves, hence we can hide the title in the child component otherwise the title will be duplicated in the tabs and the tile itself. Does that make sense?
QA Update ✅
Thanks for the clarification @benbowler ! That makes sense 💯
Moving this to 'Approval'