• Robert Knight's avatar
    Fix scroll container initial height measurement · cf110484
    Robert Knight authored
    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
    cf110484
ThreadList.js 6.88 KB