Skip to content

fix(datagrid): Delete multi-select, filter panel redesign, Cmd+R refresh#1647

Closed
J2TeamNNL wants to merge 1 commit into
TableProApp:mainfrom
J2TeamNNL:fix/datagrid-2026-06-10
Closed

fix(datagrid): Delete multi-select, filter panel redesign, Cmd+R refresh#1647
J2TeamNNL wants to merge 1 commit into
TableProApp:mainfrom
J2TeamNNL:fix/datagrid-2026-06-10

Conversation

@J2TeamNNL

Copy link
Copy Markdown
Contributor

Summary

  • D12A — Delete key now reads gridSelection.affectedRows first before falling back to selectedRowIndexes, matching the existing copy() pattern. validateUserInterfaceItem and deleteSelectedRowsIfPossible updated accordingly.
  • D12BrightMouseDown override in KeyHandlingTableView: when the clicked row is already inside the selection, the override skips super (which would collapse the selection) and calls NSMenu.popUpContextMenu directly.
  • D13 — Filter panel redesign per owner spec: "Unset" renamed to "Clear" (calls clearAppliedFilters() — keeps filter rows, returns table to unfiltered); per-row Apply/Applied buttons removed; "Apply Only This Filter" moved to each row's context menu; tri-state checkbox added to panel header to toggle all rows enabled/disabled; "Remove All Filters" added to the ... options menu (calls clearFilterState()).
  • D14 — After currentQueryTask?.cancel(), immediately set currentQueryTask = nil and reset tab.execution.isExecuting = false so runQuery() is not blocked by the guard check.

Test plan

  • Select multiple rows with Shift/Cmd click, press Delete — all selected rows should be marked for deletion
  • Select cells across rows via drag (grid selection), press Delete — affected rows should be marked for deletion
  • Select multiple rows, right-click a row inside the selection — context menu appears without collapsing the selection
  • Filter panel: click "Clear" — filter rows remain, table reloads without filters applied
  • Filter panel ... menu: click "Remove All Filters" — all filter rows removed, table reloads without filters
  • Filter panel: right-click a row, choose "Apply Only This Filter" — only that row's filter is applied
  • Filter panel header checkbox: with mixed enabled state shows dash icon; clicking enables all; clicking again disables all
  • While a query is executing, press Cmd+R — query cancels and a fresh query starts immediately

…redesign, D14 refresh fix

- Delete key now reads gridSelection.affectedRows first, mirroring the copy() pattern
- rightMouseDown override preserves multi-row selection when right-clicking inside it
- Filter panel: "Unset" -> "Clear" (clearAppliedFilters, keeps rows); per-row Apply
  buttons removed; "Apply Only This Filter" added to row context menu; tri-state
  checkbox in header toggles all rows enabled/disabled; "Remove All Filters" added
  to options menu (clearFilterState)
- Refresh: cancel + nil currentQueryTask + reset isExecuting before runQuery so
  Cmd+R while a query runs always starts a fresh query

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2fa3e5703d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 47 to +49
currentQueryTask?.cancel()
currentQueryTask = nil
tabManager.mutate(at: tabIndex) { $0.execution.isExecuting = false }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid clearing the running task before restarting

When Cmd+R is pressed while a table query is still running, these lines discard the only currentQueryTask reference and mark the tab idle before runQuery() starts the replacement. The canceled task can still be inside queryExecutor.executeQuery; when it later observes cancellation, executeQueryInternal calls resetExecutionState(...), which unconditionally sets isExecuting = false and currentQueryTask = nil for the tab, thereby clearing the newly-started refresh task's UI/task state. Because the task reference is nil before executeQueryInternal runs, its existing cancelQuery() path is also skipped for this refresh case.

Useful? React with 👍 / 👎.

@J2TeamNNL

Copy link
Copy Markdown
Contributor Author

Splitting into separate PRs per fix (D12, D13, D14).

@J2TeamNNL J2TeamNNL closed this Jun 10, 2026
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