fix(ios): TabView navigation fixes and code review cleanup (#870)

* fix(ios): inline query toolbar, dynamic tab titles, review fixes

- Move QueryEditorView run button and menu inline (TabView blocks toolbar propagation)
- Dynamic navigationTitle per tab (Tables/Query show connection name, History/Settings show tab name)
- Remove dead .navigationTitle/.toolbar from tab content views
- Show connection name in nav bar during connecting phase
- Rename tablesPath to navigationPath (shared across tabs)
- Remove duplicate safe mode icon from query action bar
- Add Sendable conformance to ConnectedTab

* fix(ios): review fixes across 6 files

- Add .environment(coordinator) to Settings tab for consistency
- Fix coordinator race condition on mid-connect navigation
- Add accessibility labels to keyboard shortcut buttons
- Localize alert/dialog strings in QueryEditorView and QueryHistoryView
- Cancel saveQueryTask on QueryEditorView disappear
- Fix reorderSection sort corruption in grouped connection list
- Refresh columnDetails on initial DataBrowserView load
- Cancel dismissSuccessTask on RowDetailView disappear
- Add systemGroupedBackground to RowDetailView

* fix(ios): restore navigationTitle on SettingsView for sheet presentation

Author: datlechin

TableProMobile/TableProMobile/Coordinators/ConnectionCoordinator.swift

@@ -33,7 +33,7 @@ final class ConnectionCoordinator {
         }
     }
     var pendingQuery: String?
-    var tablesPath = NavigationPath()
+    var navigationPath = NavigationPath()
 
     private(set) var queryHistory: [QueryHistoryItem] = []
     private let historyStorage = QueryHistoryStorage()
@@ -277,7 +277,7 @@ final class ConnectionCoordinator {
         appState.pendingTableName = nil
         selectedTab = .tables
         Task { @MainActor in
-            tablesPath.append(table)
+            navigationPath.append(table)
         }
     }
 

TableProMobile/TableProMobile/Models/ConnectedTab.swift

@@ -3,7 +3,7 @@
 //  TableProMobile
 //
 
-enum ConnectedTab: String, CaseIterable {
+enum ConnectedTab: String, CaseIterable, Sendable {
     case tables
     case query
     case history

TableProMobile/TableProMobile/Views/ConnectedView.swift

@@ -36,8 +36,10 @@ struct ConnectedView: View {
                 connectingView
             }
         }
+        .navigationTitle(connection.name.isEmpty ? connection.host : connection.name)
+        .navigationBarTitleDisplayMode(.inline)
         .task {
-            if let cached = cachedCoordinator, cached.session != nil {
+            if let cached = cachedCoordinator {
                 coordinator = cached
                 if case .connected = cached.phase { return }
                 await cached.connect()
@@ -86,7 +88,7 @@ struct ConnectedView: View {
 
     private func connectedContent(_ coordinator: ConnectionCoordinator) -> some View {
         @Bindable var coordinator = coordinator
-        return NavigationStack(path: $coordinator.tablesPath) {
+        return NavigationStack(path: $coordinator.navigationPath) {
             TabView(selection: $coordinator.selectedTab) {
                 Tab("Tables", systemImage: "tablecells", value: .tables) {
                     TableListView()
@@ -102,9 +104,10 @@ struct ConnectedView: View {
                 }
                 Tab("Settings", systemImage: "gear", value: .settings) {
                     SettingsView()
+                        .environment(coordinator)
                 }
             }
-            .navigationTitle(coordinator.displayName)
+            .navigationTitle(tabTitle(coordinator))
             .navigationBarTitleDisplayMode(.inline)
             .toolbar { connectionToolbar(coordinator) }
             .navigationDestination(for: TableInfo.self) { table in
@@ -115,15 +118,19 @@ struct ConnectedView: View {
         .background {
             Button("") { coordinator.selectedTab = .tables }
                 .keyboardShortcut("1", modifiers: .command)
+                .accessibilityLabel(Text("Tables"))
                 .hidden()
             Button("") { coordinator.selectedTab = .query }
                 .keyboardShortcut("2", modifiers: .command)
+                .accessibilityLabel(Text("Query"))
                 .hidden()
             Button("") { coordinator.selectedTab = .history }
                 .keyboardShortcut("3", modifiers: .command)
+                .accessibilityLabel(Text("History"))
                 .hidden()
             Button("") { coordinator.selectedTab = .settings }
                 .keyboardShortcut("4", modifiers: .command)
+                .accessibilityLabel(Text("Settings"))
                 .hidden()
         }
         .overlay(alignment: .top) {
@@ -168,6 +175,14 @@ struct ConnectedView: View {
         }
     }
 
+    private func tabTitle(_ coordinator: ConnectionCoordinator) -> String {
+        switch coordinator.selectedTab {
+        case .tables, .query: coordinator.displayName
+        case .history: String(localized: "History")
+        case .settings: String(localized: "Settings")
+        }
+    }
+
     // MARK: - Connection Toolbar
 
     @ToolbarContentBuilder

TableProMobile/TableProMobile/Views/ConnectionListView.swift

@@ -354,16 +354,14 @@ struct ConnectionListView: View {
         var items = sectionItems
         items.move(fromOffsets: source, toOffset: destination)
         var all = appState.connections
-        for (i, item) in items.enumerated() {
+        let baseOrder = items.compactMap { item in
+            all.firstIndex { $0.id == item.id }.map { all[$0].sortOrder }
+        }.sorted()
+        for (i, item) in items.enumerated() where i < baseOrder.count {
             if let idx = all.firstIndex(where: { $0.id == item.id }) {
-                all[idx].sortOrder = i
+                all[idx].sortOrder = baseOrder[i]
             }
         }
-        all.sort {
-            if $0.sortOrder != $1.sortOrder { return $0.sortOrder < $1.sortOrder }
-            return $0.name.localizedCaseInsensitiveCompare($1.name) == .orderedAscending
-        }
-        for index in all.indices { all[index].sortOrder = index }
         appState.reorderConnections(all)
     }
 

TableProMobile/TableProMobile/Views/DataBrowserView.swift

@@ -529,10 +529,10 @@ struct DataBrowserView: View {
             if rows.count < pagination.pageSize, pagination.totalRows == nil {
                 pagination.totalRows = pagination.currentOffset + rows.count
             }
-            if columnDetails.isEmpty {
+            if columnDetails.isEmpty || isInitial {
                 columnDetails = try await session.driver.fetchColumns(table: table.name, schema: nil)
             }
-            if foreignKeys.isEmpty {
+            if foreignKeys.isEmpty || isInitial {
                 do {
                     foreignKeys = try await session.driver.fetchForeignKeys(table: table.name, schema: nil)
                 } catch {

TableProMobile/TableProMobile/Views/QueryEditorView.swift

@@ -42,9 +42,6 @@ struct QueryEditorView: View {
             Divider()
             resultSection
         }
-        .navigationTitle(coordinator.displayName)
-        .navigationBarTitleDisplayMode(.inline)
-        .toolbar { toolbarContent }
         .onAppear {
             if let pending = coordinator.pendingQuery {
                 query = pending
@@ -61,19 +58,22 @@ struct QueryEditorView: View {
                 UserDefaults.standard.set(newValue, forKey: "lastQuery.\(connectionId.uuidString)")
             }
         }
+        .onDisappear {
+            saveQueryTask?.cancel()
+        }
         .onChange(of: coordinator.pendingQuery) { _, newQuery in
             if let newQuery {
                 query = newQuery
                 coordinator.pendingQuery = nil
             }
         }
-        .alert("Write Query Blocked", isPresented: $showWriteBlockedAlert) {
+        .alert(String(localized: "Write Query Blocked"), isPresented: $showWriteBlockedAlert) {
             Button("OK", role: .cancel) {}
         } message: {
             Text("This connection is in read-only mode. Write queries are not allowed.")
         }
-        .confirmationDialog("Execute Write Query?", isPresented: $showWriteConfirmation, titleVisibility: .visible) {
-            Button("Execute", role: .destructive) {
+        .confirmationDialog(String(localized: "Execute Write Query?"), isPresented: $showWriteConfirmation, titleVisibility: .visible) {
+            Button(String(localized: "Execute"), role: .destructive) {
                 executeTask = Task { await executeQueryDirect(pendingWriteQuery) }
             }
         } message: {
@@ -107,31 +107,57 @@ struct QueryEditorView: View {
             SQLHighlightTextView(text: $query)
                 .frame(minHeight: 80, maxHeight: result != nil || appError != nil ? 120 : 250)
 
-            if isExecuting || executionTime != nil || result != nil {
-                HStack {
-                    if isExecuting, let startTime = executionStartTime {
-                        TimelineView(.periodic(from: startTime, by: 0.1)) { context in
-                            let elapsed = context.date.timeIntervalSince(startTime)
-                            Label(String(format: "%.1fs", elapsed), systemImage: "clock")
-                                .font(.caption2)
-                                .foregroundStyle(.secondary)
-                        }
-                    } else if let time = executionTime {
-                        Label(String(format: "%.1fms", time * 1000), systemImage: "clock")
-                            .font(.caption2)
-                            .foregroundStyle(.secondary)
-                    }
-                    Spacer()
-                    if let result, !result.rows.isEmpty {
-                        Text(verbatim: "\(result.rows.count) rows")
-                            .font(.caption2)
-                            .foregroundStyle(.secondary)
-                    }
+            actionBar
+        }
+    }
+
+    private var actionBar: some View {
+        HStack(spacing: 12) {
+            Button {
+                if isExecuting {
+                    executeTask?.cancel()
+                    Task { try? await session?.driver.cancelCurrentQuery() }
+                } else {
+                    executeTask = Task { await executeQuery() }
                 }
-                .padding(.horizontal, 12)
-                .padding(.bottom, 4)
+            } label: {
+                Label(
+                    isExecuting ? String(localized: "Stop") : String(localized: "Run"),
+                    systemImage: isExecuting ? "stop.fill" : "play.fill"
+                )
+                .font(.subheadline.weight(.medium))
             }
+            .buttonStyle(.bordered)
+            .controlSize(.small)
+            .tint(isExecuting ? .red : .accentColor)
+            .disabled(!isExecuting && query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty)
+            .keyboardShortcut(.return, modifiers: .command)
+
+            Spacer()
+
+            if isExecuting, let startTime = executionStartTime {
+                TimelineView(.periodic(from: startTime, by: 0.1)) { context in
+                    let elapsed = context.date.timeIntervalSince(startTime)
+                    Text(String(format: "%.1fs", elapsed))
+                        .font(.caption2)
+                        .foregroundStyle(.secondary)
+                }
+            } else if let time = executionTime {
+                Text(String(format: "%.1fms", time * 1000))
+                    .font(.caption2)
+                    .foregroundStyle(.secondary)
+            }
+
+            if let result, !result.rows.isEmpty {
+                Text(verbatim: "\(result.rows.count) rows")
+                    .font(.caption2)
+                    .foregroundStyle(.secondary)
+            }
+
+            queryMenu
         }
+        .padding(.horizontal, 12)
+        .padding(.vertical, 6)
     }
 
     // MARK: - Results
@@ -172,7 +198,7 @@ struct QueryEditorView: View {
                         Image(systemName: "checkmark.circle.fill")
                             .font(.largeTitle)
                             .foregroundStyle(.green)
-                        Text(verbatim: "\(result.rowsAffected) row(s) affected")
+                        Text(String(format: String(localized: "%d row(s) affected"), result.rowsAffected))
                             .font(.body)
                     }
                     .frame(maxWidth: .infinity, maxHeight: .infinity)
@@ -259,93 +285,69 @@ struct QueryEditorView: View {
         }
     }
 
-    // MARK: - Toolbar
+    // MARK: - Query Menu
 
-    @ToolbarContentBuilder
-    private var toolbarContent: some ToolbarContent {
-        if safeModeLevel != .off {
-            ToolbarItem(placement: .topBarLeading) {
-                Image(systemName: safeModeLevel == .readOnly ? "lock.fill" : "shield.fill")
-                    .foregroundStyle(safeModeLevel == .readOnly ? .red : .orange)
-                    .font(.caption)
-            }
-        }
-
-        ToolbarItem(placement: .primaryAction) {
+    private var queryMenu: some View {
+        Menu {
             Button {
-                if isExecuting {
-                    executeTask?.cancel()
-                    Task { try? await session?.driver.cancelCurrentQuery() }
-                } else {
-                    executeTask = Task { await executeQuery() }
-                }
+                coordinator.selectedTab = .history
             } label: {
-                Image(systemName: isExecuting ? "stop.fill" : "play.fill")
+                Label("History", systemImage: "clock")
             }
-            .disabled(!isExecuting && query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty)
-            .keyboardShortcut(.return, modifiers: .command)
-        }
-
-        ToolbarItem(placement: .topBarTrailing) {
-            Menu {
-                Button {
-                    coordinator.selectedTab = .history
-                } label: {
-                    Label("History", systemImage: "clock")
-                }
 
-                if !tables.isEmpty {
-                    Menu {
-                        ForEach(tables) { table in
-                            Button(table.name) {
-                                let quoted = SQLBuilder.quoteIdentifier(table.name, for: databaseType)
-                                query = "SELECT * FROM \(quoted) LIMIT 100"
-                            }
+            if !tables.isEmpty {
+                Menu {
+                    ForEach(tables) { table in
+                        Button(table.name) {
+                            let quoted = SQLBuilder.quoteIdentifier(table.name, for: databaseType)
+                            query = "SELECT * FROM \(quoted) LIMIT 100"
                         }
-                    } label: {
-                        Label("SELECT * FROM ...", systemImage: "text.badge.star")
                     }
+                } label: {
+                    Label("SELECT * FROM ...", systemImage: "text.badge.star")
                 }
+            }
 
-                if let result, !result.rows.isEmpty {
-                    Section("Share Results") {
-                        ForEach(ExportFormat.allCases) { format in
-                            Button {
-                                shareText = ClipboardExporter.exportRows(
-                                    columns: result.columns, rows: result.rows,
-                                    format: format
-                                )
-                                showShareSheet = true
-                            } label: {
-                                Label(format.rawValue, systemImage: "square.and.arrow.up")
-                            }
+            if let result, !result.rows.isEmpty {
+                Section("Share Results") {
+                    ForEach(ExportFormat.allCases) { format in
+                        Button {
+                            shareText = ClipboardExporter.exportRows(
+                                columns: result.columns, rows: result.rows,
+                                format: format
+                            )
+                            showShareSheet = true
+                        } label: {
+                            Label(format.rawValue, systemImage: "square.and.arrow.up")
                         }
                     }
-                    Section("Copy Results") {
-                        ForEach(ExportFormat.allCases) { format in
-                            Button {
-                                let text = ClipboardExporter.exportRows(
-                                    columns: result.columns, rows: result.rows,
-                                    format: format
-                                )
-                                ClipboardExporter.copyToClipboard(text)
-                            } label: {
-                                Label(format.rawValue, systemImage: "doc.on.clipboard")
-                            }
+                }
+                Section("Copy Results") {
+                    ForEach(ExportFormat.allCases) { format in
+                        Button {
+                            let text = ClipboardExporter.exportRows(
+                                columns: result.columns, rows: result.rows,
+                                format: format
+                            )
+                            ClipboardExporter.copyToClipboard(text)
+                        } label: {
+                            Label(format.rawValue, systemImage: "doc.on.clipboard")
                         }
                     }
                 }
+            }
 
-                Divider()
+            Divider()
 
-                Button(role: .destructive) {
-                    showClearConfirmation = true
-                } label: {
-                    Label("Clear", systemImage: "trash")
-                }
+            Button(role: .destructive) {
+                showClearConfirmation = true
             } label: {
-                Image(systemName: "ellipsis.circle")
+                Label("Clear", systemImage: "trash")
             }
+        } label: {
+            Image(systemName: "ellipsis.circle")
+                .font(.body)
+                .foregroundStyle(.secondary)
         }
     }