-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unselect record table records on table body click #8306
Conversation
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.
PR Summary
Refactored table record unselection handling by consolidating click-outside detection and keyboard shortcuts into a dedicated component.
- Renamed
RecordTableInternalEffect
toRecordTableBodyUnselectEffect
for clearer responsibility - Added
tableBodyRef
inRecordTable.tsx
to track clicks outside table area - Fixed type mismatch between
HTMLTableElement
andHTMLDivElement
for table body ref - Simplified focus management by removing
recordTableId
parameter fromuseLeaveTableFocus
3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
@@ -32,6 +34,8 @@ export const RecordTable = ({ | |||
objectNameSingular, | |||
onColumnsChange, | |||
}: RecordTableProps) => { | |||
const tableBodyRef = useRef<HTMLTableElement>(null); |
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.
logic: Type mismatch between tableBodyRef (HTMLTableElement) and RecordTableBodyUnselectEffect props (HTMLDivElement)
<StyledTableInternalContainer ref={tableRef}> | ||
<RecordTable | ||
viewBarId={viewBarId} | ||
recordTableId={recordTableId} | ||
objectNameSingular={objectNameSingular} | ||
onColumnsChange={handleColumnsChange} | ||
/> | ||
<DragSelect | ||
dragSelectable={tableBodyRef} | ||
dragSelectable={tableRef} | ||
onDragSelectionStart={() => { | ||
resetTableRowSelection(); | ||
}} | ||
onDragSelectionChange={setRowSelected} | ||
/> |
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.
logic: DragSelect is now positioned inside StyledTableInternalContainer which could affect its behavior if the container has overflow constraints
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.
Drag selection isn't working properly if you start your selection outside the table body
We have previously fixed the unselection of table records on click outside. However, the ref was mispositioned as it selected the full height table. In the case of low record numbers, we also want the unselection to happen on table body click