DataGrid: split scroll/scroll to position/should focus position handling into hooks#3986
DataGrid: split scroll/scroll to position/should focus position handling into hooks#3986
Conversation
| return scrollStateMap.get(gridRef) ?? initialScrollState; | ||
| }, [gridRef]); | ||
|
|
||
| return useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); |
There was a problem hiding this comment.
Scroll state now uses useSyncExternalStore instead of 2xuseState+flushSync
| gridRef: React.RefObject<HTMLDivElement | null>; | ||
| selectedPosition: { idx: number; rowIdx: number }; | ||
| }) { | ||
| const shouldFocusPositionRef = useRef(false); |
There was a problem hiding this comment.
This used to be a useState, but an eslint rule complained about setting state in effect.
Strangely it did not complain when the same code was in DataGrid, probably because it's too complicated for static analysis.
Though to make sure the effect works, I had to change the selectedPosition.idx dependency to selectedPosition.
But now we 1 fewer state that can trigger re-renders!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3986 +/- ##
==========================================
+ Coverage 97.53% 97.57% +0.03%
==========================================
Files 36 38 +2
Lines 1463 1486 +23
Branches 472 478 +6
==========================================
+ Hits 1427 1450 +23
Misses 36 36
🚀 New features to boost your workflow:
|
| setScrollToPosition, | ||
| scrollToPositionElement: scrollToPosition && ( | ||
| <div | ||
| ref={(div) => { |
There was a problem hiding this comment.
before: useRef+useLayoutEffect
now: 1 function
Seems to work just as well.
Since it doesn't need any hooks anymore, I've also inlined the component
2 participants
src/DataGrid.tsxis quite fat, so I'm trying to slim it down.This improves co-locality too!