diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aaa4ae66..77c1b74c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Keep Open for a preview tab, by double-clicking it in the tab strip or from its contextual menu. (#2436) - Kafka driver plugin: topics in the grid, consumer group lag, and KafkaQL for seeking and producing. (#2419) ## [0.68.1] - 2026-08-26 diff --git a/TablePro/Core/Services/Infrastructure/EditorTabActivation.swift b/TablePro/Core/Services/Infrastructure/EditorTabActivation.swift new file mode 100644 index 000000000..c2bed6484 --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/EditorTabActivation.swift @@ -0,0 +1,70 @@ +// +// EditorTabActivation.swift +// TablePro +// + +import AppKit +import Foundation + +/// What a click on a tab in the editor tab strip is asking for. +internal enum EditorTabActivation: Equatable { + case select + /// Select, then keep the tab: the double-click that turns a preview tab permanent. + case selectAndKeep +} + +/// One click on a tab, reduced to the two things that decide what it means. +/// +/// `NSEvent.clickCount` is only valid on a mouse-down or mouse-up and raises for anything else, +/// so the type is checked here rather than at the call site. A tab's button also answers the +/// keyboard and VoiceOver, and neither leaves a mouse event current: both resolve to nil, which +/// the resolver reads as a plain selection. Keeping a tab from the keyboard goes through the +/// "Keep Open" command instead. +internal struct EditorTabClick: Equatable { + internal let clickCount: Int + internal let hasModifiers: Bool + + internal init(clickCount: Int, hasModifiers: Bool) { + self.clickCount = clickCount + self.hasModifiers = hasModifiers + } + + internal init?(event: NSEvent?) { + guard let event, Self.carriesClickCount(event.type) else { return nil } + clickCount = event.clickCount + hasModifiers = !event.modifierFlags.intersection(.deviceIndependentFlagsMask) + .subtracting(.capsLock) + .isEmpty + } + + /// Only the primary button. A control-click is the context-menu gesture and arrives as a left + /// mouse-down with a modifier, which `hasModifiers` then rejects. + private static func carriesClickCount(_ type: NSEvent.EventType) -> Bool { + type == .leftMouseDown || type == .leftMouseUp + } +} + +/// Resolves a click on a tab into what it means, without reference to AppKit or to the strip. +/// +/// The second click has to land on the tab the first one activated. Two clicks close enough in +/// time and space arrive as one click of count two whichever view each of them hit, and tabs sit +/// flush against each other, so a pair straddling a boundary would otherwise keep a tab the user +/// only meant to select. `NSTableView` has the same exposure and lives with it, because its +/// double-click opens the row the second click hit and a single click would have led there +/// anyway; here it would change a tab's state without being asked. +/// +/// Any count above one keeps the tab, rather than two exactly. That same coalescing carries the +/// count on past two, so a double-click following a nearby click arrives as counts two and three: +/// the two is refused by the guard above, and a strict `== 2` would refuse the three as well, +/// leaving a genuine double-click doing nothing. Keeping is idempotent and one way, so acting on +/// the three costs nothing. +internal enum EditorTabActivationResolver { + internal static func resolve( + click: EditorTabClick?, + tabId: UUID, + lastActivatedTabId: UUID? + ) -> EditorTabActivation { + guard let click, click.clickCount >= 2, !click.hasModifiers else { return .select } + return lastActivatedTabId == tabId ? .selectAndKeep : .select + } +} diff --git a/TablePro/Models/Query/QueryTabManager.swift b/TablePro/Models/Query/QueryTabManager.swift index f01a246a9..af178a4e8 100644 --- a/TablePro/Models/Query/QueryTabManager.swift +++ b/TablePro/Models/Query/QueryTabManager.swift @@ -80,6 +80,26 @@ final class QueryTabManager { moveTab(id: id, to: index + offset) } + /// Keeps a preview tab, so the next table opened from the sidebar lands in a tab of its own + /// instead of replacing this one. The single mutator of `isPreview`: every promotion path, + /// the sidebar's, the editor's and the tab strip's, comes through here. + /// + /// Promotion never reorders. A tab that moves as it is kept would be a different feature, + /// which is what pinning is in the editors that offer both. + /// + /// One way, deliberately. Nothing turns a kept tab back into a preview, so a tab the user + /// asked to hold on to cannot be thrown away by a later click. + func promotePreviewTab(id: UUID) { + guard let index = tabs.firstIndex(where: { $0.id == id }), tabs[index].isPreview else { return } + mutate(at: index) { $0.isPreview = false } + } + + /// Whether keeping this tab would change anything, so a menu item can dim rather than be + /// offered and do nothing. + func canPromotePreviewTab(id: UUID) -> Bool { + tabs.first { $0.id == id }?.isPreview ?? false + } + func selectTab(at index: Int) { guard tabs.indices.contains(index) else { return } selectedTabId = tabs[index].id diff --git a/TablePro/Views/Main/EditorTabStrip.swift b/TablePro/Views/Main/EditorTabStrip.swift index 711286450..d59fa494f 100644 --- a/TablePro/Views/Main/EditorTabStrip.swift +++ b/TablePro/Views/Main/EditorTabStrip.swift @@ -46,6 +46,13 @@ internal struct EditorTabStrip: View { /// separators are a property of the row: they are hidden for the whole strip while a tab is in /// flight, so a line does not appear between two tabs that are mid-swap. @State private var draggingTabId: UUID? + /// The tab the previous click activated, so the second click of a double-click can be told + /// from a click on a neighbour that happened to land inside the double-click window. + @State private var lastActivatedTabId: UUID? + /// A tab whose selection came from a click on the tab itself, which must not be recentred. + /// The track scrolls once the tabs stop fitting, and sliding the clicked tab to the middle + /// takes it out from under a second click that is already on its way. + @State private var clickSelectedTabId: UUID? @Environment(\.controlActiveState) private var controlActiveState @Environment(\.colorSchemeContrast) private var colorSchemeContrast @Environment(\.accessibilityReduceTransparency) private var reduceTransparency @@ -72,6 +79,7 @@ internal struct EditorTabStrip: View { .onChange(of: tabManager.tabs.map(\.id)) { _, ids in if let hoveredTabId, !ids.contains(hoveredTabId) { self.hoveredTabId = nil } if let draggingTabId, !ids.contains(draggingTabId) { self.draggingTabId = nil } + if let lastActivatedTabId, !ids.contains(lastActivatedTabId) { self.lastActivatedTabId = nil } } .accessibilityElement(children: .contain) .accessibilityLabel(Text("Editor Tabs")) @@ -118,6 +126,10 @@ internal struct EditorTabStrip: View { /// a tab that is scrolled out of sight, so the selection pulls itself into view. .onChange(of: tabManager.selectedTabId) { _, newValue in guard let newValue else { return } + guard clickSelectedTabId != newValue else { + clickSelectedTabId = nil + return + } withMotion(.easeOut(duration: 0.15)) { scroller.scrollTo(newValue, anchor: .center) } @@ -165,10 +177,12 @@ internal struct EditorTabStrip: View { hoveredTabId = nil } }, - onSelect: { tabManager.selectedTabId = tab.id }, + onActivate: { activate(tab.id) }, onClose: { onClose(tab.id) }, onCloseOthers: { onCloseOthers(tab.id) }, onCloseAll: onCloseAll, + canKeepOpen: tabManager.canPromotePreviewTab(id: tab.id), + onKeepOpen: { tabManager.promotePreviewTab(id: tab.id) }, canMoveLeft: tabManager.canMoveTab(id: tab.id, by: -1), canMoveRight: tabManager.canMoveTab(id: tab.id, by: 1), onMoveLeft: { tabManager.moveTab(id: tab.id, by: -1) }, @@ -191,6 +205,30 @@ internal struct EditorTabStrip: View { ) } + /// Selects the tab, and keeps it when the click that got here was the second of a double-click. + /// + /// The click count is read off the event AppKit is currently dispatching rather than arbitrated + /// by a SwiftUI gesture, which is what `NSTableView` does with `action` and `doubleAction`. + /// Measured against the shipping strip: a `TapGesture(count: 2)` in any composition holds the + /// selection back 371ms on every click and drops it entirely on the double, and a + /// `simultaneousGesture` selects twice; reading the event costs the same 22ms as selecting. + private func activate(_ tabId: UUID) { + let activation = EditorTabActivationResolver.resolve( + click: EditorTabClick(event: NSApp.currentEvent), + tabId: tabId, + lastActivatedTabId: lastActivatedTabId + ) + lastActivatedTabId = tabId + /// Set only when the selection is about to change, so the flag is always consumed by the + /// `onChange` it is meant for rather than left behind to swallow a later Cmd+1. + if tabManager.selectedTabId != tabId { + clickSelectedTabId = tabId + } + tabManager.selectedTabId = tabId + guard activation == .selectAndKeep else { return } + tabManager.promotePreviewTab(id: tabId) + } + private var isWindowActive: Bool { controlActiveState != .inactive } @@ -273,10 +311,12 @@ private struct EditorTabStripItem: View { let position: Int let count: Int let onHover: (Bool) -> Void - let onSelect: () -> Void + let onActivate: () -> Void let onClose: () -> Void let onCloseOthers: () -> Void let onCloseAll: () -> Void + let canKeepOpen: Bool + let onKeepOpen: () -> Void let canMoveLeft: Bool let canMoveRight: Bool let onMoveLeft: () -> Void @@ -301,8 +341,14 @@ private struct EditorTabStripItem: View { .frame(maxWidth: .infinity, maxHeight: .infinity) .contentShape(Rectangle()) .onHover(perform: onHover) - .help(Text(label.description)) + .help(Text(tooltip)) .contextMenu { + /// The double-click that keeps a tab is an editor idiom rather than a system one, so + /// it needs a command beside it: a gesture with no menu equivalent cannot be found by + /// a user who does not already expect it, and cannot be performed at all by VoiceOver. + Button(String(localized: "Keep Open"), action: onKeepOpen) + .disabled(!canKeepOpen) + Divider() Button(String(localized: "Close Tab"), action: onClose) Button(String(localized: "Close Other Tabs"), action: onCloseOthers) Button(String(localized: "Close All Tabs"), action: onCloseAll) @@ -322,6 +368,13 @@ private struct EditorTabStripItem: View { .accessibilityValue(Text(positionDescription)) .accessibilityAddTraits(isSelected ? .isSelected : []) .accessibilityAction(named: Text("Close Tab"), onClose) + /// Offered only where it does something, so the actions rotor matches the contextual menu + /// rather than announcing a command that silently does nothing on a tab already kept. + .accessibilityActions { + if canKeepOpen { + Button(String(localized: "Keep Open"), action: onKeepOpen) + } + } .accessibilityAction(named: Text("Move Tab Left")) { if canMoveLeft { onMoveLeft() } } .accessibilityAction(named: Text("Move Tab Right")) { if canMoveRight { onMoveRight() } } } @@ -336,7 +389,7 @@ private struct EditorTabStripItem: View { /// label never receives the click. private var surface: some View { ZStack { - Button(action: onSelect) { title } + Button(action: onActivate) { title } .buttonStyle(.plain) HStack(spacing: 0) { @@ -394,10 +447,26 @@ private struct EditorTabStripItem: View { } } + /// Carries the preview state, because the italic title cannot: an assistive technology is told + /// the string, never the face it is set in, and the HIG asks that no interface rely on a single + /// method to convey a change in state. private var positionDescription: String { - let place = String(format: String(localized: "%1$d of %2$d"), position, count) - guard tab.execution.finishedUnseenAt != nil, !isSelected else { return place } - return String(format: String(localized: "%@, finished"), place) + var description = String(format: String(localized: "%1$d of %2$d"), position, count) + if tab.isPreview { + description = String(format: String(localized: "%@, preview tab"), description) + } + guard tab.execution.finishedUnseenAt != nil, !isSelected else { return description } + return String(format: String(localized: "%@, finished"), description) + } + + /// The pointer's route to the same state the title's italic carries, and the only place the + /// gesture that keeps the tab is named outside the context menu. + private var tooltip: String { + guard tab.isPreview else { return label.description } + return String( + format: String(localized: "%@\nPreview tab. Double-click to keep it open."), + label.description + ) } /// The system draws both labels in the same face at the same size and separates them by colour diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift index 45cb6d889..282fb5547 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift @@ -392,9 +392,8 @@ extension MainContentCoordinator { } func promotePreviewTab() { - guard let (tab, tabIndex) = tabManager.selectedTabAndIndex, - tab.isPreview else { return } - tabManager.mutate(at: tabIndex) { $0.isPreview = false } + guard let selectedTabId = tabManager.selectedTabId else { return } + tabManager.promotePreviewTab(id: selectedTabId) } func showAllTablesMetadata() { diff --git a/TableProTests/Models/PreviewTabTests.swift b/TableProTests/Models/PreviewTabTests.swift index 2102b8951..8f6f421d4 100644 --- a/TableProTests/Models/PreviewTabTests.swift +++ b/TableProTests/Models/PreviewTabTests.swift @@ -103,4 +103,75 @@ struct PreviewTabTests { let payload = EditorTabPayload(connectionId: UUID(), isPreview: true) #expect(payload.isPreview == true) } + + // MARK: - Keeping a preview tab (issue #2436) + + @Test("promotePreviewTab keeps a tab that is not the selected one") + @MainActor + func promoteKeepsAnUnselectedTab() throws { + let manager = QueryTabManager() + try manager.addTableTab(tableName: "users", databaseType: .mysql, databaseName: "mydb", isPreview: true) + let previewTabId = try #require(manager.selectedTabId) + try manager.addTableTab(tableName: "orders", databaseType: .mysql, databaseName: "mydb") + + manager.promotePreviewTab(id: previewTabId) + + #expect(manager.tabs.first { $0.id == previewTabId }?.isPreview == false) + #expect(manager.selectedTabId != previewTabId) + } + + @Test("promotePreviewTab is a no-op on a tab that is already permanent") + @MainActor + func promoteIsANoOpOnAPermanentTab() throws { + let manager = QueryTabManager() + try manager.addTableTab(tableName: "users", databaseType: .mysql, databaseName: "mydb") + let tabId = try #require(manager.selectedTabId) + + manager.promotePreviewTab(id: tabId) + + #expect(manager.tabs.first { $0.id == tabId }?.isPreview == false) + } + + @Test("promotePreviewTab ignores an id no tab has") + @MainActor + func promoteIgnoresAnUnknownId() throws { + let manager = QueryTabManager() + try manager.addTableTab(tableName: "users", databaseType: .mysql, databaseName: "mydb", isPreview: true) + + manager.promotePreviewTab(id: UUID()) + + #expect(manager.selectedTab?.isPreview == true) + } + + /// Keeping a tab is not pinning: the tab holds its place in the strip. + @Test("promotePreviewTab does not reorder the strip") + @MainActor + func promoteDoesNotReorderTheStrip() throws { + let manager = QueryTabManager() + try manager.addTableTab(tableName: "users", databaseType: .mysql, databaseName: "mydb", isPreview: true) + let previewTabId = try #require(manager.selectedTabId) + try manager.addTableTab(tableName: "orders", databaseType: .mysql, databaseName: "mydb") + let orderBefore = manager.tabs.map(\.id) + + manager.promotePreviewTab(id: previewTabId) + + #expect(manager.tabs.map(\.id) == orderBefore) + } + + @Test("canPromotePreviewTab answers for a preview tab, a permanent tab and an unknown id") + @MainActor + func canPromoteAnswersEachCase() throws { + let manager = QueryTabManager() + try manager.addTableTab(tableName: "users", databaseType: .mysql, databaseName: "mydb", isPreview: true) + let previewTabId = try #require(manager.selectedTabId) + try manager.addTableTab(tableName: "orders", databaseType: .mysql, databaseName: "mydb") + let permanentTabId = try #require(manager.selectedTabId) + + #expect(manager.canPromotePreviewTab(id: previewTabId)) + #expect(manager.canPromotePreviewTab(id: permanentTabId) == false) + #expect(manager.canPromotePreviewTab(id: UUID()) == false) + + manager.promotePreviewTab(id: previewTabId) + #expect(manager.canPromotePreviewTab(id: previewTabId) == false) + } } diff --git a/TableProTests/Views/Main/EditorTabActivationTests.swift b/TableProTests/Views/Main/EditorTabActivationTests.swift new file mode 100644 index 000000000..df6a49827 --- /dev/null +++ b/TableProTests/Views/Main/EditorTabActivationTests.swift @@ -0,0 +1,134 @@ +// +// EditorTabActivationTests.swift +// TableProTests +// + +import AppKit +import Foundation +import Testing + +@testable import TablePro + +@Suite("Editor Tab Activation") +struct EditorTabActivationTests { + private static func resolve( + clickCount: Int, + hasModifiers: Bool = false, + sameTab: Bool = true + ) -> EditorTabActivation { + let tabId = UUID() + return EditorTabActivationResolver.resolve( + click: EditorTabClick(clickCount: clickCount, hasModifiers: hasModifiers), + tabId: tabId, + lastActivatedTabId: sameTab ? tabId : UUID() + ) + } + + @Test("A single click selects") + func singleClickSelects() { + #expect(Self.resolve(clickCount: 1) == .select) + } + + @Test("A second click on the same tab keeps it open") + func doubleClickKeepsTheTab() { + #expect(Self.resolve(clickCount: 2) == .selectAndKeep) + } + + /// Two clicks close enough in time and space arrive as one click of count two whichever view + /// each of them hit, and tabs sit flush against each other. Without this the pair would keep a + /// tab the user only meant to select. + @Test("A second click on a different tab only selects") + func doubleClickAcrossTabsOnlySelects() { + #expect(Self.resolve(clickCount: 2, sameTab: false) == .select) + } + + @Test("The first click of a session only selects") + func firstClickOfASessionSelects() { + #expect( + EditorTabActivationResolver.resolve( + click: EditorTabClick(clickCount: 2, hasModifiers: false), + tabId: UUID(), + lastActivatedTabId: nil + ) == .select + ) + } + + @Test("A modified click never keeps the tab") + func modifiedClickOnlySelects() { + #expect(Self.resolve(clickCount: 2, hasModifiers: true) == .select) + } + + /// A double-click that follows a nearby click arrives as counts two and three, so refusing + /// the three would leave a genuine double-click doing nothing. Keeping is idempotent, so + /// acting on it costs nothing. + @Test("A count above two still keeps the tab") + func tripleClickStillKeepsTheTab() { + #expect(Self.resolve(clickCount: 3) == .selectAndKeep) + } + + @Test("A count above two on a different tab still only selects") + func tripleClickAcrossTabsOnlySelects() { + #expect(Self.resolve(clickCount: 3, sameTab: false) == .select) + } + + /// The keyboard and VoiceOver both activate the tab's button with no mouse event current. + @Test("An activation with no click behind it only selects") + func activationWithoutAClickSelects() { + #expect( + EditorTabActivationResolver.resolve(click: nil, tabId: UUID(), lastActivatedTabId: UUID()) == .select + ) + } + + // MARK: - Reading the click off an NSEvent + + private static func mouseEvent(_ type: NSEvent.EventType, clickCount: Int, modifiers: NSEvent.ModifierFlags = []) -> NSEvent? { + NSEvent.mouseEvent( + with: type, + location: .zero, + modifierFlags: modifiers, + timestamp: 0, + windowNumber: 0, + context: nil, + eventNumber: 0, + clickCount: clickCount, + pressure: 0 + ) + } + + @Test("A left mouse-up carries its click count") + func leftMouseUpCarriesClickCount() throws { + let event = try #require(Self.mouseEvent(.leftMouseUp, clickCount: 2)) + let click = try #require(EditorTabClick(event: event)) + #expect(click.clickCount == 2) + #expect(click.hasModifiers == false) + } + + @Test("A control-click reads as modified, so it never keeps the tab") + func controlClickReadsAsModified() throws { + let event = try #require(Self.mouseEvent(.leftMouseDown, clickCount: 2, modifiers: .control)) + let click = try #require(EditorTabClick(event: event)) + #expect(click.hasModifiers) + } + + /// Caps lock says nothing about intent, and leaving it out stops a stuck key disabling the + /// gesture outright. + @Test("Caps lock does not count as a modifier") + func capsLockIsNotAModifier() throws { + let event = try #require(Self.mouseEvent(.leftMouseUp, clickCount: 2, modifiers: .capsLock)) + let click = try #require(EditorTabClick(event: event)) + #expect(click.hasModifiers == false) + } + + /// `NSEvent.clickCount` raises for anything that is not a mouse-down or mouse-up, so the type + /// has to be checked before it is read. + @Test("A right-click is not a tab activation") + func rightClickIsNotAnActivation() throws { + let event = try #require(Self.mouseEvent(.rightMouseDown, clickCount: 2)) + #expect(EditorTabClick(event: event) == nil) + } + + @Test("No current event is not a click") + func noEventIsNotAClick() { + #expect(EditorTabClick(event: nil) == nil) + } +} diff --git a/TableProTests/Views/Main/EditorTabStripGestureConventionTests.swift b/TableProTests/Views/Main/EditorTabStripGestureConventionTests.swift new file mode 100644 index 000000000..32fe26a9c --- /dev/null +++ b/TableProTests/Views/Main/EditorTabStripGestureConventionTests.swift @@ -0,0 +1,74 @@ +// +// EditorTabStripGestureConventionTests.swift +// TableProTests +// +// The tab strip promotes a preview tab on a double-click by reading the click count off the event +// AppKit is dispatching, never by composing a SwiftUI tap gesture. That is not a style preference, +// it is the only shape that measured acceptable. Driven with CGEvents posted to .cghidEventTap so +// the window server assigned the click count itself, against a Button carrying the strip's own +// shape, with NSEvent.doubleClickInterval at its 0.5s default: +// +// Button alone (the shipping strip) single: select +33ms double: select, select +// + .onTapGesture(count: 2) single: select +371ms double: PROMOTE only +// + .simultaneousGesture(TapGesture(count: 2)) single: select +26ms double: select, PROMOTE, select +// + reading NSApp.currentEvent.clickCount single: select +22ms double: select, select, PROMOTE +// +// So a count:2 tap gesture holds every selection back 371ms and drops it entirely on the double, +// and a simultaneous one selects twice. Neither is visible to any other test: the tab still +// selects and still promotes, just late, or twice. Hence this guard. +// + +import Foundation +import Testing + +@Suite("Editor tab strip gesture convention") +struct EditorTabStripGestureConventionTests { + private static let repositoryRoot: URL = { + var url = URL(fileURLWithPath: #filePath) + for _ in 0 ..< 4 { + url.deleteLastPathComponent() + } + return url + }() + + /// One spelling covers both, because `onTapGesture(count:` contains `TapGesture(count:`. + /// Listing them separately made a single offence report twice. + private static let bannedGestures = ["TapGesture(count:"] + + /// Comments are dropped before the scan, because the file documents the measurement above by + /// naming the very spellings this test bans. + private func code(of source: String) -> String { + source + .split(separator: "\n", omittingEmptySubsequences: false) + .filter { !$0.trimmingCharacters(in: .whitespaces).hasPrefix("//") } + .joined(separator: "\n") + } + + @Test("The tab strip composes no multi-click SwiftUI tap gesture") + func stripUsesNoMultiClickTapGesture() throws { + let url = Self.repositoryRoot.appendingPathComponent("TablePro/Views/Main/EditorTabStrip.swift") + let source = code(of: try String(contentsOf: url, encoding: .utf8)) + + let offenders = Self.bannedGestures.filter { source.contains($0) } + + #expect( + offenders.isEmpty, + """ + EditorTabStrip.swift uses \(offenders.joined(separator: ", ")). A multi-click SwiftUI \ + tap gesture delays every tab selection by ~371ms and suppresses it on the double-click. \ + Resolve the click through EditorTabActivationResolver instead. + """ + ) + } + + /// A scan that stops matching anything is a test that passes forever. This pins both halves: + /// a real call is still caught, and the comment that documents it is still ignored. + @Test("The scan catches a real gesture and ignores one named in a comment") + func scanCatchesCodeButNotComments() { + let withCall = code(of: " .onTapGesture(count: 2) { keep() }") + #expect(Self.bannedGestures.contains { withCall.contains($0) }) + + let withComment = code(of: " /// Never .onTapGesture(count: 2), it costs 371ms.") + #expect(Self.bannedGestures.contains { withComment.contains($0) } == false) + } +} diff --git a/TableProTests/Views/Main/OpenTableTabTests.swift b/TableProTests/Views/Main/OpenTableTabTests.swift index fb1a2541f..93b876d2b 100644 --- a/TableProTests/Views/Main/OpenTableTabTests.swift +++ b/TableProTests/Views/Main/OpenTableTabTests.swift @@ -279,6 +279,33 @@ struct OpenTableTabTests { #expect(coordinator.tabManager.selectedTab?.isPreview == false) } + /// The whole point of keeping a tab: the next table opened from the sidebar gets one of its + /// own instead of taking this one over. (#2436) + @Test("A kept tab is not reused by the next table opened from the sidebar") + @MainActor + func keptTabIsNotReusedByTheNextOpen() throws { + let connection = TestFixtures.makeConnection(database: "db_a") + let tabManager = QueryTabManager() + let coordinator = MainContentCoordinator( + connection: connection, + tabManager: tabManager, + changeManager: DataChangeManager(), + toolbarState: ConnectionToolbarState() + ) + defer { coordinator.teardown() } + + try tabManager.addTableTab( + tableName: "users", databaseType: connection.type, databaseName: "db_a", isPreview: true + ) + let keptTabId = try #require(tabManager.selectedTabId) + #expect(coordinator.isActiveTabReusable) + + tabManager.promotePreviewTab(id: keptTabId) + + #expect(coordinator.isActiveTabReusable == false) + #expect(tabManager.tabs.first { $0.id == keptTabId }?.tableContext.tableName == "users") + } + @Test("Double-click (forceNonPreview) replaces the preview tab with a permanent tab") @MainActor func forceNonPreviewReplacesWithPermanentTab() throws { diff --git a/TableProUITests/EditorTabKeepOpenUITests.swift b/TableProUITests/EditorTabKeepOpenUITests.swift new file mode 100644 index 000000000..2174d048a --- /dev/null +++ b/TableProUITests/EditorTabKeepOpenUITests.swift @@ -0,0 +1,112 @@ +import AppKit +import XCTest + +/// Issue #2436. A table already open in a preview tab could only be kept by closing it and +/// double-clicking it again in the sidebar. Double-clicking the tab itself now keeps it, the way +/// TablePlus, VS Code, DataGrip and Xcode all do. +final class EditorTabKeepOpenUITests: UITestCase { + func testDoubleClickingAPreviewTabKeepsItSoTheNextTableGetsItsOwn() throws { + let app = try launchWithSampleDatabase() + let window = try readyWindow(of: app) + + /// Two tables, so the strip is drawn at all: it stays hidden while a connection holds one + /// tab. Album is kept first so Artist lands in the preview tab that this test promotes. + doubleClick(row("Album", in: window)) + click(row("Artist", in: window)) + /// Counted only once the strip is up. Sampling before the wait reads zero on a strip that + /// has not been drawn, and `> beforeKeeping` then passes without the feature running. + XCTAssertTrue( + waitForPredicate(timeout: 20) { self.tab(named: "Artist", in: window).exists }, + "The strip must show the preview tab for Artist" + ) + let beforeKeeping = tabCount(in: window) + + doubleClick(tab(named: "Artist", in: window)) + click(row("Customer", in: window)) + + XCTAssertTrue( + waitForPredicate(timeout: 20) { self.tabCount(in: window) > beforeKeeping }, + """ + Double-clicking a preview tab keeps it, so opening a third table must add a tab rather \ + than take the kept one over. Tabs before: \(beforeKeeping), after: \ + \(tabCount(in: window)). + """ + ) + } + + func testASingleClickOnAPreviewTabDoesNotKeepIt() throws { + let app = try launchWithSampleDatabase() + let window = try readyWindow(of: app) + + doubleClick(row("Album", in: window)) + click(row("Artist", in: window)) + XCTAssertTrue( + waitForPredicate(timeout: 20) { self.tab(named: "Artist", in: window).exists }, + "The strip must show the preview tab for Artist" + ) + let beforeSelecting = tabCount(in: window) + + /// Selecting the tab it is already on: the preview tab must stay disposable, or every + /// click in the strip would quietly keep a tab. + click(tab(named: "Artist", in: window)) + click(row("Customer", in: window)) + + XCTAssertTrue( + waitForPredicate(timeout: 20) { self.tabCount(in: window) == beforeSelecting }, + """ + A single click only selects, so the preview tab must still be reused by the next table. \ + Tabs before: \(beforeSelecting), after: \(tabCount(in: window)). + """ + ) + } + + private func readyWindow(of app: XCUIApplication) throws -> XCUIElement { + let window = app.windows.firstMatch + XCTAssertTrue(window.waitToExist(timeout: 30)) + XCTAssertTrue( + waitForPredicate(timeout: 30) { window.outlines.firstMatch.outlineRows.count > 1 }, + "The object browser must list the sample database's tables" + ) + return window + } + + /// The tree draws its rows as hosted cells, so the name arrives as the static text's `value` + /// rather than as a label or an identifier. + private func row(_ name: String, in window: XCUIElement) -> XCUIElement { + let match = window.outlines.firstMatch.staticTexts + .matching(NSPredicate(format: "value == %@", "Table: \(name)")) + .firstMatch + XCTAssertTrue(match.waitToExist(timeout: 20), "The object browser must list \(name)") + return match + } + + private func tab(named name: String, in window: XCUIElement) -> XCUIElement { + window.descendants(matching: .any) + .matching(identifier: "editor-tab") + .matching(NSPredicate(format: "label == %@", name)) + .firstMatch + } + + private func click(_ element: XCUIElement) { + clickAtCenter(element) + settleBetweenClicks() + } + + private func doubleClick(_ element: XCUIElement) { + element.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)).doubleClick() + settleBetweenClicks() + } + + /// A genuine delay, not a poll: there is no state to observe between two clicks, and the only + /// thing being waited out is the window in which macOS would coalesce the next click into this + /// one. + private func settleBetweenClicks() { + Thread.sleep(forTimeInterval: NSEvent.doubleClickInterval) + } + + /// The strip is drawn only once a connection holds more than one tab, so one tab reads as zero + /// here. The tests compare counts rather than read a total, which holds either way. + private func tabCount(in window: XCUIElement) -> Int { + window.descendants(matching: .any).matching(identifier: "editor-tab").count + } +} diff --git a/docs/customization/general-settings.mdx b/docs/customization/general-settings.mdx index 01a9755bd..c5fccae50 100644 --- a/docs/customization/general-settings.mdx +++ b/docs/customization/general-settings.mdx @@ -22,7 +22,7 @@ System (the default), English, Tiếng Việt, 简体中文, 繁體中文, 한 | Setting | Default | What it does | |---------|---------|--------------| -| **Enable preview tabs** | On | Single-click opens a temporary tab that the next click replaces; double-click or an edit makes it permanent | +| **Enable preview tabs** | On | Single-click opens a temporary tab that the next click replaces; double-clicking the table, the tab, or editing it makes it permanent | | **Show connections** | On | The [connections strip](/features/workspace-rail) on the window's leading edge | | **Show recent tables** | Off | A [Recent section](/features/favorites#recent-tables) at the top of the sidebar | | **Show object icons** | On | A type icon before each object name. Off gives a plain list of names | diff --git a/docs/features/tabs.mdx b/docs/features/tabs.mdx index 13c817bcb..241d358c5 100644 --- a/docs/features/tabs.mdx +++ b/docs/features/tabs.mdx @@ -40,6 +40,8 @@ Both stage cell edits as pending changes. A query tab does it only when its quer Clicking a table opens it immediately in a preview tab, which the next click reuses. Double-click a table, or select it and press `Return`, to keep its tab: the next table then opens in one of its own, so double-clicking down a list gives you a tab each. Double-clicking a table already open switches to its tab. +A preview tab's title is italic, which marks the tab the next click takes over. The strip appears once a window holds more than one tab; from there, double-click a preview tab or choose **Keep Open** from its contextual menu to keep it. It holds its place in the strip, and the next table opens in a tab of its own. + A preview tab turns permanent as soon as you sort it, filter it, or edit data, and a tab in any of those states is never replaced by a click. One still open at quit comes back permanent, and a table opened from Favorites is permanent from the start. Turn the behavior off in **Settings > General > Tabs > Enable preview tabs**. ## Two tabs on one table