site-kit-wp
site-kit-wp copied to clipboard
Implement the loading state for the Audience Tile
Feature Description
Implement the loading state for the Audience Tile.
See audience tiles > loading state in the design doc.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- An Audience Tile in the loading state should be displayed for each audience in the current list of selected audiences while the required data is loading, with preview blocks shown in place of the metric rows.
- The loading state should follow the Figma design.
- In terms of data loading, the following data should be retrieved and processed:
- The "partial data" state should be resolved (see #8142).
- The cached list of available audiences should be resynced if the cache is stale (i.e. if an hour or more has passed since the last sync. See #8486).
- The list of selected audiences should be validated against the list of available audiences. If any of the selected audiences are no longer available, they should be removed from the selection.
- The metrics should be retrieved for the tile(s).
- Note that if an audience is removed from the selection, its tile may be replaced by a placeholder tile - however this is handled via #8146 and doesn't need to be addressed here.
- Additionally if all audiences are removed from the selection, the tiles will be replaced by the "no audiences" banner, but again this is handled separately (see #8155), and doesn't need addressing here.
Implementation Brief
- [ ] In
assets/js/modules/analytics-4/datastore/audiences.js:- [ ] Create a new action
maybeSyncAvailableAudiences:- [ ] Get the
availableAudiencesLastSyncedAtAnalytics Module settings value usinggetAvailableAudiencesLastSyncedAtselector. - [ ] Check whether audience cache needs resync using the
availableAudiencesLastSyncedAttimestamp and run thesyncAvailableAudiencesif so. - [ ] Get the
availableAudiencesandconfiguredAudiencesusing their respectiveget**selectors. - [ ] Remove any
configuredAudiencesthat are no longer available inavailableAudiences.
- [ ] Get the
- [ ] Create a new action
- [ ] In
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/index.js:- [ ] Create a new state,
availableAudiencesSyncedusinguseStatehook which is initiallyfalse. - [ ] Get the
syncAvailableAudiencesaction from Analytics module usinguseDispatchhook. - [ ] On mount, using
useMounthook:- [ ] Call the newly created
maybeSyncAvailableAudiencesaction. - [ ] Set
availableAudiencesSyncedtotrueafterwards.
- [ ] Call the newly created
- [ ] Update the logic for rendering
AudienceTileswhenavailableAudiencesSyncedis true in addition to the currenthasMatchingAudience. ie.if ( availableAudiencesSynced && hasMatchingAudience ) { ...
- [ ] Create a new state,
- [ ] In
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/index.js:- [ ] Update the loading state preview to match Figma design. This can be located under the
TODO: Loading states will be implemented as part of https://github.com/google/site-kit-wp/issues/8145.comment.
- [ ] Update the loading state preview to match Figma design. This can be located under the
Test Coverage
- Add test coverage for newly added action.
- Add test coverage for the newly introduced logic for
AudienceTilesWidgetcomponent. - Update any failing tests.
QA Brief
Changelog entry
I've moved this back to Backlog as the final in-progress changes to the design doc, relating to audience caching, will probably affect the AC for this one
The audience caching aspect of the design doc has been sufficiently finalised, and I've moved this back to AC.
Hey @kuasha420, thanks for drafting this IB.
- [ ] Update the logic for rendering
AudienceTileswhenavailableAudiencesSyncedis true in addition to the currenthasMatchingAudience. ie.if ( availableAudiencesSynced && hasMatchingAudience ) { ...
Hmm, if I understand correctly this will result in nothing being displayed for the tiles while maybeSyncAvailableAudiences() is running. This means there would be a layout shift when the action has finished and we proceed to show the tiles (or the no audiences banner, etc).
However this layout shift is something we want to avoid - we should instead be showing the tiles for the currently configured selection in their loading state while the sync is happening, in the assumption that the most likely path is for the audiences to still exist so we can show the tiles without a layout shift occurring. Of course if one of the edge cases occurs then we'll show the appropriate component and there may be a shift at that point, but we want to optimise for the UX for the most likely happy path view.
Thank you for the review and raising this great point about layout shift. We definitely want to optimize the layout shift for the happy path. I've updated the IB accordingly. Please, let me know what you think.
Cheers.
Thanks @kuasha420, that's looking good. A couple of last points:
- I've tweaked the IB to address a couple of minor copy/paste level issues, please review the recent edit and make sure it's correct.
- I think the
useMount()should in fact be auseEffect()as we probably want to useuseInViewSelect()to select the values foravailableAudiencesLastSyncedAt,availableAudiencesandconfiguredAudiences, and only dispatchmaybeSyncAvailableAudiences()when they are all defined, in order to be in-view compliant. If that SGTY, please update the IB accordingly.
Thanks @techanvil. The Edits are perfect. Thank you. I've also updated the use of useMount with a regular effect. I'm not sure if this is necessary, as the action will only call API once an hour and the effect doesn't depend on the other values that are being selected with useInViewSelect but it makes sense for consistency. Let me know what you think. Cheers.
Thanks @kuasha420! I do think it's worthwhile in order to avoid making any requests while the widget is not in-view, which is our preferred pattern. That update LGTM.
IB :white_check_mark:
QA Update ✅
- Tested on main environment.
- Verified that the implemented loading state match with the Figma design.
- Verified that loading state appears when user change audience tiles from the widget.
- Verified that the list of selected audiences validated against the list of available audiences. If any of the selected audiences are no longer available, they gets remove from the selection.
- Verified the metrics gets retrieved for the tile(s).
Loading state-
https://github.com/user-attachments/assets/c61b522c-70d1-46d0-a513-b2bb6ac88e02
Audience Removal-
https://github.com/user-attachments/assets/9b641480-8815-40e1-9293-a30956c8c6b1