refactor(datagrid): de-duplicate pagination helpers and guard All rows on unknown totals (#1364) (#1411)

Author: datlechin

TablePro/Core/Coordinators/PaginationCoordinator.swift

@@ -53,32 +53,43 @@ final class PaginationCoordinator {
               let total = tab.pagination.totalRowCount, total > 0 else { return }
 
         let tabId = tab.id
-        let alert = NSAlert()
-        alert.messageText = String(localized: "Show All Rows")
-        alert.informativeText = String(
-            format: String(localized: "This will load all %@ rows on a single page. Large result sets use significant memory. Continue?"),
-            total.formatted()
-        )
-        alert.alertStyle = .warning
-        alert.addButton(withTitle: String(localized: "Show All"))
-        alert.addButton(withTitle: String(localized: "Cancel"))
-
-        let apply: () -> Void = { [weak self] in
+        confirmLargeFetch(
+            messageText: String(localized: "Show All Rows"),
+            informativeText: String(
+                format: String(localized: "This will load all %@ rows on a single page. Large result sets use significant memory. Continue?"),
+                total.formatted()
+            ),
+            confirmTitle: String(localized: "Show All")
+        ) { [weak self] in
             guard let self,
                   let tabIndex = parent.tabManager.tabs.firstIndex(where: { $0.id == tabId }) else { return }
             paginateAfterConfirmation(tabIndex: tabIndex) { pagination in
                 pagination.updatePageSize(max(total, 1))
                 pagination.goToFirstPage()
             }
         }
+    }
+
+    private func confirmLargeFetch(
+        messageText: String,
+        informativeText: String,
+        confirmTitle: String,
+        onConfirm: @escaping () -> Void
+    ) {
+        let alert = NSAlert()
+        alert.messageText = messageText
+        alert.informativeText = informativeText
+        alert.alertStyle = .warning
+        alert.addButton(withTitle: confirmTitle)
+        alert.addButton(withTitle: String(localized: "Cancel"))
 
         if let window = parent.contentWindow ?? NSApp.keyWindow {
             alert.beginSheetModal(for: window) { response in
                 guard response == .alertFirstButtonReturn else { return }
-                apply()
+                onConfirm()
             }
         } else if alert.runModal() == .alertFirstButtonReturn {
-            apply()
+            onConfirm()
         }
     }
 
@@ -159,22 +170,12 @@ final class PaginationCoordinator {
             message = String(localized: "This will fetch all remaining rows. Large result sets use significant memory. Continue?")
         }
 
-        let alert = NSAlert()
-        alert.messageText = String(localized: "Fetch All Rows")
-        alert.informativeText = message
-        alert.alertStyle = .warning
-        alert.addButton(withTitle: String(localized: "Fetch All"))
-        alert.addButton(withTitle: String(localized: "Cancel"))
-
-        let window = parent.contentWindow ?? NSApp.keyWindow
-        if let window {
-            alert.beginSheetModal(for: window) { [weak self] response in
-                guard let self, response == .alertFirstButtonReturn else { return }
-                performFetchAll(tabId: tab.id, baseQuery: baseQuery)
-            }
-        } else {
-            let response = alert.runModal()
-            guard response == .alertFirstButtonReturn else { return }
+        confirmLargeFetch(
+            messageText: String(localized: "Fetch All Rows"),
+            informativeText: message,
+            confirmTitle: String(localized: "Fetch All")
+        ) { [weak self] in
+            guard let self else { return }
             performFetchAll(tabId: tab.id, baseQuery: baseQuery)
         }
     }

TablePro/Models/Query/QueryTabState.swift

@@ -171,43 +171,37 @@ struct PaginationState: Equatable {
 
     // MARK: - Navigation Methods
 
-    /// Navigate to next page
+    private mutating func setPage(_ page: Int) {
+        currentPage = page
+        currentOffset = (page - 1) * pageSize
+    }
+
     mutating func goToNextPage() {
         guard hasNextPage else { return }
-        currentPage += 1
-        currentOffset = (currentPage - 1) * pageSize
+        setPage(currentPage + 1)
     }
 
     mutating func goToNextPage(loadedRowCount: Int) {
         guard canGoToNextPage(loadedRowCount: loadedRowCount) else { return }
-        currentPage += 1
-        currentOffset = (currentPage - 1) * pageSize
+        setPage(currentPage + 1)
     }
 
-    /// Navigate to previous page
     mutating func goToPreviousPage() {
         guard hasPreviousPage else { return }
-        currentPage -= 1
-        currentOffset = (currentPage - 1) * pageSize
+        setPage(currentPage - 1)
     }
 
-    /// Navigate to first page
     mutating func goToFirstPage() {
-        currentPage = 1
-        currentOffset = 0
+        setPage(1)
     }
 
-    /// Navigate to last page
     mutating func goToLastPage() {
-        currentPage = totalPages
-        currentOffset = (totalPages - 1) * pageSize
+        setPage(totalPages)
     }
 
-    /// Navigate to specific page
     mutating func goToPage(_ page: Int) {
         guard page > 0 && page <= totalPages else { return }
-        currentPage = page
-        currentOffset = (page - 1) * pageSize
+        setPage(page)
     }
 
     /// Reset pagination to first page

TablePro/Views/Components/PaginationControlsView.swift

@@ -49,6 +49,7 @@ struct PaginationControlsView: View {
             Divider()
 
             Button(String(localized: "All rows…")) { onShowAll() }
+                .disabled(!pagination.isLastPageKnown)
             Button(String(localized: "Custom…")) {
                 customText = "\(pagination.pageSize)"
                 showCustomPopover = true
@@ -167,39 +168,54 @@ struct PaginationControlsView: View {
     // MARK: - Popovers
 
     private var jumpPopover: some View {
-        VStack(alignment: .leading, spacing: 8) {
-            Text("Go to page")
-                .font(.caption)
-                .foregroundStyle(.secondary)
-            HStack {
-                TextField("", text: $jumpText)
-                    .frame(width: 70)
-                    .focused($isJumpFocused)
-                    .onSubmit(submitJump)
-                Button("Go", action: submitJump)
-                    .keyboardShortcut(.defaultAction)
-            }
-        }
-        .padding(12)
-        .onAppear { isJumpFocused = true }
+        submitPopover(
+            caption: "Go to page",
+            text: $jumpText,
+            fieldWidth: 70,
+            isFocused: $isJumpFocused,
+            fieldAccessibilityLabel: String(localized: "Page number"),
+            buttonTitle: "Go",
+            action: submitJump
+        )
     }
 
     private var customPageSizePopover: some View {
+        submitPopover(
+            caption: "Rows per page",
+            text: $customText,
+            fieldWidth: 80,
+            isFocused: $isCustomFocused,
+            fieldAccessibilityLabel: String(localized: "Rows per page"),
+            buttonTitle: "Apply",
+            action: submitCustom
+        )
+    }
+
+    private func submitPopover(
+        caption: LocalizedStringKey,
+        text: Binding<String>,
+        fieldWidth: CGFloat,
+        isFocused: FocusState<Bool>.Binding,
+        fieldAccessibilityLabel: String,
+        buttonTitle: LocalizedStringKey,
+        action: @escaping () -> Void
+    ) -> some View {
         VStack(alignment: .leading, spacing: 8) {
-            Text("Rows per page")
+            Text(caption)
                 .font(.caption)
                 .foregroundStyle(.secondary)
             HStack {
-                TextField("", text: $customText)
-                    .frame(width: 80)
-                    .focused($isCustomFocused)
-                    .onSubmit(submitCustom)
-                Button("Apply", action: submitCustom)
+                TextField("", text: text)
+                    .frame(width: fieldWidth)
+                    .focused(isFocused)
+                    .onSubmit(action)
+                    .accessibilityLabel(fieldAccessibilityLabel)
+                Button(buttonTitle, action: action)
                     .keyboardShortcut(.defaultAction)
             }
         }
         .padding(12)
-        .onAppear { isCustomFocused = true }
+        .onAppear { isFocused.wrappedValue = true }
     }
 
     // MARK: - Actions

TablePro/Views/Main/Child/MainStatusBarView.swift

@@ -185,7 +185,6 @@ struct MainStatusBarView: View {
                     .help(String(localized: "Toggle Filters (⇧⌘F)"))
                 }
 
-                // Pagination controls for table tabs
                 if snapshot.tabType == .table, snapshot.hasTableName, showsPaginationControls {
                     PaginationControlsView(
                         pagination: snapshot.pagination,
@@ -210,12 +209,15 @@ struct MainStatusBarView: View {
     }
 
     private var showsPaginationControls: Bool {
+        if let total = snapshot.pagination.totalRowCount, total > 0 { return true }
+        return isPagedWithUnknownTotal
+    }
+
+    private var isPagedWithUnknownTotal: Bool {
         let pagination = snapshot.pagination
-        if let total = pagination.totalRowCount, total > 0 { return true }
         return pagination.currentPage > 1 || snapshot.rowCount >= pagination.pageSize
     }
 
-    /// Generate row info text based on selection and pagination state
     private var rowInfoText: String {
         let loadedCount = snapshot.rowCount
         let selectedCount = selectedRowIndices.count
@@ -236,7 +238,7 @@ struct MainStatusBarView: View {
             let prefix = pagination.isApproximateRowCount ? "~" : ""
 
             return String(format: String(localized: "%d-%d of %@%@ rows"), pagination.rangeStart, pagination.rangeEnd, prefix, formattedTotal)
-        } else if snapshot.tabType == .table, pagination.currentPage > 1 || loadedCount >= pagination.pageSize {
+        } else if snapshot.tabType == .table, isPagedWithUnknownTotal {
             let rangeEnd = pagination.currentOffset + loadedCount
             return String(format: String(localized: "%d-%d of ? rows"), pagination.rangeStart, rangeEnd)
         } else if loadedCount > 0 {