Skip to content
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

Merged
merged 4 commits into from
Nov 4, 2024

Conversation

charlesBochet
Copy link
Member

@charlesBochet charlesBochet commented Nov 4, 2024

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

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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 to RecordTableBodyUnselectEffect for clearer responsibility
  • Added tableBodyRef in RecordTable.tsx to track clicks outside table area
  • Fixed type mismatch between HTMLTableElement and HTMLDivElement for table body ref
  • Simplified focus management by removing recordTableId parameter from useLeaveTableFocus

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);
Copy link
Contributor

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)

Comment on lines 73 to 86
<StyledTableInternalContainer ref={tableRef}>
<RecordTable
viewBarId={viewBarId}
recordTableId={recordTableId}
objectNameSingular={objectNameSingular}
onColumnsChange={handleColumnsChange}
/>
<DragSelect
dragSelectable={tableBodyRef}
dragSelectable={tableRef}
onDragSelectionStart={() => {
resetTableRowSelection();
}}
onDragSelectionChange={setRowSelected}
/>
Copy link
Contributor

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

Copy link
Contributor

@bosiraphael bosiraphael left a 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

Enregistrement.de.l.ecran.2024-11-04.a.14.40.54.mov

@charlesBochet charlesBochet merged commit 52e5f7d into main Nov 4, 2024
18 of 19 checks passed
@charlesBochet charlesBochet deleted the fix-unselect-on-table-body-click branch November 4, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants