Skip to content

fix(tables): cmd+a always selects all cells on the table page#4562

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/tbl
May 12, 2026
Merged

fix(tables): cmd+a always selects all cells on the table page#4562
waleedlatif1 merged 2 commits into
stagingfrom
fix/tbl

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Cmd+A on the tables page now selects all cells regardless of where focus is (toolbar buttons, sidebar, page background)
  • Moved the select-all handler to a document-level listener so it fires even when focus is outside the grid container
  • Guards: skips when focus is in an input/textarea/select/contenteditable, inside an open dialog, or when the grid is in embedded mode

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 12, 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 12, 2026 1:11am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 12, 2026

PR Summary

Medium Risk
Adds a document-level Cmd/Ctrl+A keydown listener, which could affect global keyboard shortcut behavior despite added guards (inputs/contenteditable/dialogs/embedded). Otherwise change is localized to selection state updates in the table grid.

Overview
Fixes Select All behavior on the table page by moving the Cmd/Ctrl+A handler out of the grid container’s keydown logic and into a document-level listener.

The new handler selects the full cell range even when focus is outside the grid, while explicitly skipping events originating from form controls, contenteditable regions, dialogs, and when the grid is in embedded mode.

Reviewed by Cursor Bugbot for commit 2309ee7. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 12, 2026

Greptile Summary

This PR fixes Cmd+A on the tables page by promoting the select-all keyboard handler from a container-scoped listener to a document-level listener, so it fires even when focus sits on the toolbar, sidebar, or page background.

  • The old handler (inside the container's keydown effect) is removed and replaced with a new useEffect that attaches handleSelectAll to document. Guards skip the shortcut when focus is in a native input/textarea/select, a contentEditable element, or inside an open dialog, and the handler is skipped entirely in embedded mode. e.preventDefault() is now correctly called only when rows and columns are non-empty, preserving native Cmd+A when the grid is empty.
  • No functional changes to any other keyboard shortcuts or copy/paste handlers.

Confidence Score: 5/5

Safe to merge — a focused, well-guarded change that correctly widens the Cmd+A shortcut scope without breaking any existing keyboard behaviour.

The change is a straightforward relocation of an existing handler from a container element to the document, with additional guards (contentEditable, dialog) that are strictly more defensive than before. The previously reported bug (preventDefault on an empty grid) is confirmed fixed. No other shortcuts or state paths are touched.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx Moves the Cmd+A select-all handler from the container-scoped keydown listener to a document-level listener; adds guards for inputs, contentEditable, and dialogs; fixes the previously noted bug where preventDefault fired even on an empty grid.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User presses Cmd+A] --> B{embedded mode?}
    B -- Yes --> Z[Skip - no-op]
    B -- No --> C{target is INPUT / TEXTAREA / SELECT?}
    C -- Yes --> Z
    C -- No --> D{target.isContentEditable?}
    D -- Yes --> Z
    D -- No --> E{inside open dialog?}
    E -- Yes --> Z
    E -- No --> F{containerRef exists?}
    F -- No --> Z
    F -- Yes --> G{rows and cols non-empty?}
    G -- No --> Z2[Skip - grid empty, native Cmd+A preserved]
    G -- Yes --> H[Suppress default]
    H --> I[Clear editing cell and row selection]
    I --> J[Set anchor to row 0 col 0]
    J --> K[Set focus to last row and col]
    K --> L[All cells selected]
Loading

Reviews (2): Last reviewed commit: "fix(tables): move preventDefault inside ..." | Re-trigger Greptile

@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 2309ee7. Configure here.

@waleedlatif1 waleedlatif1 merged commit 22b5a1e into staging May 12, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/tbl branch May 12, 2026 01:25
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