Skip to content

fix(structure): row visual state, per-cell modified tint, native NSTableView refresh, row-specific undo delete #1189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
datlechin merged 1 commit into from
May 10, 2026
Merged

fix(structure): row visual state, per-cell modified tint, native NSTableView refresh, row-specific undo delete #1189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Structure tab: double-click and Return on dropdown / type-picker columns (Nullable, Primary Key, Auto Increment, Unique, On Delete, On Update, Type) now enter inline text edit so the user can type a value the picker doesn't list (e.g. a custom column type, a numeric default). Previously these gestures appeared to do nothing because `editEligibility` blocked dropdown/type-picker columns from inline edit. The chevron button continues to open the picker; the cell body opens the text editor.
- Structure tab: pressing Cmd+Shift+N (or any path that adds a column / index / foreign key while changes already exist) now displays the new row in the grid. Previously the row was added to the change manager and the SQL Preview reflected it, but the grid kept the old row count because `TableStructureView` only observed `hasChanges` (which had already been true). The view now also observes `reloadVersion` and re-evaluates the grid snapshot on every mutation.
- Structure tab: pressing Delete on a row now turns the entire row red, not just the focused cell. `StructureRowViewWithMenu` inherits from `DataGridRowView` so it gets the deleted-row tint, layer-backing optimization, and the cell-emphasis invalidation that all data-tab rows already had. The grid's `tableView(_:rowViewForRow:)` also now applies the visual state to delegate-provided row views, so recycled rows pick up state changes. `StructureGridDelegate.dataGridDeleteRows` calls `tableView.reloadData(forRowIndexes:columnIndexes:)` for visible rows so the soft-deleted row repaints (existing-column deletes leave the row in place with a red tint, so row count alone doesn't trigger a reload).
- Structure tab: editing a cell now shows the new value in that cell and tints only the cells the user actually edited, matching the data tab. Previously the change manager recorded the edit (SQL Preview reflected it, partial yellow tint appeared), but the cell still displayed the pre-edit value and the tint cut off mid-row, then the prior fix tinted the entire row even when only one field changed. Two fixes: (1) `tableRowsProvider` is now a closure that rebuilds the snapshot from the change manager on every call (mirrors the data tab's `coordinator.tabSessionRegistry.existingTableRows(for:)` pattern), so `tableView.reloadData(forRowIndexes:)` after an edit re-fetches the post-edit row. (2) Per-cell modified tinting is computed by diffing the working entity against `currentColumns` / `currentIndexes` / `currentForeignKeys` field-by-field, mapped to grid column indices via the tab's `orderedFields`. `StructureEditingSupport` gains `columnModifiedIndices`, `indexModifiedIndices`, and `foreignKeyModifiedIndices` helpers; `StructureGridDelegate.dataGridVisualState(forRow:)` calls them and only the changed cells get the yellow tint. `StructureGridDelegate.dataGridAttach(tableViewCoordinator:)` and `CreateTableGridDelegate.dataGridAttach(tableViewCoordinator:)` store the grid coordinator weakly so the delegate can issue the targeted reload after edits, soft-deletes, undo, and redo.
- Structure tab: Cmd+Z no longer leaves the row's yellow modified background in place after the change reverts. The view observes `reloadVersion` to bump `displayVersion`, and `dataGridUndo` / `dataGridRedo` ask the table view to reload its visible rows so the cell text and modified-tint follow the change manager. The SQL Preview popover also clears its content when the change set returns to empty, so reopening the popover after undo correctly shows "no changes" instead of stale SQL.
- Structure tab: saving or discarding changes now clears the yellow modified tint on the affected rows. After save, `StructureChangeManager.loadSchema` resets working state and bumps `reloadVersion`, but `DataGridView.updateNSView` only calls `reloadData` when row count or column schema changes — a column rename leaves the row count identical so cells kept their pre-save visual state. The save and discard paths in `TableStructureView+Schema` now call the new `StructureGridDelegate.reloadAllVisibleRows()` after the manager resets.
- Structure tab: deleted rows now paint the red strikethrough tint and right-click "Undo Delete" undeletes the specific row, matching the data tab. `NSTableView.reloadData(forRowIndexes:columnIndexes:)` only refreshes cell views, not row views, so the per-row deleted-state tint and context-menu state went stale after every model mutation. `TableViewCoordinator` now exposes `reloadVisibleRowsAndStates()` and `reloadRowAndState(at:)` that pair `reloadData(forRowIndexes:)` with `enumerateAvailableRowViews` and `applyVisualState`, so cells and row decoration refresh together. `DataGridRowView` stores the full `RowVisualState` as the source of truth, and `StructureRowViewWithMenu` reads `visualState.isDeleted` directly instead of a shadow `isRowDeleted` flag that was only assigned on row-view creation. `StructureChangeManager.undoDelete(for:at:)` clears the per-row deletion mark without touching the global NSUndoManager stack, so the right-click affordance and Cmd+Z stay independent, matching the separation the data tab already uses.
- Cmd+Shift+N (Add Row) on a Structure tab no longer routes to the data-tab row-editing coordinator (which silently did nothing or added a data row). `MainContentCommandActions.addNewRow` now branches on `resultsViewMode == .structure` and dispatches to the Structure grid delegate.
- Create Table: foreign-key on-delete / on-update referential-action dropdowns and the index-type dropdown now render as dropdowns. Previously `CreateTableView` constructed `DataGridConfiguration` without `customDropdownOptions`, so those columns showed plain text values without the picker affordance.
- iOS: row detail pager no longer carries the index-based row iteration that caused the build 11 filter crash. A recent refactor reintroduced the pattern; row detail now uses the same identity-based row wrapping the data browser does.
- Tables in the sidebar now load automatically after a slow connect. SQL Server connections previously showed "No Tables" until the user manually picked a schema; the same race could affect other engines on slow networks. The post-connect listener now triggers the schema load once the driver is bound.
- SQL Server `switchDatabase` now actually switches the database (`USE <database>`) instead of being routed through a schema switch. Switching from a saved tab pointing at a different database used to overwrite the current schema with the database name and leave the table list empty until the user manually re-picked a schema.
Expand Down
142 changes: 52 additions & 90 deletions TablePro/Core/SchemaTracking/StructureChangeManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,20 @@ final class StructureChangeManager: ChangeManaging {

// MARK: - Undo/Redo Support

/// Private `NSUndoManager` owned by this change manager. Each
/// `StructureChangeManager` instance has its own, so the registered actions
/// can never outlive the manager (the UndoManager is freed when the manager
/// is deallocated, taking its action queue with it). The app does not have
/// an NSDocument-backed `NSWindow.undoManager`, and no view in the
/// responder chain provides one, so wiring this through the window would
/// silently no-op. Cmd+Z is routed by the app's own `.commands` block in
/// `TableProApp` to `MainContentCommandActions.undoChange()`, which checks
/// the active tab's `resultsViewMode` and calls into this manager directly.
private let undoManager: UndoManager = {
let manager = UndoManager()
manager.levelsOfUndo = 100
return manager
}()
private var visualStateCache: [VisualStateCacheKey: RowVisualState] = [:]

var canUndo: Bool { undoManager.canUndo }
var canRedo: Bool { undoManager.canRedo }
Expand Down Expand Up @@ -129,7 +137,6 @@ final class StructureChangeManager: ChangeManaging {
undoManager.setActionName(String(localized: "Add Column"))
validate()
reloadVersion += 1
rebuildVisualStateCache()
}

func addNewIndex() {
Expand All @@ -144,7 +151,6 @@ final class StructureChangeManager: ChangeManaging {
undoManager.setActionName(String(localized: "Add Index"))
validate()
reloadVersion += 1
rebuildVisualStateCache()
}

func addNewForeignKey() {
Expand All @@ -159,7 +165,6 @@ final class StructureChangeManager: ChangeManaging {
undoManager.setActionName(String(localized: "Add Foreign Key"))
validate()
reloadVersion += 1
rebuildVisualStateCache()
}

// MARK: - Paste Operations (public methods for adding copied items)
Expand All @@ -174,7 +179,6 @@ final class StructureChangeManager: ChangeManaging {
}
undoManager.setActionName(String(localized: "Add Column"))
reloadVersion += 1
rebuildVisualStateCache()
}

func addIndex(_ index: EditableIndexDefinition) {
Expand All @@ -187,7 +191,6 @@ final class StructureChangeManager: ChangeManaging {
}
undoManager.setActionName(String(localized: "Add Index"))
reloadVersion += 1
rebuildVisualStateCache()
}

func addForeignKey(_ foreignKey: EditableForeignKeyDefinition) {
Expand All @@ -200,7 +203,6 @@ final class StructureChangeManager: ChangeManaging {
}
undoManager.setActionName(String(localized: "Add Foreign Key"))
reloadVersion += 1
rebuildVisualStateCache()
}

// MARK: - Column Operations
Expand Down Expand Up @@ -238,7 +240,6 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

func deleteColumn(id: UUID) {
Expand All @@ -265,7 +266,6 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

// MARK: - Index Operations
Expand Down Expand Up @@ -303,7 +303,6 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

func deleteIndex(id: UUID) {
Expand All @@ -330,7 +329,6 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

// MARK: - Foreign Key Operations
Expand Down Expand Up @@ -368,7 +366,6 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

func deleteForeignKey(id: UUID) {
Expand All @@ -395,32 +392,38 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

// MARK: - Primary Key Operations
// MARK: - Row-Specific Undo Delete

func updatePrimaryKey(_ columns: [String]) {
// Push undo action before modifying
if columns != workingPrimaryKey {
let oldPK = workingPrimaryKey
undoManager.registerUndo(withTarget: self) { target in
target.applySchemaUndo(.primaryKeyChange(old: oldPK, new: columns))
}
undoManager.setActionName(String(localized: "Change Primary Key"))
}

let key = SchemaChangeIdentifier.primaryKey
if columns != currentPrimaryKey {
pendingChanges[key] = .modifyPrimaryKey(old: currentPrimaryKey, new: columns)
trackChangeKey(key)
} else {
pendingChanges.removeValue(forKey: key)
untrackChangeKey(key)
}

workingPrimaryKey = columns
/// Clear the deletion mark for the entity at `row` in `tab`. Mirrors
/// `DataChangeManager.undoRowDeletion(rowIndex:)`: the global NSUndoManager
/// stack is intentionally left alone. The original `applySchemaUndo(...)`
/// handler the deletion registered remains on the stack; if global Cmd+Z
/// later invokes it, the handler finds `pendingChanges` no longer marks
/// this row as deleted and treats the redo as a no-op for this entity. The
/// row-specific affordance and the global undo stack are independent
/// affordances. The data tab uses the same separation.
func undoDelete(for tab: StructureTab, at row: Int) {
let key: SchemaChangeIdentifier
switch tab {
case .columns:
guard row < workingColumns.count else { return }
key = .column(workingColumns[row].id)
case .indexes:
guard row < workingIndexes.count else { return }
key = .index(workingIndexes[row].id)
case .foreignKeys:
guard row < workingForeignKeys.count else { return }
key = .foreignKey(workingForeignKeys[row].id)
case .ddl, .parts:
return
}
guard pendingChanges[key]?.isDelete == true else { return }
pendingChanges.removeValue(forKey: key)
untrackChangeKey(key)
validate()
reloadVersion += 1
}

// MARK: - Validation
Expand Down Expand Up @@ -520,7 +523,6 @@ final class StructureChangeManager: ChangeManaging {
validationErrors.removeAll()
resetWorkingState()
reloadVersion += 1
rebuildVisualStateCache()
undoManager.removeAllActions()
}

Expand Down Expand Up @@ -566,7 +568,6 @@ final class StructureChangeManager: ChangeManaging {

validate()
reloadVersion += 1
rebuildVisualStateCache()
}

private func applyColumnEditUndo(id: UUID, old: EditableColumnDefinition, new: EditableColumnDefinition) {
Expand Down Expand Up @@ -775,77 +776,38 @@ final class StructureChangeManager: ChangeManaging {

// MARK: - Visual State Management

func getVisualState(for row: Int, tab: StructureTab) -> RowVisualState {
let cacheKey = VisualStateCacheKey(tab: tab, row: row)
if let cached = visualStateCache[cacheKey] {
return cached
}

let state: RowVisualState

/// Per-row delete/insert flags. Modified-column tinting is computed by the
/// `StructureGridDelegate` because it requires the tab's `orderedFields`
/// (which depends on the database type and is a UI concern). The delegate
/// merges the result of this method with `modifiedColumns` from
/// `StructureEditingSupport` field-diff helpers to build the final
/// `RowVisualState`.
func deleteInsertState(for row: Int, tab: StructureTab) -> (isDeleted: Bool, isInserted: Bool) {
switch tab {
case .columns:
guard row < workingColumns.count else { return .empty }
guard row < workingColumns.count else { return (false, false) }
let column = workingColumns[row]
let change = pendingChanges[.column(column.id)]

let isDeleted = change?.isDelete ?? false
let isInserted = !currentColumns.contains(where: { $0.id == column.id })
let isModified = change != nil && !isDeleted && !isInserted

state = RowVisualState(
isDeleted: isDeleted,
isInserted: isInserted,
modifiedColumns: isModified ? Set(0..<6) : []
)

return (isDeleted, isInserted)
case .indexes:
guard row < workingIndexes.count else { return .empty }
guard row < workingIndexes.count else { return (false, false) }
let index = workingIndexes[row]
let change = pendingChanges[.index(index.id)]

let isDeleted = change?.isDelete ?? false
let isInserted = !currentIndexes.contains(where: { $0.id == index.id })
let isModified = change != nil && !isDeleted && !isInserted

state = RowVisualState(
isDeleted: isDeleted,
isInserted: isInserted,
modifiedColumns: isModified ? Set(0..<5) : []
)

return (isDeleted, isInserted)
case .foreignKeys:
guard row < workingForeignKeys.count else { return .empty }
guard row < workingForeignKeys.count else { return (false, false) }
let fk = workingForeignKeys[row]
let change = pendingChanges[.foreignKey(fk.id)]

let isDeleted = change?.isDelete ?? false
let isInserted = !currentForeignKeys.contains(where: { $0.id == fk.id })
let isModified = change != nil && !isDeleted && !isInserted

state = RowVisualState(
isDeleted: isDeleted,
isInserted: isInserted,
modifiedColumns: isModified ? Set(0..<7) : []
)

case .ddl:
state = .empty
case .parts:
state = .empty
return (isDeleted, isInserted)
case .ddl, .parts:
return (false, false)
}

visualStateCache[cacheKey] = state
return state
}

func rebuildVisualStateCache() {
visualStateCache.removeAll()
}

private struct VisualStateCacheKey: Hashable {
let tab: StructureTab
let row: Int
}

// MARK: - ChangeManaging Conformance (Data-Specific No-Ops)
Expand Down
10 changes: 9 additions & 1 deletion TablePro/Views/Main/MainContentCommandActions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,15 @@ final class MainContentCommandActions {
// MARK: - Row Operations (Group A — Called Directly)

func addNewRow() {
coordinator?.addNewRow()
// The structure tab routes through StructureGridDelegate, which inserts
// a column / index / FK row depending on the active Structure sub-tab.
// The data tab routes through MainContentCoordinator.addNewRow which
// calls RowEditingCoordinator.addNewRow (data-only).
if coordinator?.tabManager.selectedTab?.display.resultsViewMode == .structure {
coordinator?.structureActions?.addRow?()
} else {
coordinator?.addNewRow()
}
}

func deleteSelectedRows(rowIndices: Set<Int>? = nil) {
Expand Down
2 changes: 1 addition & 1 deletion TablePro/Views/Results/Cells/DataGridCellView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ final class DataGridCellView: NSView {
let nextTint: NSColor?
if state.visualState.isDeleted || state.visualState.isInserted {
nextTint = nil
} else if state.visualState.modifiedColumns.contains(state.columnIndex) {
} else if state.visualState.isModified(columnIndex: state.columnIndex) {
nextTint = palette.modifiedColumnTint
} else {
nextTint = nil
Expand Down
23 changes: 23 additions & 0 deletions TablePro/Views/Results/DataGridCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,17 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
func invalidateCachesForUndoRedo() {
invalidateAllDisplayCaches()
updateCache()
reloadVisibleRowsAndStates()
}

/// Repaint visible rows in two layers Apple's NSTableView contract requires:
/// `reloadData(forRowIndexes:columnIndexes:)` re-fetches cells via
/// `tableView(_:viewFor:row:)` but does not touch row views, so per-row
/// decoration (deleted/inserted tint, deleted-row context menu state) goes
/// stale. `enumerateAvailableRowViews` then visits each live `NSTableRowView`
/// so `applyVisualState` can mutate row-level state without recreating views.
/// Both delegates call this after model mutations that don't change row count.
func reloadVisibleRowsAndStates() {
guard let tableView else { return }
let visibleRange = tableView.rows(in: tableView.visibleRect)
guard visibleRange.length > 0 else { return }
Expand All @@ -433,6 +444,18 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData
refreshVisibleRowVisualStates()
}

/// Single-row equivalent of `reloadVisibleRowsAndStates` for cases where
/// only one row's content + visual state changed (cell edit, single-row
/// undo delete).
func reloadRowAndState(at row: Int) {
guard let tableView, row >= 0, row < tableView.numberOfRows else { return }
tableView.reloadData(
forRowIndexes: IndexSet(integer: row),
columnIndexes: IndexSet(integersIn: 0..<tableView.numberOfColumns)
)
refreshRowVisualState(at: row)
}

func refreshVisibleRowVisualStates() {
guard let tableView else { return }
tableView.enumerateAvailableRowViews { [weak self] rowView, row in
Expand Down
Loading
Loading