Use the `useInViewSelect()` hook where applicable in the Audience Segmentation feature
Feature Description
The various Audience Segmentation components which are within the scope of an "active" InViewProvider should be updated to use the useInViewSelect() hook in place for useSelect() for any selectors which use a resolver or result in a resolver firing via another selector.
This is to ensure that we don't trigger unnecessary API calls and other unwanted side effects when the component is not considered in-view.
An "active" InViewProvider is one whose in-view state is not hardwired to true. In practical terms for the Audience Segmentation feature, this means the InViewProvider instances rendered within WidgetAreaRenderer. So, a simpler way to describe the requirement is for any of the components rendered in the Audiences Widget Area to use the useInViewSelect() hook as described above.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Audience Segmentation components which are rendered within the scope of the Audiences Widget Area should be updated to use the
useInViewSelect()hook in place ofuseSelect()for any selectors which use a resolver or result in a resolver firing via another selector. - An observable result of this is that the widgets in the Audiences Widget Area should not trigger any API requests when it's not in-view (for the area to be considered in-view, the Traffic section should be the current section on the dashboard).
Implementation Brief
- [ ] Update the following uses of
useSelectto useuseInViewSelect:- [ ]
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/InfoNoticeWidget/index.js: https://github.com/google/site-kit-wp/blob/e12d144418423b637879d8dd0f0d181acaa89b1b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/InfoNoticeWidget/index.js#L43-L45 - [ ]
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js, NOTE: the selectors will change as part of the pivot report structure refactoring, however the same useSelects should updated once this work has been completed.: https://github.com/google/site-kit-wp/blob/e12d144418423b637879d8dd0f0d181acaa89b1b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L219-L221 https://github.com/google/site-kit-wp/blob/e12d144418423b637879d8dd0f0d181acaa89b1b/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L232-L241 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L67-L69 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L73-L75 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L98-L100 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L115-L119 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L147-L152 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L173-L178 https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js#L199-L204 - [ ]
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/ChangeGroupsLink.js: https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/ChangeGroupsLink.js#L38-L40 - [ ]
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTile/index.js: https://github.com/google/site-kit-wp/blob/0933d6c17774e59fed8990f5893a6c6c0b672eeb/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTile/index.js#L83-L106
- [ ]
Test Coverage
- No additional test coverage required.
QA Brief
Changelog entry
Thank you for the IB, @benbowler !
- I don't think we need to update the following components as they are not a part of the widget area and are rendered at the top of the dashboard on top of any widgets:
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupCTAWidget.jsassets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupSuccessSubtleNotification.js
- We should consider updating the
isPromptDismissedselector inassets/js/modules/analytics-4/components/audience-segmentation/dashboard/InfoNoticeWidget/index.jsas this is a part of the widget area and this selector invokes a resolver. - We should consider updating the
getDismissedItemsandisAudiencePartialDataselectors inassets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js. - A minor nit, but in the test coverage section, if no test updates are needed, please mention such so that it looks like this was taken into consideration.
Please let me know what you think, thank you!
@benbowler @nfmohit Just to chip in, useInViewSelect has been modified to expect direct dependencies in #8789 which will be merged soon, it is worth to mention it as part of the IB for the current issue. I will ping about this in the group as well once the PR for the referenced issue is merged
Thank you @zutigrm ! I've added #8789 as a blocker for this issue. CC: @techanvil @benbowler
Thank you for the update, @benbowler . I see that the requirement to make changes in assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupSuccessSubtleNotification.js wasn't removed. Was that left on purpose?
I missed that one thanks @nfmohit
Thanks @benbowler ! IB LGTM 👍 ✅
QA Update ✅
-
Loaded the dashboard at the monetization section, when slowly scrolling to the traffic section, the Audience Segmentation section loads as it comes into view.
https://github.com/user-attachments/assets/4456c159-6457-43d3-b752-04b833a555e5
-
Did a more detailed test with the dev tools Network tab and no report or *audience endpoints will be called until we scroll the audience segmentation widget into view.
https://github.com/user-attachments/assets/f33c1e70-0224-4cff-8bbd-6dc8ab8a5580
@nfmohit @techanvil did we miss an this? https://github.com/google/site-kit-wp/blob/ebcb7b7975da13bae3c2b91923706b4d68195805/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/AudienceItems.js#L79-L133
Good catch @aaemnnosttv! It looks like we missed it 😮 Would you like us to address this via a follow-up PR, or create a new issue?
Good catch @aaemnnosttv! It looks like we missed it 😮 Would you like us to address this via a follow-up PR, or create a new issue?
Hmm, actually this isn't a miss for the issue as specced. AudienceItems is rendered in AudienceSelectionPanel which is in turn rendered directly in DashboardMainApp (along with MetricsSelectionPanel):
https://github.com/google/site-kit-wp/blob/b6c7c59c9bece36c8b0138d2e3d4f05a2f09479a/assets/js/components/DashboardMainApp.js#L295-L297
That means it's not in a WidgetAreaRenderer, and will use the in-view state provided by Root's InViewProvider which is fixed to true, so there's no point using useInViewSelect().
https://github.com/google/site-kit-wp/blob/b6c7c59c9bece36c8b0138d2e3d4f05a2f09479a/assets/js/components/Root/index.js#L57-L64
So, it's not a miss for this issue as it stands. We could however look at improving our Selection Panels, maybe we can render them in their related WidgetAreaRenderer components so they do honour the in-view state. WDYT? @aaemnnosttv @nfmohit
So, it's not a miss for this issue as it stands. We could however look at improving our Selection Panels, maybe we can render them in their related
WidgetAreaRenderercomponents so they do honour the in-view state. WDYT? @aaemnnosttv @nfmohit
Thanks for the context @techanvil, I think your suggestion makes sense, but we could also potentially give the side panel it's own in-view state which could simply be tied to whether or not it is open or closed rather than putting it into a widget area and tying it to intersection observer, etc. The KM panel never made API requests to Analytics I don't think (unless perhaps a custom dimension was needed) but that is an on-demand action. The general principal behind the in view select was to avoid side effects that hit Google APIs from loading the dashboard all at once. Since the panel does source data about available audiences, and even reporting data, I think it makes sense that these should be guarded by the in view state where possible. I think available audiences needs to be requested to inform it's CTA or something else before it may be in view, but if so, this should be minimized as much as possible and ideally uses requests that are cached.
Closing this out for now, we can follow up with anything else in new issues 👍
As mentioned above though, we should aim to avoid requests being made by selectors within the panel components when it's closed.
Thanks @aaemnnosttv. Your suggestion to make the in-view state dependent on the open/closed state is a good one.
Actually, it got me wondering if we could take a simpler approach and make the rendering of the children in SelectionPanel conditional so they are only rendered into the DOM when the panel has been opened, so none of their rendering logic runs on page load. I gave it a quick spin, it works well on my laptop at its normal speed, but when I throttle the CPU the panel appears empty when it starts scrolling into view, so maybe this isn't such a good approach. Anyway, thought I'd mention it, food for thought and that.
The KM panel never made API requests to Analytics I don't think (unless perhaps a custom dimension was needed) but that is an on-demand action.
Actually, it does call getAvailableCustomDimensions() on page load, as per the comment this is a fix to prevent a flicker with the notice. This could be problematic if we make it an in-view select, although only with the combination of a very quick user action and a slow custom dimension sync.
https://github.com/google/site-kit-wp/blob/2919899c5e40e580142ccfc6c49e78b63e66a653/assets/js/components/KeyMetrics/MetricsSelectionPanel/CustomDimensionsNotice.js#L67-L71
I think available audiences needs to be requested to inform it's CTA or something else before it may be in view
That one has already been migrated to an in-view select, so we are OK on that front:
https://github.com/google/site-kit-wp/blob/2919899c5e40e580142ccfc6c49e78b63e66a653/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/ChangeGroupsLink.js#L36-L39
Anyhow - the main points you've raised are all good ones; maybe this issue should have gone a bit further in the first instance.
I've gone ahead created a new issue to follow up on this, which I've moved to ACR: https://github.com/google/site-kit-wp/issues/9312
So, it's not a miss for this issue as it stands. We could however look at improving our Selection Panels, maybe we can render them in their related
WidgetAreaRenderercomponents so they do honour the in-view state. WDYT? @aaemnnosttv @nfmohit
Oops, that's correct. It looks like I made an abrupt judgement mistakenly confusing AudienceItems with AudienceTiles. Thank you for correcting and for addressing this, folks!
Thanks Nahid, no worries!