feat(datagrid): add visible add and remove buttons to the table structure editor (#1319)#1451
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 249d8b74a6
ℹ️ 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".
| private var shouldShowIndexesEmptyState: Bool { | ||
| loadedTabs.contains(.indexes) | ||
| && structureChangeManager.workingIndexes.isEmpty | ||
| && connection.type.supportsAddIndex | ||
| } |
There was a problem hiding this comment.
Gate empty add states on schema editing
For schema-read-only connections that still expose metadata capabilities (for example CockroachDB has supportsSchemaEditing == false while supportsForeignKeys == true, and index add support defaults to true), this condition replaces the read-only grid with an empty state containing an active Add button. Clicking it calls gridDelegate.dataGridAddRow(), whose index/FK paths do not check supportsSchemaEditing, so users can create unsavable structure changes even though the footer is correctly hidden and the grid is non-editable. Include connection.type.supportsSchemaEditing in these empty-state gates (and the FK one below) or render a non-action empty state for read-only connections.
Useful? React with 👍 / 👎.
249d8b7 to
b6e2698
Compare
b6e2698 to
b018a7c
Compare
…d footer state by owner (#1319)
Fixes #1319 — the table structure editor has no visible button to add a column, index, or foreign key. The reporter (on macOS 14) found only the Add Index entry by right-click and missed the Add Column one; the maintainer agreed to track this for UI/UX improvement.
Why it was undiscoverable
TableStructureView's toolbar was only a centered segmented sub-tab picker — no+anywhere. The only add affordances were the right-click empty-space menu (one entry per sub-tab) and theCmd+Shift+Nshortcut. TheEmptyStateView.indexes(onAdd:)/.foreignKeys(onAdd:)convenience initializers had been sitting unused, waiting for this work. Thedocs/features/table-structure.mdxpage already described "Open Structure → Columns, click +" — the docs were aspirational; this PR makes the app match what it says.The fix
Add / Remove controls live on the existing bottom status bar, in the trailing slot that already hosts Columns / Filters / Pagination in Data mode. Each mode owns that slot:
ControlGroupwith+and−, labels and capability adapt to the active sub-tab (Columns / Indexes / Foreign Keys).The
Data | Structure | JSONpicker stays anchored at the leading edge in all modes — it does not shift position when the mode changes.+routes togridDelegate.dataGridAddRow()(same path asCmd+Shift+N).−routes togridDelegate.dataGridDeleteRows(selectedRows)(same path as right-click Delete, and now wired toCmd+Delete). Tooltip and accessibility label per sub-tab: Add Column / Add Index / Add Foreign Key and the matching Remove labels.+is disabled when the sub-tab's capability flag is false;−is also disabled when no row is selected. Controls hide entirely when!connection.type.supportsSchemaEditingor on the read-only.ddl/.partssub-tabs.When the Indexes or Foreign Keys sub-tab is loaded and empty, the grid is replaced by
ContentUnavailableViewwith a labelled Add button (EmptyStateView.indexes(onAdd:)/.foreignKeys(onAdd:)).Architecture
StructureFooterState(new,@Observable) is the single source of truth for capability / label state.TableStructureViewpublishes to it on appear, sub-tab change, and selection change;MainStatusBarViewreads it to decide which buttons to render and when to enable them.StructureViewActionHandler.removeRowis added alongsideaddRow.MainContentCommandActions.deleteSelectedRowsroutes to it whenresultsViewMode == .structure, givingCmd+Deleteparity withCmd+Shift+N.MainStatusBarViewis now mode-aware: in Structure mode the data-only chrome (row info, Columns popover, Filters toggle, pagination) is hidden, so the status bar shows just the picker and+/−.Considered and rejected
Data | Structure | JSONrow; native apps consolidate into one footer.+/−next to the picker. Pushes the picker sideways when switching modes — the segmented control should not move.+/−next to the segmented picker. Native macOS reserves the top toolbar for window-level actions; bottom is the list-editor pattern.Tests
StructureGridDelegateAddRowTests(new) coversdataGridAddRow()per sub-tab (columns / indexes / FK route to the correct mutator; ddl / parts are no-ops; SQLite indexes is a no-op viasupportsAddIndex: false) anddataGridDeleteRows([0])safety on the read-only sub-tabs.StructureActionHandlerTestscoversaddRowandremoveRowclosure dispatch.CommandActionsDispatchTestsassertsaddNewRowanddeleteSelectedRowsroute tostructureActions?.addRow?()/structureActions?.removeRow?()whenresultsViewMode == .structure.MainStatusBarLayoutTestsupdated for the newstructureFooterState/onStructureAdd/onStructureRemoveparams.Out of scope
EmptyStateView.columns(onAdd:)and.checkConstraints(onAdd:)convenience initializers — this PR uses.indexesand.foreignKeys; the other two are dead but unrelated cleanup.