Skip to content

fix(tables): suppress phantom rows on sort, center gutter numbers, stop select-all viewport jump#4445

Merged
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/table-sort-bug
May 5, 2026
Merged

fix(tables): suppress phantom rows on sort, center gutter numbers, stop select-all viewport jump#4445
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/table-sort-bug

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • skip position-gap row rendering when a sort is active (was producing phantom empty rows and out-of-order numbering)
  • align gap-row gutter numbers to match data-row structure and center them in the gutter
  • suppress scroll-into-view on select-all so the viewport stays put (matches Google Sheets)

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@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 3:49am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 5, 2026

PR Summary

Medium Risk
Touches table selection/scroll behavior and gap-row rendering logic, which can cause subtle UX regressions (unexpected scrolling, incorrect selection highlighting, or missing placeholder rows) across keyboard and mouse interactions.

Overview
Prevents the table viewport from jumping on select-all (both header checkbox and Ctrl/Cmd+A) by suppressing the subsequent scrollIntoView side effect.

Stops rendering position gap placeholder rows when a sort is active (in addition to filters), avoiding phantom empty rows/out-of-order row numbering.

Adjusts gutter/checkbox cell layout so row numbers are centered and the per-row run/stop button stays aligned (including for gap rows).

Reviewed by Cursor Bugbot for commit c9c0c48. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

  • Phantom empty rows during sorted views are suppressed by extending the existing queryOptions.filter guard to also skip position-gap row rendering when queryOptions.sort is active.
  • Viewport jumps on select-all (handleSelectAllRows and Ctrl+A shortcut) are prevented via a suppressFocusScrollRef boolean ref that is read and reset inside the scroll useEffect, relying on React 18 batching to ensure the flag is still set when the effect fires.
  • Gap-row gutter cell HTML is restructured to match the DataRow gutter layout (centering the number/checkbox widget), and justify-between on the DataRow gutter is replaced with justify-center + ml-auto on the run button.

Confidence Score: 5/5

Safe to merge — all three fixes are narrowly scoped, use correct React patterns, and carry no functional regression risk.

No P0 or P1 issues found. The ref-based scroll-suppression pattern is correct: the flag is set synchronously before batched state updates, and the useEffect checks and clears it before any scroll is attempted. The sort-gap-row guard is a straightforward truthy extension of the existing filter guard. Gutter layout changes are purely visual.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx Three targeted bug fixes: suppress position-gap rows when sort is active, center gutter row numbers in both DataRow and PositionGapRows, and prevent viewport jump on select-all via a ref-based scroll suppression flag.

Sequence Diagram

sequenceDiagram
    participant User
    participant HandleSelectAll
    participant React State
    participant useEffect (scroll)

    User->>HandleSelectAll: click select-all / Ctrl+A
    HandleSelectAll->>HandleSelectAll: suppressFocusScrollRef.current = true
    HandleSelectAll->>React State: setSelectionAnchor({0,0})
    HandleSelectAll->>React State: setSelectionFocus({maxRow,maxCol})
    HandleSelectAll->>React State: setIsColumnSelection(false)
    Note over React State: React 18 batches all updates → single re-render
    React State-->>useEffect (scroll): fires with new selectionAnchor/Focus
    useEffect (scroll)->>useEffect (scroll): suppressFocusScrollRef.current == true?
    useEffect (scroll)->>useEffect (scroll): reset flag → false, return early
    Note over useEffect (scroll): scroll-into-view is skipped (viewport stays put)
Loading

Reviews (2): Last reviewed commit: "fix(tables): suppress scroll on Ctrl+A s..." | Re-trigger Greptile

Cmd/Ctrl+A duplicates the select-all logic but missed the
suppressFocusScrollRef flag, so the keyboard path still triggered
the viewport jump.

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

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c9c0c48. Configure here.

@waleedlatif1 waleedlatif1 merged commit e14a3a5 into staging May 5, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/table-sort-bug branch May 5, 2026 03:56
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