Skip to content

feat(inspector): insert columns left/right and delete selected columns in the CSV editor#1940

Merged
datlechin merged 2 commits into
mainfrom
feat/1469-csv-column-editing
Jul 22, 2026
Merged

feat(inspector): insert columns left/right and delete selected columns in the CSV editor#1940
datlechin merged 2 commits into
mainfrom
feat/1469-csv-column-editing

Conversation

@datlechin

Copy link
Copy Markdown
Member

Part 2 of 4 toward #1469 (CSV structural editing). Focused on columns. Stacked on #1939 (base branch feat/1469-csv-row-editing).

What

  • The column-header menu now reads Insert Column Left / Insert Column Right (renamed from the Before/After wording feat(inspector): add column header context menu to rename, insert, and delete columns #1932 shipped unreleased), matching the issue and spreadsheet conventions (Numbers, Sheets, Tablecruncher).
  • Multi-column delete: select several columns (click a header, Cmd-click others), and the menu reads Delete Columns, removing all of them in one undoable step. Deleting a data-bearing column (or columns) asks first.
  • Insert Column Left / Right and Delete Column also appear in the Edit menu, acting on the selected column.

Why

#1932 added single-column rename/insert/delete but always operated on the clicked column only, used Before/After labels, and never guarded a data-bearing delete. #1469 asks for Left/Right verbs that operate on the #1446 multi-column selection and confirm destructive deletes.

How (native / HIG)

  • The clicked column plus the current column selection resolve to a delete-target set inside the inspector, read from the shared grid's selectionController.selection.affectedColumns. When the clicked column is inside a multi-column selection, the verb operates on the whole selection; otherwise on the clicked column.
  • Multiple removeColumn calls are wrapped in one beginUndoGrouping/endUndoGrouping and deleted highest-index-first so lower indices stay valid; the group is named ("Undo Delete Columns").
  • Edit-menu items dispatch through the responder chain gated on keyWindowIsInspector, same as the row verbs in feat(inspector): insert rows and confirm data-bearing deletes in the CSV editor #1939.
  • Delete confirmation is scoped to columns that actually hold data (a short-circuiting scan), and the copy does not claim the action is irreversible, since column deletes are undoable.

Tests

  • InspectorColumnMenuBuilder: Left/Right action wiring, singular vs plural Delete label, and the target-columns payload.
  • InspectorDeleteConfirmation.columnDeleteTitle singular/plural.

Docs / changelog

  • docs/features/csv-inspector.mdx column-operations section updated.
  • CHANGELOG entry under [Unreleased].

Needs a local Xcode build to confirm compilation.

@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: 922879e34a

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +380 to +385
private func columnDeleteSelection(clicked: Int) -> [Int] {
guard let affected = gridDelegate.coordinator?.selectionController.selection.affectedColumns,
affected.contains(clicked), affected.count > 1 else {
return [clicked]
}
return affected.sorted()

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 Support additive header selection for multi-column deletion

The documented Cmd-click workflow cannot produce the multi-column selection this code expects: SortableHeaderView.mouseDown sends every Cmd-click to selectColumn(dataIndex), and that method replaces the grid selection with one entire column. Consequently, Cmd-clicking a second header clears the first selection, so the affected.count > 1 branch is unreachable through the advertised header interaction and the context menu never offers Delete Columns. Make header Cmd-click add/toggle the clicked column (or revise the interaction and documentation).

Useful? React with 👍 / 👎.

@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: 06c60c1031

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +159 to +162
forColumn: index,
currentType: currentType,
deleteColumns: [index]
)

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 Honor selected columns in the toolbar delete submenu

After Cmd-clicking multiple headers, opening the toolbar Columns menu and choosing Delete for one of those columns still deletes only that one: this explicit [index] payload takes precedence over the grid selection in columnDeleteTargets. The header context menu correctly supplies the multi-column target set, but the toolbar is documented as exposing the same submenu, so this route silently bypasses the new multi-column delete behavior.

Useful? React with 👍 / 👎.

@datlechin

Copy link
Copy Markdown
Member Author

Addressed a code review (including the codex bot note). Findings and fixes:

  • Safety: column verbs targeted selection.affectedColumns, which fires on any multi-column cell drag, so Edit > Delete Column could remove whole columns from an incidental selection (no confirm when empty). Column ops now use genuinely fully-selected columns via a new GridSelectionController.selectedFullColumns().
  • Documented multi-select now works: Cmd-click on a header was a replace, so accumulating columns was impossible. Cmd-click is now additive (addEntireColumn); single-select is unchanged.
  • Extracted the target resolution into a pure, tested InspectorColumnTargets helper, and corrected the docs.

Base automatically changed from feat/1469-csv-row-editing to main July 22, 2026 03:50
@mintlify

mintlify Bot commented Jul 22, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Jul 22, 2026, 3:53 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@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: 88c3e26c71

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

func addEntireColumn(_ column: Int, totalRows: Int) {
guard column >= 0, totalRows > 0 else { return }

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 Allow header selection when the grid has no rows

When an empty CSV is open (or the current filter produces no visible rows), Cmd-clicking headers cannot build a multi-column selection because this returns without updating selection. As a result the advertised Delete Columns path and the Edit-menu column actions cannot operate on selected columns in that state; header selections should be representable even when totalRows is zero.

Useful? React with 👍 / 👎.

@datlechin
datlechin merged commit 65e4ad7 into main Jul 22, 2026
3 checks passed
@datlechin
datlechin deleted the feat/1469-csv-column-editing branch July 22, 2026 03:54
@mintlify

mintlify Bot commented Jul 22, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🟡 Building Jul 22, 2026, 3:52 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

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