• Robert Knight's avatar
    Fix thread list jumping when scrolling on mobile · 84996e44
    Robert Knight authored
    Re-measurement of the height of visible threads happened in an effect
    that depended only on the set of top-level threads, not the subset that
    were currently visible.
    
    On desktop, hover events would cause annotations to be focused as the
    user scrolled through the list, resulting in the root thread being
    recalculated regularly and hence the internal `threadHeights` state in
    `ThreadList` was usually accurate.
    
    On mobile however, hover events don't fire and therefore `threadHeights` was
    not updated as often as it should be. The mismatch between the actual
    rendered height of threads and the value in `threadHeights` caused the
    thread list to jump as a thread above the viewport was transitioned from a
    rendered to a non-rendered state. This issue can be reproduced in Chrome
    on desktop when the device type is set to "Mobile (Touch)" in dev tools.
    
    This PR fixes the issue by making recalculation depend on the set of
    _visible_ threads. In practice `visibleThreads` changes on most renders,
    however the amount of work the effect does if no measured heights
    changed is small, so this shouldn't be an issue.
    
    In addition to fixing this issue, this commit also adds a test for the
    overall behavior as the user scrolls through the list. Since correct
    behavior depends on complex interaction between the DOM, `ThreadList`
    and logic in two utility modules, we avoid mocking either the DOM or
    utilities in these tests.
    84996e44
thread-list-test.js 10.9 KB