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

Implement the “partial data” badges

Open techanvil opened this issue 1 year ago • 5 comments

Feature Description

This includes the badge at the top of the Audience Tile and the badge for the “Top content” metric.

See audience tiles > partial data in the design doc.

Note that as discussed on Figma, the copy for the "Top content" badge tooltip will read as follows:

Still collecting full data for this timeframe, partial data is displayed for this metric

While, as seen on Figma the badge at the top of the tile's tooltip will read as follows:

Still collecting full data for this timeframe, partial data is displayed for this group


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

Acceptance criteria

  • As described in the design doc, and following the Figma design (see the "partial data" badge for both the audience tile as a whole, and for the "Top content" metric - and please also check the mobile designs):
    • If an audience is determined to be in the “partial data” state, it should show a badge at the top of the tile.
    • If the “Top content” metric is determined to be in the ”partial data” state it should display a badge in line with the metric title.
      • To determine the “partial data” state, we’ll query a report for the earliest tracked event for the custom dimension used by the metric. For further detail see https://github.com/google/site-kit-wp/issues/8141.
    • In desktop viewports:
      • The audience-level badge will contain an info icon, which will display a tooltip when hovered or tapped. The tooltip will show the following copy: Still collecting full data for this timeframe, partial data is displayed for this group
      • The "Top content" metric badge will also provide a tooltip, with the following copy: Still collecting full data for this timeframe, partial data is displayed for this metric
    • In mobile viewports, the mobile design should be followed which doesn't provide tooltips and rather renders the above tooltip text inline in the badges.
    • Note that only one instance of the badge should be shown for a given audience; the “Top content” badge should not be shown if the audience itself is in the “partial data” state.
    • Note too that for both audiences and the “Top content” metric, the “partial data” badge should not be shown if the current property itself is in the “partial data” state. This is to avoid showing a “partial data” badge when the entire dashboard is effectively partial data, i.e. when Site Kit and the Audience Segmentation feature were both set up on the same day, as it would be confusing to show the badge when the entire dashboard is in the same state.
    • Please confirm the copy with Figma, which is the source of truth.

Implementation Brief

  • [ ] Create PartialDataBadgeDesktop in assets/js/modules/analytics-4/components/audience-segmentation/dashboard folder.
  • [ ] It should take the tooltipTitle prop which will be of type propTypes.node.
  • [ ] It should use the Tooltip component to show the tooltip text and the children inside it should have Partial data translatable string with the info icon as seen in the Figma design.
    • [ ] Style the component to match figma design. Styles should be placed on assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-partial-data-badge.scss
  • [ ] Create PartialDataBadgeMobile in assets/js/modules/analytics-4/components/audience-segmentation/dashboard folder.
  • [ ] This should take content prop and render it inside a styled div as seen in the Figma design for mobile.
  • [ ] In assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTile/index.js file:
    • [ ] Add a new audienceResourceName prop.
    • [ ] Get the propertyID using the getPropertyID selector.
    • [ ] Get whether the current property is in partial data using the isPropertyPartialData selector.
    • [ ] Get whether the audience is partial data using the isAudiencePartialData selector.
    • [ ] Get whether googlesitekit_post_type custom dimension is in partial data using the isCustomDimensionPartialData selector.
    • [ ] Render PartialDataBadge** components in appropriate places in the markup when the conditions met.
      • [ ] If property is in partial data:
        • [ ] Badges for audience and custom dimension should not be rendered.
      • [ ] If the property is not in partial data:
        • [ ] If the audience is in partial data:
          • [ ] The badge for the whole audience should be rendered.
          • [ ] The badge for custom dimension should not be rendered.
        • [ ] If the audience is not in partial data:
          • [ ] Only the badge for custom dimension should be rendered (In top content area, see the design for exact placement).
  • [ ] In assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTiles.js
    • [ ] Pass the newly added prop audienceResourceName to the rendered AudienceTiles elements.

Test Coverage

  • Add story variants and VRTs for various partial data states.

QA Brief

  • Go to Storybook -> Modules -> Analytics4 -> Components -> Audience Segmentation -> Dashboard -> AudienceTile.
  • Verify the new Audience partial data and Top content partial data story variants and ensure that the behaviour is in line with Figma and the ACs.

Changelog entry

techanvil avatar Jan 24 '24 17:01 techanvil

Added a bit of higher estimate because of the creation of new components which involves lot of viewport dependent styling, which always gets complicated. Cheers.

kuasha420 avatar May 13 '24 06:05 kuasha420

Thanks @kuasha420. I've made a minor tweak for the type of the tooltipTitle prop. This IB LGTM, nice one!

techanvil avatar May 13 '24 15:05 techanvil

@techanvil @kuasha420 Quick question: The AC mentions that the tooltip should be shown when the info icon is hovered over, however, the IB implies that it should open when anywhere in the badge is hovered. Which should we go with? From the Figma designs, it does look like the tooltip appears immediately above the info icon.

nfmohit avatar May 17 '24 11:05 nfmohit

@nfmohit, thanks, good spot. The tooltip should indeed be triggered when the icon is hovered - this will be consistent with elsewhere, for example the info tooltip for the Audience Tile.

In fact, I missed this during the IB review but it looks like we should use the InfoTooltip component, as the AudienceTile component does, rather than using Tooltip directly:

https://github.com/google/site-kit-wp/blob/5d1b1b7de7c52b230b8d3746ac81848f615ca140/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTile/index.js#L74-L82

techanvil avatar May 17 '24 15:05 techanvil

Thanks for the confirmation, @techanvil!

nfmohit avatar May 17 '24 16:05 nfmohit

QA Update ❌

Tested this and it's looking good overall. I do have some observations though. They are documented down here:

ITEM 1: I noticed that on Storybook, the tooltip would appear at the bottom instead of the top on Figma. Can you confirm that this is because of the window spacing that we have above and that by default, it will appear at the bottom?

Implementation: Screenshot 2024-05-25 at 10 33 22

Figma: Screenshot 2024-05-25 at 10 35 50


ITEM 2: The margins around the badge is not consistent with Figma Refer to the details below.

Implementation: 19px at the top and 25px on the side Screenshot 2024-05-25 at 10 37 41

Figma: 24px at the top and side and 20px on the bottom Screenshot 2024-05-25 at 10 39 29


ITEM 3: At the metric level badge on desktop:

  • Top margin should be 16px instead of 12px
  • Bottom margin should be 14px instead of 10px
  • Note that on the details the tooltip popup is on top of the badge and not above placement of tooltip Details are below
**Implementation:**

Top margin is 12px Screenshot 2024-05-25 at 10 46 37

Bottom margin is 10px Screenshot 2024-05-25 at 10 46 58

No overlap between tooltip and badge: Screenshot 2024-05-25 at 10 48 32

Figma: Top margin is 16px Screenshot 2024-05-25 at 10 44 30

Bottom margin is 14px: Screenshot 2024-05-25 at 10 44 51

Tooltip overlaps with badge: Screenshot 2024-05-25 at 10 49 49

ITEM 4:

Bottom margin should be 12px instead of 16px

Implementation: Margins are 16px top, bottom and sides Screenshot 2024-05-25 at 10 25 37

Figma: Bottom is 12px. Screenshot 2024-05-25 at 10 52 33

kelvinballoo avatar May 25 '24 06:05 kelvinballoo

Thank you for sharing your observations, @kelvinballoo!

ITEM 1: I noticed that on Storybook, the tooltip would appear at the bottom instead of the top on Figma. Can you confirm that this is because of the window spacing that we have above and that by default, it will appear at the bottom?

That is correct. I can confirm that the tooltip by default will show up at the top, unless there isn't any space available at the top, in which case, it will appear at the bottom.

ITEM 2: The margins around the badge is not consistent with Figma Refer to the details below.

We are not changing the dimensions of the tile header here because that is outside the scope of this issue.

If you look at the Figma designs, the tile heading seems to have an inconsistency when it comes to height, for example, in this mock, the tile header has a height of 72 pixels, while here, it has a height of 76 pixels. It might be worth raising this inconsistency internally to see if an additional issue is needed to apply the correct dimensions.

Instead of changing the tile header height in this issue, we're simply adding the partial data badge but keeping the height same as it was before so that it doesn't look inconsistent with other tiles that may not have the badge.

ITEM 3: At the metric level badge on desktop:

  • Top margin should be 16px instead of 12px
  • Bottom margin should be 14px instead of 10px
  • Note that on the details the tooltip popup is on top of the badge and not above placement of tooltip Details are below

For the top and bottom margins, I have opened a follow-up PR #8760 to increase them.

However, regarding the overlap of the tooltip into the badge, we use our re-usable tooltip component here (and everywhere else) which by principle does not overlap with the host element. I believe it would be wiser to follow the same behavior here for consistency.

nfmohit avatar May 27 '24 11:05 nfmohit

QA Update ✅

Thanks @hussain-t ,

Reviewed the top and bottom the partial badge at the metric level and results are looking good:

  • 16px at the top

  • 14 px at the bottom

    12px + 4 px = 16px at the top: Screenshot 2024-05-29 at 17 50 50 Screenshot 2024-05-29 at 17 54 26

    14px at bottom: Screenshot 2024-05-29 at 17 50 38

Remaining ACs were tested good. The new Audience partial data and Top content partial data story variants match Figma closely. There are some bits that are not exactly the same because of consistency across other parts of the website. The part about inconsistency in the Figma designs, I'll raise this separately internally.

Screenshot 2024-05-29 at 17 57 28 Screenshot 2024-05-29 at 17 57 37

Moving ticket to 'Approval'

kelvinballoo avatar May 29 '24 13:05 kelvinballoo