-
Notifications
You must be signed in to change notification settings - Fork 295
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
Implement the zero data state of an Audience Tile #8143
Comments
Hi @ankitrox, thanks for drafting this IB. A few points:
As ever, feel free to drop me a line if you want to get on a call to discuss any of this. |
Hi @techanvil , Thank you for reviewing the IB and adding the feedback. I have addressed the points you mentioned and revised the IB description. Assigning this to you for re-review. Thank you. |
Hey @ankitrox, thanks for updating the IB. I've actually updated it further myself; I had started a minor update to correct some grammatical points and a couple of minor errors, however, it ended up being more of a rewrite than I'd initially intended. Please can you review the changes, again if you are happy then feel free to move it to the EB. I'll give this an IB ✅ for now but if you want to discuss any of it just let me know :) |
Thank you @techanvil . I have reviewed the changes and those are looking good to me, so moving this to EB. Thanks again! |
…-tile Enhance/#8143 - Implement the zero data state of an Audience Tile
QA Update ❌I tested this but some parts are not working out as they should. ITEM 1: ❌ I think this might only happen when the tile is in position 2. Video is attached below for reference:
https://github.com/google/site-kit-wp/assets/125576926/02228bed-194d-4906-8de8-93a2b0a9c01a
ITEM 2: ❌
When 'All Visitors' is in position 1, there is data for the first 4 lines with 5.9K visitors, etc...:
When the same tile is swapped in position 2, there is no data. It's worth noting that the analytics figures at the bottom are showing accordingly. ITEM 3: ITEM 4: |
Hi @kelvinballoo, thanks for raising your observations.
That's a good catch. The logic changed while working on the CR feedback, which caused it. I have fixed it in this follow-up PR.
You're referring to the display name, not the
That's a good catch. However, it is an existing issue, not part of the changes made to this ticket. I confirmed it by reverting all the changes made in this PR. Could you please create a separate ticket? cc: @techanvil
Unfortunately, we don't have a Figma design for the zero data state single tile.
That's expected, not an issue. |
Thanks @hussain-t ! |
Enhance/#8143 - Zero Data Audience Tile (follow-up)
Back to you for another pass @kelvinballoo! |
QA Update ✅Tested this and it's looking good
Moving this ticket to approval. |
Feature Description
Implement the zero data state of an audience. This includes the “temporarily hide” behavior.
See audience tiles > zero data in the design doc.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
AudienceTileCollectingData
andAudienceTileCollectingDataHideable
.AudienceTileCollectingData
will display the SVG icon and the text "Site Kit is collecting data for this group.".AudienceTileCollectingDataHideable
will display the SVG icon, the text "Site Kit is collecting data for this group. You can hide this group until data is available." and the "Temporarily hide" link.Temporarily hide
CTA should dispatch adismissItem
action withaudience-tile-${audienceResourceName}
for the slug and0
for the expiration.AudienceTiles
component:hasZeroDataForAudience
inside the component to check if the slice of the report that relates to the specific audience has zero data.hasZeroDataForAudience
should acceptaudienceResourceName
as the parameter.hasZeroDataForAudience
should check for a row where theaudienceResourceName
dimension matches the audience's name, and verify that it has a non-zero count for thetotalUsers
metric.isAudiencePartialData
selector of theanalytics-4
module.hasZeroDataForAudience
created previously.visibleAudiences
array - it should initially be created fromconfiguredAudiences
.configuredAudiences
have been temporarily dismissed).visibleAudiences
still has a length greater than 1, remove the audience fromvisibleAudiences
.dismissItem
selector with a very short expiry time (for example, 1 second).visibleAudiences
and do the following:visibleAudiences
is only 1, display theAudienceTileCollectingData
component.visibleAudiences
is greater than 1, display theAudienceTileCollectingDataHideable
component.AudienceTile
component as it is doing in the current implementation.Test Coverage
AudienceTileCollectingData
andAudienceTileCollectingDataHideable
components.AudienceTiles
component which shows both of the above components.QA Brief
Verify Zero Data State for Audience Tile
Temporarily Hide Audience Tile
dismiss-item
endpoint with the correct parameters:audience-tile-${audienceResourceName}
for the slug and0
for theexpiration
.Reappearing the Temporarily Hidden Tile
wp_googlesitekitpersistent_dismissed_items
item from thewp_usermeta
table in the database. This will reset the dismissed state and allow the tile to reappear.Component Stories
Changelog entry
The text was updated successfully, but these errors were encountered: