Skip to content

fix(tables): row gutter click toggles select; select-all works under sort/filter#4446

Open
waleedlatif1 wants to merge 1 commit intostagingfrom
waleedlatif1/table-row-select-fixes
Open

fix(tables): row gutter click toggles select; select-all works under sort/filter#4446
waleedlatif1 wants to merge 1 commit intostagingfrom
waleedlatif1/table-row-select-fixes

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Clicking anywhere in the row gutter cell now toggles row selection, matching the boolean cell pattern. Previously only the inner checkbox div was hot.
  • Select-all and Cmd/Ctrl+A now produce a correct selection range under sort and filter. The previous code computed `maxPosition` from the last visual row's `position`, which under sort is not the largest position — the resulting range missed rows and the highlight didn't render.
  • `isAllRowsSelected` now checks that every loaded row's `position` falls within the selection range, rather than comparing the range's exact endpoints to a single `maxPosition`. This keeps the top-left checkbox highlight correct under any sort/filter combination.

Test plan

  • Click anywhere in a row's gutter cell (not just the checkbox) → row checks/unchecks
  • Per-row run button still triggers run/stop and does not toggle the checkbox
  • Apply a sort, click the top-left select-all checkbox → all rows highlight
  • Apply a sort, press Cmd/Ctrl+A → all rows highlight
  • Apply a filter, click select-all → all visible rows highlight
  • Unsorted, unfiltered: select-all and Ctrl+A still work as before

…sort/filter

- Move row-toggle onMouseDown from inner checkbox div to entire gutter
  <td> so clicking anywhere in the gutter cell toggles the row, matching
  boolean cell behavior. Stop propagation on the per-row run button so
  it doesn't double as a row toggle.
- Compute maxPosition via Math.max over loaded rows instead of
  rows[last].position. Under sort, the last visual row's position is
  not the largest position, so select-all/Ctrl+A produced a range that
  missed rows whose position fell outside [0, lastVisualRow.position].
- Add minPosition for the same reason and use it as the select-all
  anchor — under filter, position 0 may not be loaded, so anchoring at
  0 produced a range that visually appeared empty.
- Re-derive isAllRowsSelected from "every loaded row's position is
  within selection range" rather than checking exact start/end equality
  against maxPosition, so the top-left checkbox highlight is correct
  under any sort/filter state.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 5, 2026 5:35am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 5, 2026

PR Summary

Low Risk
Low risk UI-only changes to table selection logic and click handling; main risk is subtle regressions in row selection/highlighting behavior under edge cases (paged rows, gaps, shift-selection).

Overview
Fixes table row selection behavior when rows are sorted/filtered and improves the row-gutter hit area.

select all (and Cmd/Ctrl+A) now anchors the selection using the min/max row position across loaded rows (not assuming the last rendered row has the highest position), and isAllRowsSelected now verifies that every loaded row falls within the selection range.

Clicking anywhere in the row-number gutter cell (including gap rows) now toggles row selection, while the per-row run/stop button stops propagation so it doesn’t toggle selection.

Reviewed by Cursor Bugbot for commit 9a4e61a. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR fixes three related bugs in the table component: maxPosition was incorrectly read from the last visual row instead of computed as the true maximum across all rows (causing select-all to miss rows under sort), isAllRowsSelected was checking exact endpoint equality instead of verifying that every loaded row's position falls within the range, and the gutter cell's click target was too narrow (only the inner checkbox div). The fixes are correct and the run-button stopPropagation guard is a nice catch. One minor gap: the Ctrl+Space column-select handler and handleGroupSelect still hardcode rowIndex: 0 as the selection anchor, the same pattern fixed for Ctrl+A elsewhere in the same file.

Confidence Score: 4/5

Safe to merge; the core fixes are correct and the only remaining issue is a minor inconsistency in two non-primary code paths.

All three advertised bugs are correctly fixed. The single P2 finding (residual rowIndex: 0 in Ctrl+Space and handleGroupSelect) is a pre-existing inconsistency that the PR does not claim to address and has no data-loss risk.

No files require special attention beyond the noted Ctrl+Space / handleGroupSelect anchor inconsistency in table.tsx.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx Fixes max/min position computation for select-all under sort/filter, expands gutter click target to full td, and adds run-button propagation stop. One residual rowIndex: 0 anchor in the Ctrl+Space handler (and handleGroupSelect) was not updated.

Comments Outside Diff (1)

  1. apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx, line 1348-1350 (link)

    P1 The same rowIndex: 0 anchor bug that was fixed for Ctrl+A and handleSelectAllRows is still present in the Ctrl+Space handler. Under a sort/filter where minPosition > 0, the anchor lands at a position with no loaded row, which could cause errant scroll-to or navigation behavior. handleGroupSelect (line 809) has the same issue.

Reviews (1): Last reviewed commit: "fix(tables): row gutter click toggles se..." | Re-trigger Greptile

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.

1 participant