Create useInView hook, context, and provider based on InteractionObserver/view/ref of closest element

[tofumatt]

Feature Description

For #4096 to function correctly, we need a new context, provider, and hook that tracks a div/element via a ref using InteractionObserver and informs the hook if the element is on-screen or not.

The way this should behave is that a hook (useInView) should get data from a <InViewContext.Provider> provider—because React allows multiple providers for the same context and will use whichever is "closest", this approach will allow us to always provide widgets and even the app with a high-level Provider but then allow components to use a more granular approach if desired.

That provider's value should be supplied by useIntersection.

This issue should include creating a context insider every WidgetAreaRenderer, based on the intersection of

site-kit-wp/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.js

Line 123 in a1d3e7b

<Grid

---

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

Acceptance criteria

• There should be a new context: InViewContext, used by the component defined below.
• There should be a new component offers an <InViewContext.Provider> based on InViewContext, which should be used to provide information to the useInView() hook created in Create useInViewSelect hook #4096.
• There should always be an InViewContext.Provider somewhere in the tree, so <InViewContext.Provider> should be added to the <Root> component, but this context should always return true. This means if there is, for some reason, no lower-down contexts for InView, the components will assume they're in view and render.
• There should be a new hook useInView in assets/js/hooks/useInView.js.
  • This hook should use the context created above to check if the closest parent container whose "in-view" status has been triggered. The hook should return either true or false—false if the parent component/context has NEVER been on-screen, but true as soon as it has been visible even once.
    • If the component appears on-screen, then is scrolled out-of-screen, the useInView hook should still return true.
    • It should be possible to "reset" the status of all useInView() hook's true status by dispatching an action in the datastore. (Likely this would be something like dispatch( CORE_UI ).resetInView() or similar. This may mean it's best for each useInView hook to store it's "viewed" status in the CORE_UI datastore, perhaps using an instance ID for each hook. This is up to the IB to define but it should be possible to easily and globally reset the useInView status.)

Implementation Brief

• Create the context in assets/js/components/InViewProvider/context.js: const InViewContext = createContext( false );
• Create the component in assets/js/components/InViewProvider/index.js: it should export the Provider from the context above as its default export, similar to the FeaturesProvider. (Model this entire component on the FeaturesProvider)
• The WidgetAreRender component's top-level <Grid> component (except for the hidden instance) should utilise the useIntersection() hook to check if the WidgetArea is on-screen (one pixel is enough):

  const widgetAreaRef = useRef();
  const intersectionEntry = useIntersection( widgetAreaRef, {
      rootMargin: '0px',
      threshold: 0 // Trigger "in-view" as soon as one pixel is visible.
  } );
  
  // [...]
  
  <Grid ref={ widgetAreaRef }>

• Every WidgetAreaRender should have a new top-level component: <InView.Provider value={!!intersectionEntry?.intersectionRatio}>. This context will be consumed by the useInView hook in Create useInViewSelect hook #4096.
• Add an <InView.Provider value={true}> to the Root component.
• Remove react-intersection-observer from dependencies
• Refactor existing use of useInView to use useIntersection
  • For instances where triggerOnce was used previously, use a new state variable to track this (this is currently used to prevent tracking a view event every time an element comes into view)
• Use conditional chaining for useIntersection as its return value might be null.
• Store a useState state-managed hasBeenInViewOnce value used by each useInView hook. Initially it will be based on useContext(InViewContext)'s value (true if ref is on-screen; false if off-screen), and this value should update to true anytime useContext(InViewContext)'s value updates to true. But once the state-managed value has ever been true, it should not change back to false unless reset by either the getDateRange() changing or the "reset" action is dispatched (see below):
  • Add a useEffect to useInView that's based on the current date range (select(CORE_USER). getDateRange()) as a dependency–whenever the date range updates, it should set the state-managed inView value to the real current value from the I.O. entry (essentially resetting it, but without forcing a re-render if the value hasn't changed)
  • There should be a mechanism to "reset" the useInView state for any hook that has had its useInView hook return true at least once:
    • resetInView() action: Reset all hooks by incrementing the number returned by select(CORE_UI).getValue('useInViewResetCount'), eg by dispatching dispatch(CORE_UI).setValues('useInViewResetCount', existingCount + 1)
• Add an options argument to useInView with one param: sticky. It should default to false, which means useInView() will always return false once the ref is scrolled off-screen. When this param is set to true, the hasBeenInViewOnce local state value should be returned by the useInView hook instead of the useContext(InViewContext) value.

Test Coverage

• Add Jest unit tests for useInView, mainly around the reset behaviour.

QA Brief

• Ensure all unit tests pass.
• Verify existing components that used the old useInView (assets/js/components/wp-dashboard/WPDashboardIdeaHub.js, assets/js/modules/idea-hub/components/dashboard/DashboardCTA.js, assets/js/modules/idea-hub/components/dashboard/DashboardIdeasWidget/index.js, and assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeed.js) still function as expected using the new useIntersection hook.

Changelog entry

• Implement the new useInView hook.

[aaemnnosttv]

@tofumatt this looks good to me. I made a few additions such as refactoring the useInView hook we use now to use useIntersection instead which is much simpler in what it does. I also bumped up the estimate slightly as a result.

One thing to keep in mind with this hook is that the intersectionEntry it returns may be null so we need to use conditional chaining when accessing its properties.

[tofumatt]

After a discussion with @aaemnnosttv, I've moved the "has ever been onscreen" and "sticky return values after being true" logic to this hook from useInViewSelect, which we agreed made sense.

I've upped this estimate to a 15 now since most of the logic lives here and reduced #4096's a bit. I think this is all we discussed @aaemnnosttv, so back to you to review.

I'll take this issue and #4096 in the next sprint since I have a concrete idea of their approach 🙂

[aaemnnosttv]

@tofumatt the IB LGTM overall, my only critique is around the name of the option resetWhenOffScreen which feels a bit odd to disable something for an additional opt-in behavior. How about we keep triggerOnce for consistency with the old hook? Technically it could be more than once with the reset, but otherwise it would only trigger once when enabled.

• Add a useEffect to useInView that's based on the current date range (select(CORE_USER).getDateRangeDates())

This would be getDateRange; getDateRangeDates returns { startDate, endDate } 🙂

Because resetting the array will cause all useInView() hooks to re-run, they will all re-calculate when a reset is dispatched (for instance, when the date range is changed), causing only the still-on-screen areas to render.

I think this may have been from a previous version as the array referenced here was state I think?

[tofumatt]

Hmm, triggerOnce is a bit odd because it implies the hook only triggers once and thus the check only occurs once—eg on page load. But what really is happening is that the check is always run, but the value returned will never revert to false unless reset.

I've tweaked the other points and I'm happy to rename that param, but I don't think "once" should be in the name of the param because it is a bit misleading 🙂

[aaemnnosttv]

Hmm, triggerOnce is a bit odd because it implies the hook only triggers once and thus the check only occurs once—eg on page load. But what really is happening is that the check is always run, but the value returned will never revert to false unless reset.

@tofumatt Yeah, maybe something without "once" would be more appropriate. In general, I'd prefer if the option was something we enable rather than disable. By default the value coming from the hook would be the current state (intersecting or not) which is intuitive I think; I don't really think of that as resetting when it changes. When we enable the "sticky" in-view state (maybe that would be a good name/option for it?) only then it needs to be reset, but it's only the sticky aspect of it which is being reset, not necessarily the actual in-view state as the element could still be in view in which case the context value wouldn't change 😄 Any objections to using sticky: true instead of resetWhenOffScreen: false ?

[techanvil]

QA: ❌

• ✔️ Unit tests all pass.
• ✔️ assets/js/modules/idea-hub/components/dashboard/DashboardCTA.js: An event is being tracked the first time the component scrolls into view. Subsequent scrolling does not trigger an event.
• ✔️ assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeed.js: An event is being tracked the first time the component scrolls into view. Subsequent scrolling does not trigger an event.
• ❌ assets/js/components/wp-dashboard/WPDashboardIdeaHub.js: An event is not being tracked when the component scrolls into view.
  • Digging into this a little, useEffect is not triggering when scrolling into view. I set a breakpoint here but it only breaks once when the component first renders.

    site-kit-wp/assets/js/components/wp-dashboard/WPDashboardIdeaHub.js

    Line 65 in 2b270bd

    if ( inView && ! hasBeenInView ) {

  • I stepped into useIntersection and set breakpoints here, discovering that it's never being called with ref.current defined: https://github.com/streamich/react-use/blob/10c492474900d9a7260bfee7c94f3d1cd819cdf0/src/useIntersection.ts#L10-L11.
  • Looking at the implementation of useIntersection, the use of ref.current in the useEffect dependencies is a bit of a red flag as mutating ref.current won't result in a re-render and thus won't trigger the useEffect... see https://github.com/streamich/react-use/blob/10c492474900d9a7260bfee7c94f3d1cd819cdf0/src/useIntersection.ts#L25
• Test setup: Only the ideaHubModule feature flag enabled, and Search Console setup (initially I also had Analytics setup but then removed it).
• Verified locally, and on a Jurassic Ninja site switching between main and develop.

[tofumatt]

Alright, finally tracked down the cause here: anytime we use useIntersection we can't start by rendering null at the outset. These lines are the issue:

site-kit-wp/assets/js/components/wp-dashboard/WPDashboardIdeaHub.js

Lines 78 to 80 in 1be7723

	if ( ! savedIdeas?.length ) {
	return null;
	}

If we add:

if ( savedIdeas === undefined ) {
	return <div ref={ trackingRef }></div>;
}

above those lines everything works as-expected. I'll go through and make the relevant changes to any components using useIntersection now.

[techanvil]

Nice one @tofumatt.

From what I can see it seems like the old react-intersection-observer function didn't have this restriction. What's the reason to switch from that to the react-use version?

[techanvil]

Also I wonder if it's possible/worth writing a thin layer around useIntersection which warns when the ref.current isn't set... Did your investigation suggest that might be doable at all?

[tofumatt]

What's especially odd is in that Idea Hub component it causes issues, but elsewhere it's fine—what seems to be the issue is if the component directly does return null or does what some widgets do which is return <Null />.

I'm a bit puzzled; the main reason for switching dependencies was reducing the number of external libraries as the more "advanced" features of react-intersection-observer weren't needed by us, but it seems it might be the better choice afterall. I'll take a look 😅

[aaemnnosttv]

@techanvil I think one reason was due to useIntersection taking the ref to use as an argument, whereas useInView before returned the ref to use. Mainly though due to the simplicity I think. There's also the resettable sticky detail that it has which didn't seem very straight forward to do (at least if we wanted to reuse its triggerOnce option) with react-intersection-observer.

[techanvil]

@aaemnnosttv hmm that's interesting about the ref angle. By the look of it, the useInView ref approach, using a callback instead of a createRef object (see https://github.com/thebuilder/react-intersection-observer/blob/ef0bb84363601992d55a48e26ee19faeb3dd0303/src/useInView.tsx#L51), is what makes useInView a little more robust as it can always react to the ref being set, whereas useIntersection depends on the useEffect firing while the ref is set (https://github.com/streamich/react-use/blob/10c492474900d9a7260bfee7c94f3d1cd819cdf0/src/useIntersection.ts#L10-L11), which seems like it's not happening when the component is initially rendering null...

Anyway, it's useful to get the info, thanks for the detail!

[eugene-manuilov]

@techanvil sending it back to you for another round of QA

[techanvil]

QA: ✅

• ✔️ Unit tests all pass.
• ✔️ assets/js/modules/idea-hub/components/dashboard/DashboardCTA.js: An event is being tracked the first time the component scrolls into view. Subsequent scrolling does not trigger an event.
• ✔️ assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeed.js: An event is being tracked the first time the component scrolls into view. Subsequent scrolling does not trigger an event.
• ✔️ assets/js/components/wp-dashboard/WPDashboardIdeaHub.js: An event is being tracked the first time the component scrolls into view. Subsequent scrolling does not trigger an event.