Fix scroll container initial height measurement

Fix an issue that caused `ThreadList` to sometimes have an incorrect (too small)
internal `scrollContainerHeight` state after the app loaded, resulting in not
enough annotation cards being rendered for the height of the sidebar.

`ThreadList` measures the height of the scroll container after the
initial render in a `useLayoutEffect` callback. It then registers handlers for
window resize events in a `useEffect` callback order to re-measure the height
when it changes.

The sequence of events that lead to the incorrect state was:

 1. The sidebar iframe starts out hidden, by a `display: none` on a container of
    the iframe in the parent frame.
 2. When the `useLayoutEffect` callback runs, the iframe is hidden and so the
    scroll container height is measured as zero.
 3. The iframe is then shown and a `resize` event is fired at the window
 4. The ThreadList's `useEffect` callback runs and registers a handler for the
    window's `resize` event

Because the initial window `resize` event is fired before the listener is
registered, the scroll container height did not get re-measured.

The fix is to combine the `useLayoutEffect` and `useEffect` callbacks from steps
(2) and (4) so that the `resize` listener is registered immediately after the
initial size is measured. Combining the effects is also beneficial from a
readability perspective as it locates all the related state and logic together
in the component.

Fixes https://github.com/hypothesis/client/issues/3915

src/sidebar/components/ThreadList.js

@@ -47,21 +47,38 @@ function roundScrollPosition(pos) {
  * @param {ThreadListProps} props
  */
 function ThreadList({ threads }) {
-  // Height of the visible area of the scroll container.
-  const [scrollContainerHeight, setScrollContainerHeight] = useState(
-    window.innerHeight
-  );
+  // Client height of the scroll container.
+  const [scrollContainerHeight, setScrollContainerHeight] = useState(0);
 
-  // Scroll offset of scroll container. This is updated after the scroll
-  // container is scrolled, with debouncing to limit update frequency.
-  // These values are in multiples of `SCROLL_PRECISION` to optimize
-  // for performance.
+  // Scroll offset of scroll container, rounded to a multiple of `SCROLL_PRECISION`
+  // to avoid excessive re-renderings.
   const [scrollPosition, setScrollPosition] = useState(0);
 
+  // Measure the initial size and offset of the scroll container once rendering
+  // is complete and attach listeners to observe future size or scroll offset changes.
   useLayoutEffect(() => {
+    const listeners = new ListenerCollection();
     const scrollContainer = getScrollContainer();
+
     setScrollContainerHeight(scrollContainer.clientHeight);
     setScrollPosition(roundScrollPosition(scrollContainer.scrollTop));
+
+    const updateScrollPosition = debounce(
+      () => {
+        setScrollContainerHeight(scrollContainer.clientHeight);
+        setScrollPosition(roundScrollPosition(scrollContainer.scrollTop));
+      },
+      10,
+      { maxWait: 100 }
+    );
+
+    listeners.add(scrollContainer, 'scroll', updateScrollPosition);
+    listeners.add(window, 'resize', updateScrollPosition);
+
+    return () => {
+      listeners.removeAll();
+      updateScrollPosition.cancel();
+    };
   }, []);
 
   // Map of thread ID to measured height of thread.
@@ -140,30 +157,6 @@ function ThreadList({ threads }) {
     scrollContainer.scrollTop = yOffset;
   }, [scrollToId, topLevelThreads, threadHeights]);
 
-  // Attach listeners such that whenever the scroll container is scrolled or the
-  // window resized, a recalculation of visible threads is triggered
-  useEffect(() => {
-    const listeners = new ListenerCollection();
-    const scrollContainer = getScrollContainer();
-
-    const updateScrollPosition = debounce(
-      () => {
-        setScrollContainerHeight(scrollContainer.clientHeight);
-        setScrollPosition(roundScrollPosition(scrollContainer.scrollTop));
-      },
-      10,
-      { maxWait: 100 }
-    );
-
-    listeners.add(scrollContainer, 'scroll', updateScrollPosition);
-    listeners.add(window, 'resize', updateScrollPosition);
-
-    return () => {
-      listeners.removeAll();
-      updateScrollPosition.cancel();
-    };
-  }, []);
-
   // When the set of visible threads changes, recalculate the real rendered
   // heights of thread cards and update `threadHeights` state if there are changes.
   useEffect(() => {