From 9745d6df3e093496724360c7c644fadf8b547f2c Mon Sep 17 00:00:00 2001
From: infinite-persistence <inf.persistence@gmail.com>
Date: Mon, 5 Jul 2021 11:27:22 +0800
Subject: [PATCH] WaitUntilOnPage: Fix scroll listener always registered + bump
 debounce ms

## Issue
The scroll listener never unregisters, and is always registering itself on every scroll. I believe it was done that way to also handle the case of "element is already in viewport when mounted".

## Change
Tried to separate both "element is already in viewport when mounted" and "element scrolled into viewport" into different effects.

The timeout value used is a bit arbitrary, but is needed because the initial size is (0, 0), and to debounce any layout shifts. Reasoning: If an element is explicitly placed under this wrapper, the additional delay is acceptable since it's meant to be lazy-loaded anyway.
---
 ui/component/common/wait-until-on-page.jsx | 59 ++++++++++++++--------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/ui/component/common/wait-until-on-page.jsx b/ui/component/common/wait-until-on-page.jsx
index 53cb9886e..5d2bb734f 100644
--- a/ui/component/common/wait-until-on-page.jsx
+++ b/ui/component/common/wait-until-on-page.jsx
@@ -2,7 +2,7 @@
 import React from 'react';
 import debounce from 'util/debounce';
 
-const DEBOUNCE_SCROLL_HANDLER_MS = 25;
+const DEBOUNCE_SCROLL_HANDLER_MS = 50;
 
 type Props = {
   children: any,
@@ -14,33 +14,48 @@ export default function WaitUntilOnPage(props: Props) {
   const ref = React.useRef();
   const [shouldRender, setShouldRender] = React.useState(false);
 
-  React.useEffect(() => {
-    const handleDisplayingRef = debounce((e) => {
-      const element = ref && ref.current;
-      if (element) {
-        const bounding = element.getBoundingClientRect();
-        if (
-          bounding.bottom >= 0 &&
-          bounding.right >= 0 &&
-          // $FlowFixMe
-          bounding.top <= (window.innerHeight || document.documentElement.clientHeight) &&
-          // $FlowFixMe
-          bounding.left <= (window.innerWidth || document.documentElement.clientWidth)
-        ) {
-          setShouldRender(true);
-        }
+  const shouldElementRender = React.useCallback((ref) => {
+    const element = ref && ref.current;
+    if (element) {
+      const bounding = element.getBoundingClientRect();
+      if (
+        bounding.width > 0 &&
+        bounding.height > 0 &&
+        bounding.bottom >= 0 &&
+        bounding.right >= 0 &&
+        // $FlowFixMe
+        bounding.top <= (window.innerHeight || document.documentElement.clientHeight) &&
+        // $FlowFixMe
+        bounding.left <= (window.innerWidth || document.documentElement.clientWidth)
+      ) {
+        return true;
       }
+    }
+    return false;
+  }, []);
 
-      if (element && !shouldRender) {
-        window.addEventListener('scroll', handleDisplayingRef);
-        return () => window.removeEventListener('scroll', handleDisplayingRef);
+  // Handles "element is already in viewport when mounted".
+  React.useEffect(() => {
+    setTimeout(() => {
+      if (!shouldRender && shouldElementRender(ref)) {
+        setShouldRender(true);
+      }
+    }, 500);
+  }, []); // eslint-disable-line react-hooks/exhaustive-deps
+
+  // Handles "element scrolled into viewport".
+  React.useEffect(() => {
+    const handleDisplayingRef = debounce(() => {
+      if (shouldElementRender(ref)) {
+        setShouldRender(true);
       }
     }, DEBOUNCE_SCROLL_HANDLER_MS);
 
-    if (ref) {
-      handleDisplayingRef();
+    if (ref && ref.current && !shouldRender) {
+      window.addEventListener('scroll', handleDisplayingRef);
+      return () => window.removeEventListener('scroll', handleDisplayingRef);
     }
-  }, [ref, setShouldRender, shouldRender]);
+  }, [ref, setShouldRender, shouldRender, shouldElementRender]);
 
   const render = props.skipWait || shouldRender;