Update Selection Panels to be in-view aware, and use the `useInViewSelect()` hook where applicable

techanvil commented 1 month ago

Feature Description

Update the Audience and Metric Selection Panels to be in-view aware, and update them to use the useInViewSelect() hook in place of useSelect() for any selectors which use a resolver or result in a resolver firing via another selector.

This is a followup to 8879, see the comments on that issue for more context: https://github.com/google/site-kit-wp/issues/8879#issuecomment-2332401260

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

Acceptance criteria

The generic SelectionPanel component should wrap its children in an InViewProvider whose in-view state is determined by the open state of the panel.
Components which are rendered as descendents of the Audience and Metric Selection Panels should be updated to use the useInViewSelect() hook in place of useSelect() for any selectors which use a resolver or result in a resolver firing via another selector.
An observable result of this is that the Selection Panels should not trigger any API requests upon page load, unless the request is explicitly determined to be required.
The flicker that PR https://github.com/google/site-kit-wp/pull/7779 fixed in the Metrics Selection Panel should not return as a result of these changes.
The panels should in general continue to behave as expected.

Implementation Brief

[ ] Update assets/js/components/SelectionPanel/SelectionPanel.js, importing InViewProvider and wrapping { children } in this component, passing isOpen to the value prop.

Key Metrics Selection Panel

Update the following useSelect's to use useInViewSelect: from within this selection panel and it's children:

Audience Segmentation Panel

Update the following useSelect's to use useInViewSelect: from within this selection panel and it's children:

Test Coverage

No additional test coverage required, confirm that existing tests pass.

QA Brief

Changelog entry

eugene-manuilov commented 2 weeks ago

An observable result of this is that the Selection Panels should not trigger any API requests upon page load, unless the request is explicitly determined to be required.

@techanvil if the goal is to ensure that no API requests are triggered when the selection panel is closed, then why not to just not render children?

techanvil commented 2 weeks ago

@eugene-manuilov it's a fair question, and one I considered myself. However, as discussed here, I found it would cause a regression for slower machines as the panel would appear empty while it scrolls into view.

eugene-manuilov commented 1 week ago

I found it would cause a regression for slower machines as the panel would appear empty while it scrolls into view

@techanvil, we can show a progress bar/preview block while data is being loaded, can't we? This is the standard pattern that we use for components that depends on data that loads on demand.

techanvil commented 1 week ago

@eugene-manuilov, we do already show the Audience Selection Panel in a loading state, which is dynamic based on the current cached audience list and audience selection.

So, we've already got that aspect covered in the current implementation. We could restructure things so we have an explicitly defined "loading" component which we switch out for the loaded version when the panel is opened and the loaded state has been determined, but I think the proposed in-view approach would probably be easier to manage.

eugene-manuilov commented 1 week ago

Ok, thanks, @techanvil. AC ✔️

google / site-kit-wp