From b018a7c34dbdb83cf2c881e19d64309d00dd4e90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Thu, 28 May 2026 12:38:00 +0700 Subject: [PATCH 1/3] feat(datagrid): add visible add and remove buttons to the table structure editor (#1319) --- CHANGELOG.md | 1 + .../Views/Structure/TableStructureView.swift | 115 +++++++++++++++++- .../Main/CommandActionsDispatchTests.swift | 21 ++++ .../Main/StructureActionHandlerTests.swift | 18 ++- .../StructureGridDelegateAddRowTests.swift | 112 +++++++++++++++++ 5 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 898c83693..9da0e8005 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Oracle connections can now use a SID instead of a service name. Set Connection Type to SID in the connection form and enter the SID. (#1425) - Cmd-click a foreign key arrow to open the referenced table in a new tab instead of the current one. The right-click menu has the same Open in New Tab option. (#1421) - Cells holding JSON or PHP serialized values in text columns now open in the structured viewer automatically, without requiring the column type to be JSON. +- Add and remove buttons in the table structure editor for columns, indexes, and foreign keys, alongside the existing Cmd+Shift+N shortcut. An empty Indexes or Foreign Keys tab also shows a labelled add button. (#1319) ### Changed diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index 662b6af37..1025ce4aa 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -84,6 +84,7 @@ struct TableStructureView: View { toolbar Divider() contentArea + structureFooter } .task(loadInitialData) .onChange(of: selectedRows) { _, newRows in selectionState.indices = newRows } @@ -171,6 +172,94 @@ struct TableStructureView: View { .padding() } + // MARK: - Footer (Add / Remove) + + @ViewBuilder + private var structureFooter: some View { + if isEditableTab { + VStack(spacing: 0) { + Divider() + HStack(spacing: 0) { + addButton + removeButton + Spacer() + } + .padding(.horizontal, 8) + .frame(height: 28) + .background(.bar) + } + } + } + + private var addButton: some View { + Button(action: { gridDelegate.dataGridAddRow() }) { + Label(addLabel(for: selectedTab), systemImage: "plus") + .labelStyle(.iconOnly) + .frame(width: 22, height: 22) + } + .buttonStyle(.borderless) + .help(addLabel(for: selectedTab)) + .disabled(!canAdd(for: selectedTab)) + } + + private var removeButton: some View { + Button(action: { gridDelegate.dataGridDeleteRows(selectedRows) }) { + Label(removeLabel(for: selectedTab), systemImage: "minus") + .labelStyle(.iconOnly) + .frame(width: 22, height: 22) + } + .buttonStyle(.borderless) + .help(removeLabel(for: selectedTab)) + .disabled(!canRemove(for: selectedTab)) + } + + private var isEditableTab: Bool { + guard connection.type.supportsSchemaEditing else { return false } + switch selectedTab { + case .columns, .indexes, .foreignKeys: + return true + case .ddl, .parts: + return false + } + } + + private func canAdd(for tab: StructureTab) -> Bool { + switch tab { + case .columns: return connection.type.supportsAddColumn + case .indexes: return connection.type.supportsAddIndex + case .foreignKeys: return connection.type.supportsForeignKeys + case .ddl, .parts: return false + } + } + + private func canRemove(for tab: StructureTab) -> Bool { + guard !selectedRows.isEmpty else { return false } + switch tab { + case .columns: return connection.type.supportsDropColumn + case .indexes: return connection.type.supportsDropIndex + case .foreignKeys: return connection.type.supportsForeignKeys + case .ddl, .parts: return false + } + } + + private func addLabel(for tab: StructureTab) -> String { + switch tab { + case .columns: return String(localized: "Add Column") + case .indexes: return String(localized: "Add Index") + case .foreignKeys: return String(localized: "Add Foreign Key") + case .ddl, .parts: return String(localized: "Add Row") + } + } + + private func removeLabel(for tab: StructureTab) -> String { + switch tab { + case .columns: return String(localized: "Remove Column") + case .indexes: return String(localized: "Remove Index") + case .foreignKeys: return String(localized: "Remove Foreign Key") + case .ddl, .parts: return String(localized: "Remove Row") + } + } + // MARK: - Tab Label with Count Badge private func tabLabel(for tab: StructureTab) -> String { @@ -206,8 +295,20 @@ struct TableStructureView: View { @ViewBuilder private var tabContent: some View { switch selectedTab { - case .columns, .indexes, .foreignKeys: + case .columns: structureGrid + case .indexes: + if shouldShowIndexesEmptyState { + EmptyStateView.indexes { gridDelegate.dataGridAddRow() } + } else { + structureGrid + } + case .foreignKeys: + if shouldShowForeignKeysEmptyState { + EmptyStateView.foreignKeys { gridDelegate.dataGridAddRow() } + } else { + structureGrid + } case .ddl: ddlView case .parts: @@ -215,6 +316,18 @@ struct TableStructureView: View { } } + private var shouldShowIndexesEmptyState: Bool { + loadedTabs.contains(.indexes) + && structureChangeManager.workingIndexes.isEmpty + && connection.type.supportsAddIndex + } + + private var shouldShowForeignKeysEmptyState: Bool { + loadedTabs.contains(.foreignKeys) + && structureChangeManager.workingForeignKeys.isEmpty + && connection.type.supportsForeignKeys + } + // MARK: - Structure Grid (DataGridView) private func makeCurrentProvider() -> StructureRowProvider { diff --git a/TableProTests/Views/Main/CommandActionsDispatchTests.swift b/TableProTests/Views/Main/CommandActionsDispatchTests.swift index 29ebe2e13..d5be0ff07 100644 --- a/TableProTests/Views/Main/CommandActionsDispatchTests.swift +++ b/TableProTests/Views/Main/CommandActionsDispatchTests.swift @@ -131,4 +131,25 @@ struct CommandActionsDispatchTests { #expect(pasteRowsCalled) } + + // MARK: - addNewRow (structure mode) + + @Test("addNewRow in structure mode calls structureActions.addRow") + func addNewRow_structureMode_callsStructureActions() { + let (actions, coordinator) = makeSUT() + coordinator.tabManager.addTab(databaseName: "testdb") + + if let idx = coordinator.tabManager.selectedTabIndex { + coordinator.tabManager.tabs[idx].display.resultsViewMode = .structure + } + + let handler = StructureViewActionHandler() + var addRowCalled = false + handler.addRow = { addRowCalled = true } + coordinator.structureActions = handler + + actions.addNewRow() + + #expect(addRowCalled) + } } diff --git a/TableProTests/Views/Main/StructureActionHandlerTests.swift b/TableProTests/Views/Main/StructureActionHandlerTests.swift index 9a6e0870b..cc867c0cb 100644 --- a/TableProTests/Views/Main/StructureActionHandlerTests.swift +++ b/TableProTests/Views/Main/StructureActionHandlerTests.swift @@ -88,9 +88,20 @@ struct StructureActionHandlerTests { #expect(count == 1) } - // MARK: - All Six Closures Fire Independently + @Test("addRow closure fires when invoked") + func addRow_fires() { + let handler = StructureViewActionHandler() + var count = 0 + handler.addRow = { count += 1 } + + handler.addRow?() + + #expect(count == 1) + } + + // MARK: - All Closures Fire Independently - @Test("all six closures fire independently without cross-talk") + @Test("all closures fire independently without cross-talk") func allClosures_fireIndependently() { let handler = StructureViewActionHandler() var counts = [String: Int]() @@ -101,6 +112,7 @@ struct StructureActionHandlerTests { handler.pasteRows = { counts["pasteRows", default: 0] += 1 } handler.undo = { counts["undo", default: 0] += 1 } handler.redo = { counts["redo", default: 0] += 1 } + handler.addRow = { counts["addRow", default: 0] += 1 } handler.saveChanges?() handler.previewSQL?() @@ -108,6 +120,7 @@ struct StructureActionHandlerTests { handler.pasteRows?() handler.undo?() handler.redo?() + handler.addRow?() #expect(counts["saveChanges"] == 1) #expect(counts["previewSQL"] == 1) @@ -115,6 +128,7 @@ struct StructureActionHandlerTests { #expect(counts["pasteRows"] == 1) #expect(counts["undo"] == 1) #expect(counts["redo"] == 1) + #expect(counts["addRow"] == 1) } // MARK: - Nil Closures Are Safe diff --git a/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift b/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift new file mode 100644 index 000000000..690d8ba4c --- /dev/null +++ b/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift @@ -0,0 +1,112 @@ +// +// StructureGridDelegateAddRowTests.swift +// TableProTests +// +// Tests for StructureGridDelegate.dataGridAddRow() / dataGridDeleteRows(_:) +// routing per active sub-tab. These cover the contract the new structure +// footer +/- buttons depend on. +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@MainActor @Suite("StructureGridDelegate add and delete row routing") +struct StructureGridDelegateAddRowTests { + private func makeDelegate( + selectedTab: StructureTab = .columns, + type: DatabaseType = .mysql + ) -> (StructureGridDelegate, StructureChangeManager) { + let manager = StructureChangeManager() + let connection = TestFixtures.makeConnection(type: type) + let delegate = StructureGridDelegate( + structureChangeManager: manager, + selectedTab: selectedTab, + connection: connection, + tableName: "t", + coordinator: nil + ) + return (delegate, manager) + } + + @Test("Columns sub-tab: dataGridAddRow appends a placeholder column") + func columnsTab_addsColumn() { + let (delegate, manager) = makeDelegate(selectedTab: .columns) + let before = manager.workingColumns.count + delegate.dataGridAddRow() + #expect(manager.workingColumns.count == before + 1) + } + + @Test("Indexes sub-tab: dataGridAddRow appends a placeholder index") + func indexesTab_addsIndex() { + let (delegate, manager) = makeDelegate(selectedTab: .indexes) + let before = manager.workingIndexes.count + delegate.dataGridAddRow() + #expect(manager.workingIndexes.count == before + 1) + } + + @Test("Foreign keys sub-tab: dataGridAddRow appends a placeholder foreign key") + func foreignKeysTab_addsForeignKey() { + let (delegate, manager) = makeDelegate(selectedTab: .foreignKeys) + let before = manager.workingForeignKeys.count + delegate.dataGridAddRow() + #expect(manager.workingForeignKeys.count == before + 1) + } + + @Test("DDL sub-tab: dataGridAddRow is a no-op") + func ddlTab_isNoOp() { + let (delegate, manager) = makeDelegate(selectedTab: .ddl) + let columnsBefore = manager.workingColumns.count + let indexesBefore = manager.workingIndexes.count + let fksBefore = manager.workingForeignKeys.count + delegate.dataGridAddRow() + #expect(manager.workingColumns.count == columnsBefore) + #expect(manager.workingIndexes.count == indexesBefore) + #expect(manager.workingForeignKeys.count == fksBefore) + } + + @Test("Parts sub-tab: dataGridAddRow is a no-op") + func partsTab_isNoOp() { + let (delegate, manager) = makeDelegate(selectedTab: .parts) + let columnsBefore = manager.workingColumns.count + let indexesBefore = manager.workingIndexes.count + let fksBefore = manager.workingForeignKeys.count + delegate.dataGridAddRow() + #expect(manager.workingColumns.count == columnsBefore) + #expect(manager.workingIndexes.count == indexesBefore) + #expect(manager.workingForeignKeys.count == fksBefore) + } + + @Test("Indexes sub-tab on SQLite: dataGridAddRow is a no-op (supportsAddIndex == false)") + func sqliteIndexes_isNoOp() { + let (delegate, manager) = makeDelegate(selectedTab: .indexes, type: .sqlite) + let before = manager.workingIndexes.count + delegate.dataGridAddRow() + #expect(manager.workingIndexes.count == before) + } + + @Test("Delete: ddl sub-tab is a no-op") + func ddlTab_deleteIsNoOp() { + let (delegate, manager) = makeDelegate(selectedTab: .ddl) + let columnsBefore = manager.workingColumns.count + let indexesBefore = manager.workingIndexes.count + let fksBefore = manager.workingForeignKeys.count + delegate.dataGridDeleteRows([0]) + #expect(manager.workingColumns.count == columnsBefore) + #expect(manager.workingIndexes.count == indexesBefore) + #expect(manager.workingForeignKeys.count == fksBefore) + } + + @Test("Delete: parts sub-tab is a no-op") + func partsTab_deleteIsNoOp() { + let (delegate, manager) = makeDelegate(selectedTab: .parts) + let columnsBefore = manager.workingColumns.count + let indexesBefore = manager.workingIndexes.count + let fksBefore = manager.workingForeignKeys.count + delegate.dataGridDeleteRows([0]) + #expect(manager.workingColumns.count == columnsBefore) + #expect(manager.workingIndexes.count == indexesBefore) + #expect(manager.workingForeignKeys.count == fksBefore) + } +} From e5fd4f5c278ac03c43d8205177275d95368c5c4a Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 28 May 2026 17:55:22 +0700 Subject: [PATCH 2/3] refactor(datagrid): move structure add and remove into the status bar trailing slot (#1319) --- CHANGELOG.md | 2 +- TablePro/Resources/Localizable.xcstrings | 9 + .../Main/Child/MainEditorContentView.swift | 5 +- .../Views/Main/Child/MainStatusBarView.swift | 156 +++++++++++------- .../Main/MainContentCommandActions.swift | 5 + .../Views/Main/MainContentCoordinator.swift | 4 + .../Structure/StructureFooterState.swift | 25 +++ .../StructureViewActionHandler.swift | 6 +- .../Views/Structure/TableStructureView.swift | 96 ++++------- .../Main/CommandActionsDispatchTests.swift | 25 ++- .../Views/Main/MainStatusBarLayoutTests.swift | 5 +- .../Main/StructureActionHandlerTests.swift | 16 +- 12 files changed, 221 insertions(+), 133 deletions(-) create mode 100644 TablePro/Views/Structure/StructureFooterState.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9da0e8005..994caf3ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Oracle connections can now use a SID instead of a service name. Set Connection Type to SID in the connection form and enter the SID. (#1425) - Cmd-click a foreign key arrow to open the referenced table in a new tab instead of the current one. The right-click menu has the same Open in New Tab option. (#1421) - Cells holding JSON or PHP serialized values in text columns now open in the structured viewer automatically, without requiring the column type to be JSON. -- Add and remove buttons in the table structure editor for columns, indexes, and foreign keys, alongside the existing Cmd+Shift+N shortcut. An empty Indexes or Foreign Keys tab also shows a labelled add button. (#1319) +- Add and remove buttons in the table structure editor for columns, indexes, and foreign keys, on the bottom status bar alongside the view-mode picker. Cmd+Shift+N adds and Cmd+Delete removes. An empty Indexes or Foreign Keys tab also shows a labelled add button. (#1319) ### Changed diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index d9f25edc0..b3deed808 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -38342,6 +38342,9 @@ }, "Remove attachment" : { + }, + "Remove Column" : { + }, "Remove Failed" : { @@ -38455,6 +38458,9 @@ } } } + }, + "Remove Foreign Key" : { + }, "Remove from Group" : { "localizations" : { @@ -38483,6 +38489,9 @@ }, "Remove image" : { + }, + "Remove Index" : { + }, "Remove jump host" : { "localizations" : { diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 0d4ef29a6..dcd208707 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -759,7 +759,10 @@ struct MainEditorContentView: View { onShowAllColumns: { coordinator.showAllColumns() }, onHideAllColumns: { coordinator.hideAllColumns($0) }, onToggleFilters: { coordinator.toggleFilterPanel() }, - onFetchAll: { coordinator.fetchAllRows() } + onFetchAll: { coordinator.fetchAllRows() }, + structureFooterState: coordinator.structureFooterState, + onStructureAdd: { coordinator.structureActions?.addRow?() }, + onStructureRemove: { coordinator.structureActions?.removeRow?() } ) } diff --git a/TablePro/Views/Main/Child/MainStatusBarView.swift b/TablePro/Views/Main/Child/MainStatusBarView.swift index ce99b1e40..1012e2315 100644 --- a/TablePro/Views/Main/Child/MainStatusBarView.swift +++ b/TablePro/Views/Main/Child/MainStatusBarView.swift @@ -59,9 +59,17 @@ struct MainStatusBarView: View { // Truncated result callback var onFetchAll: (() -> Void)? + // Structure-mode footer accessory (Add/Remove buttons) + let structureFooterState: StructureFooterState? + let onStructureAdd: (() -> Void)? + let onStructureRemove: (() -> Void)? + private var hasHiddenColumns: Bool { !hiddenColumns.isEmpty } private var hiddenCount: Int { hiddenColumns.count } + private var isStructureMode: Bool { viewMode == .structure } + private var showsDataChrome: Bool { !isStructureMode } + var body: some View { HStack { if snapshot.tabId != nil { @@ -89,8 +97,7 @@ struct MainStatusBarView: View { Spacer() - // Center: Row info (selection or pagination summary) and status message - if snapshot.hasRows { + if showsDataChrome, snapshot.hasRows { HStack(spacing: 4) { if snapshot.pagination.isLoadingMore { ProgressView() @@ -132,71 +139,74 @@ struct MainStatusBarView: View { Spacer() - // Right: Columns, Filters toggle and Pagination controls HStack(spacing: 8) { - // Columns visibility button (works for both table and query tabs) - if snapshot.hasColumns { - Button { - showColumnPopover.toggle() - } label: { - HStack(spacing: 4) { - Image(systemName: hasHiddenColumns - ? "eye.slash.circle.fill" - : "eye.circle") - Text("Columns") - if hasHiddenColumns { - let visible = allColumns.count - hiddenCount - Text("(\(visible)/\(allColumns.count))") - .foregroundStyle(.secondary) + if isStructureMode, let state = structureFooterState, state.isActive { + structureFooterControls(state: state) + } + + if showsDataChrome { + if snapshot.hasColumns { + Button { + showColumnPopover.toggle() + } label: { + HStack(spacing: 4) { + Image(systemName: hasHiddenColumns + ? "eye.slash.circle.fill" + : "eye.circle") + Text("Columns") + if hasHiddenColumns { + let visible = allColumns.count - hiddenCount + Text("(\(visible)/\(allColumns.count))") + .foregroundStyle(.secondary) + } } } + .controlSize(.small) + .popover(isPresented: $showColumnPopover) { + ColumnVisibilityPopover( + columns: allColumns, + hiddenColumns: hiddenColumns, + onToggleColumn: onToggleColumn, + onShowAll: onShowAllColumns, + onHideAll: onHideAllColumns + ) + } } - .controlSize(.small) - .popover(isPresented: $showColumnPopover) { - ColumnVisibilityPopover( - columns: allColumns, - hiddenColumns: hiddenColumns, - onToggleColumn: onToggleColumn, - onShowAll: onShowAllColumns, - onHideAll: onHideAllColumns - ) - } - } - // Filters toggle button - if snapshot.tabType == .table, snapshot.hasTableName { - Toggle(isOn: Binding( - get: { filterState.isVisible }, - set: { _ in onToggleFilters() } - )) { - HStack(spacing: 4) { - Image(systemName: filterState.hasAppliedFilters - ? "line.3.horizontal.decrease.circle.fill" - : "line.3.horizontal.decrease.circle") - Text("Filters") - if filterState.hasAppliedFilters { - Text("(\(filterState.appliedFilters.count))") - .foregroundStyle(.secondary) + if snapshot.tabType == .table, snapshot.hasTableName { + Toggle(isOn: Binding( + get: { filterState.isVisible }, + set: { _ in onToggleFilters() } + )) { + HStack(spacing: 4) { + Image(systemName: filterState.hasAppliedFilters + ? "line.3.horizontal.decrease.circle.fill" + : "line.3.horizontal.decrease.circle") + Text("Filters") + if filterState.hasAppliedFilters { + Text("(\(filterState.appliedFilters.count))") + .foregroundStyle(.secondary) + } } } + .toggleStyle(.button) + .controlSize(.small) + .help(String(localized: "Toggle Filters (⇧⌘F)")) } - .toggleStyle(.button) - .controlSize(.small) - .help(String(localized: "Toggle Filters (⇧⌘F)")) - } - if snapshot.tabType == .table, snapshot.hasTableName, showsPaginationControls { - PaginationControlsView( - pagination: snapshot.pagination, - loadedRowCount: snapshot.rowCount, - onFirst: onFirstPage, - onPrevious: onPreviousPage, - onNext: onNextPage, - onLast: onLastPage, - onPageSizeChange: onPageSizeChange, - onShowAll: onShowAll, - onGoToPage: onGoToPage - ) + if snapshot.tabType == .table, snapshot.hasTableName, showsPaginationControls { + PaginationControlsView( + pagination: snapshot.pagination, + loadedRowCount: snapshot.rowCount, + onFirst: onFirstPage, + onPrevious: onPreviousPage, + onNext: onNextPage, + onLast: onLastPage, + onPageSizeChange: onPageSizeChange, + onShowAll: onShowAll, + onGoToPage: onGoToPage + ) + } } } } @@ -208,6 +218,36 @@ struct MainStatusBarView: View { } } + @ViewBuilder + private func structureFooterControls(state: StructureFooterState) -> some View { + ControlGroup { + Button { + onStructureAdd?() + } label: { + Label(state.addLabel, systemImage: "plus") + .labelStyle(.iconOnly) + } + .help(state.addLabel) + .accessibilityLabel(state.addLabel) + .disabled(!state.canAdd) + .keyboardShortcut("n", modifiers: [.command, .shift]) + + Button { + onStructureRemove?() + } label: { + Label(state.removeLabel, systemImage: "minus") + .labelStyle(.iconOnly) + } + .help(state.removeLabel) + .accessibilityLabel(state.removeLabel) + .disabled(!state.canRemove) + .keyboardShortcut(.delete, modifiers: .command) + } + .controlGroupStyle(.navigation) + .controlSize(.small) + .frame(width: 64) + } + private var showsPaginationControls: Bool { if let total = snapshot.pagination.totalRowCount, total > 0 { return true } return isPagedWithUnknownTotal diff --git a/TablePro/Views/Main/MainContentCommandActions.swift b/TablePro/Views/Main/MainContentCommandActions.swift index 2e4a65feb..575450b59 100644 --- a/TablePro/Views/Main/MainContentCommandActions.swift +++ b/TablePro/Views/Main/MainContentCommandActions.swift @@ -179,6 +179,11 @@ final class MainContentCommandActions { func deleteSelectedRows(rowIndices: Set? = nil) { let fromDataGrid = rowIndices != nil + if coordinator?.tabManager.selectedTab?.display.resultsViewMode == .structure { + coordinator?.structureActions?.removeRow?() + return + } + let indices = rowIndices ?? selectionState.indices if !indices.isEmpty { coordinator?.deleteSelectedRows(indices: indices) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index bc430815e..8e66e8e7b 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -126,6 +126,10 @@ final class MainContentCoordinator { /// Direct reference to structure view actions — eliminates notification broadcasts weak var structureActions: StructureViewActionHandler? + /// Published capability/labels for the structure-mode footer in the bottom status bar. + /// `TableStructureView` writes to this; `MainStatusBarView` reads from it. + let structureFooterState = StructureFooterState() + /// Direct reference to AI chat viewmodel — eliminates notification broadcasts weak var aiViewModel: AIChatViewModel? diff --git a/TablePro/Views/Structure/StructureFooterState.swift b/TablePro/Views/Structure/StructureFooterState.swift new file mode 100644 index 000000000..4e0a8524c --- /dev/null +++ b/TablePro/Views/Structure/StructureFooterState.swift @@ -0,0 +1,25 @@ +// +// StructureFooterState.swift +// TablePro +// + +import Foundation +import Observation + +@Observable +@MainActor +final class StructureFooterState { + var isActive: Bool = false + var canAdd: Bool = false + var canRemove: Bool = false + var addLabel: String = "" + var removeLabel: String = "" + + func deactivate() { + isActive = false + canAdd = false + canRemove = false + addLabel = "" + removeLabel = "" + } +} diff --git a/TablePro/Views/Structure/StructureViewActionHandler.swift b/TablePro/Views/Structure/StructureViewActionHandler.swift index dce43383c..f29936029 100644 --- a/TablePro/Views/Structure/StructureViewActionHandler.swift +++ b/TablePro/Views/Structure/StructureViewActionHandler.swift @@ -2,14 +2,9 @@ // StructureViewActionHandler.swift // TablePro // -// Action handler for structure view — allows coordinator to call -// structure-view actions directly instead of broadcasting notifications. -// import Foundation -/// Provides direct action dispatch from coordinator to structure view, -/// replacing notification-based communication. @MainActor final class StructureViewActionHandler { var saveChanges: (() -> Void)? @@ -19,4 +14,5 @@ final class StructureViewActionHandler { var undo: (() -> Void)? var redo: (() -> Void)? var addRow: (() -> Void)? + var removeRow: (() -> Void)? } diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index 1025ce4aa..058d3f971 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -84,11 +84,16 @@ struct TableStructureView: View { toolbar Divider() contentArea - structureFooter } .task(loadInitialData) - .onChange(of: selectedRows) { _, newRows in selectionState.indices = newRows } - .onChange(of: selectedTab) { _, newValue in onSelectedTabChanged(newValue) } + .onChange(of: selectedRows) { _, newRows in + selectionState.indices = newRows + publishFooterState() + } + .onChange(of: selectedTab) { _, newValue in + onSelectedTabChanged(newValue) + publishFooterState() + } .onChange(of: columns) { onColumnsChanged() } .onChange(of: indexes) { onIndexesChanged() } .onChange(of: foreignKeys) { onForeignKeysChanged() } @@ -119,11 +124,14 @@ struct TableStructureView: View { actionHandler.undo = { self.gridDelegate.dataGridUndo() } actionHandler.redo = { self.gridDelegate.dataGridRedo() } actionHandler.addRow = { self.gridDelegate.dataGridAddRow() } + actionHandler.removeRow = { self.gridDelegate.dataGridDeleteRows(self.selectedRows) } coordinator?.structureActions = actionHandler + publishFooterState() } .onDisappear { coordinator?.toolbarState.hasStructureChanges = false coordinator?.structureActions = nil + coordinator?.structureFooterState.deactivate() selectionState.indices = [] } .onChange(of: structureChangeManager.hasChanges) { _, newValue in @@ -172,55 +180,20 @@ struct TableStructureView: View { .padding() } - // MARK: - Footer (Add / Remove) - - @ViewBuilder - private var structureFooter: some View { - if isEditableTab { - VStack(spacing: 0) { - Divider() - HStack(spacing: 0) { - addButton - removeButton - Spacer() - } - .padding(.horizontal, 8) - .frame(height: 28) - .background(.bar) - } - } - } - - private var addButton: some View { - Button(action: { gridDelegate.dataGridAddRow() }) { - Label(addLabel(for: selectedTab), systemImage: "plus") - .labelStyle(.iconOnly) - .frame(width: 22, height: 22) - } - .buttonStyle(.borderless) - .help(addLabel(for: selectedTab)) - .disabled(!canAdd(for: selectedTab)) - } - - private var removeButton: some View { - Button(action: { gridDelegate.dataGridDeleteRows(selectedRows) }) { - Label(removeLabel(for: selectedTab), systemImage: "minus") - .labelStyle(.iconOnly) - .frame(width: 22, height: 22) - } - .buttonStyle(.borderless) - .help(removeLabel(for: selectedTab)) - .disabled(!canRemove(for: selectedTab)) - } + // MARK: - Footer state (rendered by MainStatusBarView) - private var isEditableTab: Bool { - guard connection.type.supportsSchemaEditing else { return false } - switch selectedTab { - case .columns, .indexes, .foreignKeys: - return true - case .ddl, .parts: - return false + private func publishFooterState() { + guard let footer = coordinator?.structureFooterState else { return } + guard connection.type.supportsSchemaEditing, + let labels = footerLabels(for: selectedTab) else { + footer.deactivate() + return } + footer.isActive = true + footer.addLabel = labels.add + footer.removeLabel = labels.remove + footer.canAdd = canAdd(for: selectedTab) + footer.canRemove = canRemove(for: selectedTab) } private func canAdd(for tab: StructureTab) -> Bool { @@ -242,21 +215,16 @@ struct TableStructureView: View { } } - private func addLabel(for tab: StructureTab) -> String { + private func footerLabels(for tab: StructureTab) -> (add: String, remove: String)? { switch tab { - case .columns: return String(localized: "Add Column") - case .indexes: return String(localized: "Add Index") - case .foreignKeys: return String(localized: "Add Foreign Key") - case .ddl, .parts: return String(localized: "Add Row") - } - } - - private func removeLabel(for tab: StructureTab) -> String { - switch tab { - case .columns: return String(localized: "Remove Column") - case .indexes: return String(localized: "Remove Index") - case .foreignKeys: return String(localized: "Remove Foreign Key") - case .ddl, .parts: return String(localized: "Remove Row") + case .columns: + return (String(localized: "Add Column"), String(localized: "Remove Column")) + case .indexes: + return (String(localized: "Add Index"), String(localized: "Remove Index")) + case .foreignKeys: + return (String(localized: "Add Foreign Key"), String(localized: "Remove Foreign Key")) + case .ddl, .parts: + return nil } } diff --git a/TableProTests/Views/Main/CommandActionsDispatchTests.swift b/TableProTests/Views/Main/CommandActionsDispatchTests.swift index d5be0ff07..a69f959ff 100644 --- a/TableProTests/Views/Main/CommandActionsDispatchTests.swift +++ b/TableProTests/Views/Main/CommandActionsDispatchTests.swift @@ -7,10 +7,10 @@ // import Foundation -import TableProPluginKit import SwiftUI -import Testing @testable import TablePro +import TableProPluginKit +import Testing @MainActor @Suite("CommandActions Dispatch") struct CommandActionsDispatchTests { @@ -152,4 +152,25 @@ struct CommandActionsDispatchTests { #expect(addRowCalled) } + + // MARK: - deleteSelectedRows (structure mode) + + @Test("deleteSelectedRows in structure mode calls structureActions.removeRow") + func deleteSelectedRows_structureMode_callsStructureActions() { + let (actions, coordinator) = makeSUT() + coordinator.tabManager.addTab(databaseName: "testdb") + + if let idx = coordinator.tabManager.selectedTabIndex { + coordinator.tabManager.tabs[idx].display.resultsViewMode = .structure + } + + let handler = StructureViewActionHandler() + var removeRowCalled = false + handler.removeRow = { removeRowCalled = true } + coordinator.structureActions = handler + + actions.deleteSelectedRows() + + #expect(removeRowCalled) + } } diff --git a/TableProTests/Views/Main/MainStatusBarLayoutTests.swift b/TableProTests/Views/Main/MainStatusBarLayoutTests.swift index 6633d85f2..311e1362b 100644 --- a/TableProTests/Views/Main/MainStatusBarLayoutTests.swift +++ b/TableProTests/Views/Main/MainStatusBarLayoutTests.swift @@ -32,7 +32,10 @@ struct MainStatusBarLayoutTests { onToggleColumn: { _ in }, onShowAllColumns: {}, onHideAllColumns: { _ in }, - onToggleFilters: {} + onToggleFilters: {}, + structureFooterState: nil, + onStructureAdd: nil, + onStructureRemove: nil ) #expect(type(of: view.body) != Never.self) } diff --git a/TableProTests/Views/Main/StructureActionHandlerTests.swift b/TableProTests/Views/Main/StructureActionHandlerTests.swift index cc867c0cb..42f403263 100644 --- a/TableProTests/Views/Main/StructureActionHandlerTests.swift +++ b/TableProTests/Views/Main/StructureActionHandlerTests.swift @@ -6,9 +6,9 @@ // import Foundation +@testable import TablePro import TableProPluginKit import Testing -@testable import TablePro @MainActor @Suite("StructureViewActionHandler") struct StructureActionHandlerTests { @@ -99,6 +99,17 @@ struct StructureActionHandlerTests { #expect(count == 1) } + @Test("removeRow closure fires when invoked") + func removeRow_fires() { + let handler = StructureViewActionHandler() + var count = 0 + handler.removeRow = { count += 1 } + + handler.removeRow?() + + #expect(count == 1) + } + // MARK: - All Closures Fire Independently @Test("all closures fire independently without cross-talk") @@ -113,6 +124,7 @@ struct StructureActionHandlerTests { handler.undo = { counts["undo", default: 0] += 1 } handler.redo = { counts["redo", default: 0] += 1 } handler.addRow = { counts["addRow", default: 0] += 1 } + handler.removeRow = { counts["removeRow", default: 0] += 1 } handler.saveChanges?() handler.previewSQL?() @@ -121,6 +133,7 @@ struct StructureActionHandlerTests { handler.undo?() handler.redo?() handler.addRow?() + handler.removeRow?() #expect(counts["saveChanges"] == 1) #expect(counts["previewSQL"] == 1) @@ -129,6 +142,7 @@ struct StructureActionHandlerTests { #expect(counts["undo"] == 1) #expect(counts["redo"] == 1) #expect(counts["addRow"] == 1) + #expect(counts["removeRow"] == 1) } // MARK: - Nil Closures Are Safe From b70363155524afa3389f1820687a569b69d2c9ec Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 28 May 2026 18:03:21 +0700 Subject: [PATCH 3/3] refactor(datagrid): defer structure shortcuts to menu wiring and guard footer state by owner (#1319) --- .../Views/Main/Child/MainStatusBarView.swift | 4 +- .../Structure/StructureFooterState.swift | 21 +++++++++- .../Views/Structure/TableStructureView.swift | 17 ++++---- .../StructureGridDelegateAddRowTests.swift | 39 +++++++++++++++++++ 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/TablePro/Views/Main/Child/MainStatusBarView.swift b/TablePro/Views/Main/Child/MainStatusBarView.swift index 1012e2315..85a86b14b 100644 --- a/TablePro/Views/Main/Child/MainStatusBarView.swift +++ b/TablePro/Views/Main/Child/MainStatusBarView.swift @@ -230,7 +230,6 @@ struct MainStatusBarView: View { .help(state.addLabel) .accessibilityLabel(state.addLabel) .disabled(!state.canAdd) - .keyboardShortcut("n", modifiers: [.command, .shift]) Button { onStructureRemove?() @@ -241,11 +240,10 @@ struct MainStatusBarView: View { .help(state.removeLabel) .accessibilityLabel(state.removeLabel) .disabled(!state.canRemove) - .keyboardShortcut(.delete, modifiers: .command) } .controlGroupStyle(.navigation) .controlSize(.small) - .frame(width: 64) + .fixedSize() } private var showsPaginationControls: Bool { diff --git a/TablePro/Views/Structure/StructureFooterState.swift b/TablePro/Views/Structure/StructureFooterState.swift index 4e0a8524c..1ad6d82e1 100644 --- a/TablePro/Views/Structure/StructureFooterState.swift +++ b/TablePro/Views/Structure/StructureFooterState.swift @@ -15,7 +15,26 @@ final class StructureFooterState { var addLabel: String = "" var removeLabel: String = "" - func deactivate() { + private(set) var currentOwner: UUID? + + func update( + owner: UUID, + canAdd: Bool, + canRemove: Bool, + addLabel: String, + removeLabel: String + ) { + currentOwner = owner + isActive = true + self.canAdd = canAdd + self.canRemove = canRemove + self.addLabel = addLabel + self.removeLabel = removeLabel + } + + func deactivate(owner: UUID) { + guard currentOwner == owner else { return } + currentOwner = nil isActive = false canAdd = false canRemove = false diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index 058d3f971..b75aed694 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -53,6 +53,7 @@ struct TableStructureView: View { @State var columnLayoutPersister: any ColumnLayoutPersisting = FileColumnLayoutPersister() @State var actionHandler = StructureViewActionHandler() @State var gridDelegate: StructureGridDelegate + @State private var footerOwnerId = UUID() init( tableName: String, @@ -131,7 +132,7 @@ struct TableStructureView: View { .onDisappear { coordinator?.toolbarState.hasStructureChanges = false coordinator?.structureActions = nil - coordinator?.structureFooterState.deactivate() + coordinator?.structureFooterState.deactivate(owner: footerOwnerId) selectionState.indices = [] } .onChange(of: structureChangeManager.hasChanges) { _, newValue in @@ -186,14 +187,16 @@ struct TableStructureView: View { guard let footer = coordinator?.structureFooterState else { return } guard connection.type.supportsSchemaEditing, let labels = footerLabels(for: selectedTab) else { - footer.deactivate() + footer.deactivate(owner: footerOwnerId) return } - footer.isActive = true - footer.addLabel = labels.add - footer.removeLabel = labels.remove - footer.canAdd = canAdd(for: selectedTab) - footer.canRemove = canRemove(for: selectedTab) + footer.update( + owner: footerOwnerId, + canAdd: canAdd(for: selectedTab), + canRemove: canRemove(for: selectedTab), + addLabel: labels.add, + removeLabel: labels.remove + ) } private func canAdd(for tab: StructureTab) -> Bool { diff --git a/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift b/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift index 690d8ba4c..ee47e9d87 100644 --- a/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift +++ b/TableProTests/Views/Structure/StructureGridDelegateAddRowTests.swift @@ -109,4 +109,43 @@ struct StructureGridDelegateAddRowTests { #expect(manager.workingIndexes.count == indexesBefore) #expect(manager.workingForeignKeys.count == fksBefore) } + + @Test("Columns sub-tab: dataGridDeleteRows removes the selected column") + func columnsTab_deleteRemovesColumn() { + let (delegate, manager) = makeDelegate(selectedTab: .columns) + delegate.dataGridAddRow() + let after = manager.workingColumns.count + #expect(after > 0) + delegate.dataGridDeleteRows([after - 1]) + #expect(manager.workingColumns.count == after - 1) + } + + @Test("Indexes sub-tab: dataGridDeleteRows removes the selected index") + func indexesTab_deleteRemovesIndex() { + let (delegate, manager) = makeDelegate(selectedTab: .indexes) + delegate.dataGridAddRow() + let after = manager.workingIndexes.count + #expect(after > 0) + delegate.dataGridDeleteRows([after - 1]) + #expect(manager.workingIndexes.count == after - 1) + } + + @Test("Foreign keys sub-tab: dataGridDeleteRows removes the selected foreign key") + func foreignKeysTab_deleteRemovesForeignKey() { + let (delegate, manager) = makeDelegate(selectedTab: .foreignKeys) + delegate.dataGridAddRow() + let after = manager.workingForeignKeys.count + #expect(after > 0) + delegate.dataGridDeleteRows([after - 1]) + #expect(manager.workingForeignKeys.count == after - 1) + } + + @Test("Indexes sub-tab on SQLite: dataGridDeleteRows is a no-op (supportsDropIndex == false)") + func sqliteIndexes_deleteIsNoOp() { + let (delegate, manager) = makeDelegate(selectedTab: .indexes, type: .sqlite) + manager.addIndex(.placeholder()) + let before = manager.workingIndexes.count + delegate.dataGridDeleteRows([before - 1]) + #expect(manager.workingIndexes.count == before) + } }