Fix scroll position jumping when shift tabbing into non-virtualized tableview#2233
Fix scroll position jumping when shift tabbing into non-virtualized tableview#2233LFDanLu wants to merge 12 commits into
Conversation
allows non virtualized tables/grids created with useTable/useGrid to have automatic scrolling when moving focus via arrow keys
some components will have a ref that is not the same element as the scrollable region so add a prop for scrollRef
…ualized table preserves the scroll position if item is in view, otherwise scrolls it into view
LFDanLu
requested review from
dannify,
devongovett and
snowystinger
as code owners
also refined the detection for when a element is out of view
LFDanLu
changed the title
|
TODO: fix the tests and fix the case where user shift tabs back into the collection and the previously focused item was 3/4ths of the way down the list. If it is close enough to the bottom it scrolls down too much. Perhaps I have to prevent the tab default behavior still... |
| useEffect(() => { | ||
| let onFocus = (e) => { |
There was a problem hiding this comment.
Problem with this approach: if the user focuses a cell -> scrolls so that it is out of view -> moves focus out of the table -> clicks on the table column header -> the table's scrollable region will scroll to bring the previous cell into view. This happens because this capturing listener triggers before useGridCell/useSelectableItem's onFocus can make the column header the new focusedKey, thus this block thinks it needs to scroll the old cell into view.
Not sure what to do here
| useEffect(() => { | ||
| let onKeyDown = (e) => { |
There was a problem hiding this comment.
This block was added to block the scrollable region from scrolling since shift tabbing back into the table will focus the last checkbox in the table before it shifts focus back to the tableview's focused key -> causes a scroll to happen -> messes up the calculation of scrollIntoView.
Problem is that this doesn't quite work when tabbing from outside the window (e.g. browser bar)...
There was a problem hiding this comment.
There also exists a bug for non-virtualized tables where tabbing from the browser bar to the table will focus the select all checkbox -> causes the focused key to update, overwriting the previously focused key if one existed.
https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/grid/src/useGridCell.ts#L200-L202 is supposed to stop cases like that but isFocusVisible doesn't return true when tabbing from the browser bar. This is because https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/interactions/src/useFocusVisible.ts#L132 won't capture the tab keydown from the browser bar to the table since it is outside the document
|
Build successful! 🎉 |
LFDanLu
changed the title
|
This feels pretty hacky... gonna need some second opinions, might be missing a more obvious approach |
|
Build successful! 🎉 |
|
This seems like... a lot. Can you explain why the solution we use in Virtualizer didn't work here? |
|
@devongovett Well, the changes in this PR were an attempt to mimic what we do in Since this is the non-virtualizer case and I don't have access to The |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Closing in favor of #2314 |
3 participants
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: