-
Notifications
You must be signed in to change notification settings - Fork 14
fix: table moves up when doing selection #559
Conversation
const bodyCellStyle = useMemo(() => getBodyCellStyle(layout, theme), [layout, theme.name()]); | ||
const hoverEffect = layout.components?.[0]?.content?.hoverEffect; | ||
const cellStyle = { color: bodyCellStyle.color, backgroundColor: theme.table.backgroundColor }; | ||
const TableBodyWrapper = forwardRef( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To fix a warning, now it looks weird
Reference:
https://stackoverflow.com/questions/59716140/using-forwardref-with-proptypes-and-eslint
facebook/react#16653
@@ -73,7 +74,7 @@ export default function TableWrapper(props) { | |||
return () => { | |||
memoedWrapper.removeEventListener('focusout', focusOutCallback); | |||
}; | |||
}, []); | |||
}, [keyboard]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does keyboard
affect the hebaviour of focusout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, if keyboard.enabled is false it does nothing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good
const memoedContainer = tableBodyWrapperRef.current; | ||
if (!memoedContainer) return () => {}; | ||
|
||
const keyDownHandler = (evt) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have a quick way to test it, have to leave it.
top: 0, | ||
behavior: 'smooth', | ||
top: Math.max(0, targetOffsetTop), | ||
behavior: 'instant', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that instant is more like a native one
I'm looking at the |
Sounds good. I am looking into it. |
switch to #627 |
Head:
when a focused cell is updated, the scroll position is updated accordingly.
Current:
when a focused cell is updated only when using the keyboard to move up, the scroll position is updated accordingly