Scroll to leaf nodes instead of the branches to avoid janky scrolling behavior#81
Conversation
|
@mellis481 given that the recreation scenario is likely one that is similar to our flagship product’s scenarios (ie large size of items) this seems to be a fix that we would also want to take advantage of. So can you bring in UIEN-3615 into our current support queue and get it assigned to literally anyone on our team? This is a good opportunity to share knowledge of this library with someone who hasn’t been made familiar yet. My expectation is that they fully understand the well described problem (thanks @philipp-spiess) and then evaluates if the test properly recreates that problem and accurately proves it is fixed. One way to do that is to remove the fix and see that the test fails. I would also expect them to verify that no external props/interfaces have changed without the author seeking prior approval. thanks again @philipp-spiess for the contribution |
We're evaluating the use of
react-accessible-treeviewfor Sourcegraph and one issue that stood out for us that happens primarely when we're dealing with very large nested lists is that this library relies onfocus()of the branch lists to move content into the viewport. Since branches can be very long (possibly longer than the total viewport height), this causes some unexpected jumping when using arrow keys to navigate as the user agent is trying to move as much of the node that was focused into the viewport as possible.You can recreate this problem easily on the
react-accessible-treeviewpreview page, here's a video recording from the storybook to demo the issue:Screen.Recording.2023-01-19.at.14.47.03.mov
You can see in the video above that sometimes the leaf that is highlight is in the middle of the viewport and the next leaf is clearly within the viewport as well when the unexpected scrolling happens.
To fix this, I've made some changes so that in addition to the tabbable nodes (the one that the
focusshould move to), we're also keeping track of all leaf nodes. We can then usefocus({ preventScroll: true })and manually scroll to the leaf node.Note that I have put the focus after the scrolling so that in the case of
preventScrollbeing unsupported, the behavior is unchanged.The test is a bit awkward as jsdom doesn't seem to implement scrollIntoView so we have to spy on
focus()and stubscrollIntoView(). I’m happy to make changes if you have other ideas though :)Test plan
Screen.Recording.2023-01-19.at.14.19.11.mov