Skip to content

refactor(datagrid): make Cmd+C and Cmd+Shift+C robust when focus left the grid#1338

Merged
datlechin merged 1 commit into
mainfrom
refactor/copy-shortcut-robustness
May 19, 2026
Merged

refactor(datagrid): make Cmd+C and Cmd+Shift+C robust when focus left the grid#1338
datlechin merged 1 commit into
mainfrom
refactor/copy-shortcut-robustness

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

Follow-up to #1337. Two open items from the post-merge review of that PR:

  1. Responder-chain regression for .copyRows. After feat(datagrid): copy focused cell value with Cmd+C, add Cmd+Shift+C for rows (#1332) #1337, both Cmd+C (when router resolves to .copyRows) and Cmd+Shift+C dispatch the action via NSApp.sendAction(_:to:nil:from:). If the first responder isn't in a chain that reaches KeyHandlingTableView (e.g. focus moved to a different text field while a row selection persists in the grid), the action is silently lost. Before feat(datagrid): copy focused cell value with Cmd+C, add Cmd+Shift+C for rows (#1332) #1337, the menu button called actions?.copySelectedRows() directly, which didn't depend on responder chain.

  2. CreateTableView has the same selection-mirror problem TableStructureView had: @State selectedRows never feeds GridSelectionState.indices, so .disabled(!hasRowSelection) on the new "Copy Rows" menu item stays true and Cmd+Shift+C is blocked.

Changes

  • PasteboardCommands: both the "Copy" .copyRows branch and the "Copy Rows" button now fall back to actions?.copySelectedRows() when NSApp.sendAction returns false. Responder chain stays the preferred path; the SwiftUI-side action is the safety net.
  • CreateTableView: takes selectionState: GridSelectionState, mirrors selectedRows into it via .onChange(of:), and clears it .onDisappear. Same pattern TableStructureView already uses.
  • MainEditorContentView: passes selectionState into the CreateTableView call site.

Why this is the right shape

  • Native macOS pattern stays in place: menu items dispatch via the responder chain first. AppKit's validateUserInterfaceItem on KeyHandlingTableView continues to guard copy: and copyRowsAsTSV:.
  • The fallback is the SwiftUI action that the prior pre-feat(datagrid): copy focused cell value with Cmd+C, add Cmd+Shift+C for rows (#1332) #1337 codebase already trusted as the single dispatch path. Not a new abstraction.
  • For .copyCellValue, there's no fallback needed because cell-copy only fires when the user can see the focus border, which requires the table view to be the active emphasized responder, which means the responder chain will reach it.

Test plan

  • xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation
  • Select rows in data grid, click the SQL editor to move focus, press Cmd+Shift+C — clipboard contains the row TSV (was silently lost before this PR)
  • Same setup, press Cmd+C — clipboard contains the row TSV
  • Open Create Table tab, add columns, click rows in the columns grid, press Cmd+Shift+C — clipboard contains the structure-row payload
  • Original scenarios still work: single row + cell focus + Cmd+C copies the cell, multi-row select + Cmd+C copies TSV

@datlechin datlechin merged commit 449724c into main May 19, 2026
1 check passed
@datlechin datlechin deleted the refactor/copy-shortcut-robustness branch May 19, 2026 12:38
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