From 10851b266dcb1209a0b5126b0fbf1baa48e8c3dc Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 24 May 2026 15:21:56 +0700 Subject: [PATCH] refactor(datagrid): de-duplicate pagination helpers and guard All rows on unknown totals (#1364) --- .../Coordinators/PaginationCoordinator.swift | 59 +++++++++--------- TablePro/Models/Query/QueryTabState.swift | 28 ++++----- .../Components/PaginationControlsView.swift | 60 ++++++++++++------- .../Views/Main/Child/MainStatusBarView.swift | 10 ++-- 4 files changed, 85 insertions(+), 72 deletions(-) diff --git a/TablePro/Core/Coordinators/PaginationCoordinator.swift b/TablePro/Core/Coordinators/PaginationCoordinator.swift index ef4ab868e..498dfc170 100644 --- a/TablePro/Core/Coordinators/PaginationCoordinator.swift +++ b/TablePro/Core/Coordinators/PaginationCoordinator.swift @@ -53,17 +53,14 @@ 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 @@ -71,14 +68,28 @@ final class PaginationCoordinator { 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) } } diff --git a/TablePro/Models/Query/QueryTabState.swift b/TablePro/Models/Query/QueryTabState.swift index 2d1e72742..9464d83e8 100644 --- a/TablePro/Models/Query/QueryTabState.swift +++ b/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 diff --git a/TablePro/Views/Components/PaginationControlsView.swift b/TablePro/Views/Components/PaginationControlsView.swift index 0bffd2a49..89fd83ba6 100644 --- a/TablePro/Views/Components/PaginationControlsView.swift +++ b/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, + fieldWidth: CGFloat, + isFocused: FocusState.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 diff --git a/TablePro/Views/Main/Child/MainStatusBarView.swift b/TablePro/Views/Main/Child/MainStatusBarView.swift index 1900ea98a..ce99b1e40 100644 --- a/TablePro/Views/Main/Child/MainStatusBarView.swift +++ b/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 {