From 70f8d9e847ad7f6a08bb790eaea8dcf48aa6e233 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 15 Sep 2026 18:47:57 +0700 Subject: [PATCH] revert(editor): put query history back in the bottom drawer --- CHANGELOG.md | 2 - .../MainSplitViewController.swift | 50 +------------------ .../Infrastructure/TrailingPaneProxy.swift | 6 --- .../Infrastructure/WorkspacePanes.swift | 8 +-- TablePro/Models/UI/TrailingPaneState.swift | 9 ---- TablePro/Models/UI/TrailingPaneSurface.swift | 7 --- .../TrailingPaneUnavailableView.swift | 2 - .../Editor/History/HistoryPanelView.swift | 19 ++----- .../Main/Child/MainEditorContentView.swift | 29 ++++++++--- .../Main/MainContentCommandActions.swift | 7 ++- docs/features/keyboard-shortcuts.mdx | 4 +- docs/features/query-history.mdx | 22 ++++---- docs/features/query-insights.mdx | 2 +- 13 files changed, 45 insertions(+), 122 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba027b98e5..94f494d21f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,7 +55,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Connection rows without colored dots, on the Mac and on iOS. - SQL Server sessions open with the ANSI SET profile the server requires, matching every other client. - Compared columns in data Compare & Sync chosen per table, and saved with each table's key, filter and row limit. (#2537) -- Query history opens in the trailing pane beside the inspector and the assistant, with its entry list above its detail. - Query editor command bar with one control size, the container picker leading and the commands trailing. ### Removed @@ -64,7 +63,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Ctrl+Cmd+J` from the editor's reserved shortcuts, so it can be bound in Settings > Keyboard. - Result tab strip above the query results, and the "Query" heading above the editor. - Trash button that cleared the query and the results under one name. -- Query history drawer under the editor and results. ### Fixed diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift index a7241b9a4f..f1b3c867f8 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController.swift @@ -333,10 +333,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan super.splitViewDidResizeSubviews(notification) recomputeWindowMinSize() toolbarOwner?.syncSidebarSelection() - /// A divider drag can collapse the trailing pane without going through `hideTrailingPane`, - /// and history's activation is keyed on the flag rather than on the pane, so it has to be - /// reconciled here as well or the panel keeps querying behind a collapsed divider. - syncHistoryPanelVisibility() } override func viewWillAppear() { @@ -721,7 +717,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan workspace.panes.detail.rootView = AnyView(buildDetailView(for: workspace)) workspace.panes.inspector.rootView = AnyView(buildInspectorView(for: workspace)) workspace.panes.assistant.rootView = AnyView(buildAssistantView(for: workspace)) - workspace.panes.history.rootView = AnyView(buildHistoryView(for: workspace)) refreshTabStripPane(of: workspace) workspace.panes.markRendered(workspace.paneRenderKey) guard isShowing(workspace) else { return } @@ -902,16 +897,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan } } - @ViewBuilder - private func buildHistoryView(for workspace: ConnectionWorkspace) -> some View { - if workspace.resolvedPane == .content, - let coordinator = workspace.sessionState?.coordinator { - HistoryPanelView(coordinator: coordinator) - } else { - TrailingPaneUnavailableView(surface: .history) - } - } - /// Rebuilds the trailing surfaces alone. `commandActions` is read eagerly by both, and it only /// exists once the detail pane has appeared, which is after `rebuildPanes()` has already built /// them against a nil value. Rebuilding the detail pane too would remount the very view that @@ -920,7 +905,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan guard let selected = workspaces.selected else { return } selected.panes.inspector.rootView = AnyView(buildInspectorView(for: selected)) selected.panes.assistant.rootView = AnyView(buildAssistantView(for: selected)) - selected.panes.history.rootView = AnyView(buildHistoryView(for: selected)) } /// Parents whichever surface the selected workspace is showing. @@ -939,11 +923,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan isAIEnabled: AppSettingsManager.shared.ai.enabled ) inspectorPaneHost.show(selected.panes.trailingPane(for: surface)) - /// The single reconciliation point. Every route that changes which surface the pane shows, - /// or which workspace it belongs to, ends here: a reveal, a hide, a workspace switch and an - /// availability change all call this, so the flag cannot describe a different connection's - /// pane than the one on screen. - syncHistoryPanelVisibility() } // MARK: - Session Bindings @@ -1027,10 +1006,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan isTrailingPaneOpen && resolvedTrailingSurface == .assistant } - var isHistoryVisible: Bool { - isTrailingPaneOpen && resolvedTrailingSurface == .history - } - func showInspector() { reveal(.inspector) } @@ -1040,41 +1015,19 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan reveal(.assistant) } - func showHistory() { - reveal(.history) - } - /// Auto-show follows a grid click, which is not a request for a different surface. Revealing /// the inspector unconditionally swapped the assistant out from under a half-typed question and /// persisted the inspector as that connection's surface, on every row the user clicked. func revealInspectorForSelection() { - /// History is deliberately opened, the same way the assistant is, so a row click must not - /// take it away and persist the inspector as this connection's surface behind it. - guard !isAssistantVisible, !isHistoryVisible else { return } + guard !isAssistantVisible else { return } showInspector() } func hideTrailingPane() { inspectorSplitItem?.animator().isCollapsed = true - syncHistoryPanelVisibility() recomputeWindowMinSize() } - /// Keeps `HistoryPanelState.isVisible` saying what the window is actually showing. - /// - /// The flag is not redundant with the surface. It is what `HistoryPanelView`'s `.task(id:)` - /// keys its activation on, so the view model only builds and starts querying while history is - /// on screen, and it is what the Find Past Queries tip reads to know it has been answered. - /// Leaving it behind when history moved into the trailing pane would have left the panel - /// mounted and inert, which is the shape of a pane that renders nothing forever. - internal func syncHistoryPanelVisibility() { - guard isViewLoaded, let connectionId = workspaces.selected?.connectionId else { return } - let showing = isTrailingPaneOpen && resolvedTrailingSurface == .history - let state = HistoryPanelState.forConnection(connectionId) - guard state.isVisible != showing else { return } - state.isVisible = showing - } - /// Puts the hosted child back in step with what the settings now allow. /// /// The stored surface is left alone: a user who turns the assistant off and on again gets it @@ -1091,7 +1044,6 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan rebuildTrailingPanes() showSelectedTrailingPane() inspectorSplitItem?.animator().isCollapsed = false - syncHistoryPanelVisibility() recomputeWindowMinSize() } diff --git a/TablePro/Core/Services/Infrastructure/TrailingPaneProxy.swift b/TablePro/Core/Services/Infrastructure/TrailingPaneProxy.swift index 50fd9c4883..890d1e67ed 100644 --- a/TablePro/Core/Services/Infrastructure/TrailingPaneProxy.swift +++ b/TablePro/Core/Services/Infrastructure/TrailingPaneProxy.swift @@ -17,10 +17,8 @@ import Foundation internal protocol TrailingPaneProxy: AnyObject { var isInspectorVisible: Bool { get } var isAssistantVisible: Bool { get } - var isHistoryVisible: Bool { get } func showInspector() func showAssistant() - func showHistory() func hideTrailingPane() /// Reveals the inspector for a selection the user made somewhere else, and only if that does @@ -39,8 +37,4 @@ internal extension TrailingPaneProxy { func toggleAssistant() { if isAssistantVisible { hideTrailingPane() } else { showAssistant() } } - - func toggleHistory() { - if isHistoryVisible { hideTrailingPane() } else { showHistory() } - } } diff --git a/TablePro/Core/Services/Infrastructure/WorkspacePanes.swift b/TablePro/Core/Services/Infrastructure/WorkspacePanes.swift index 4b45f9802b..f7aadc13ff 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspacePanes.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspacePanes.swift @@ -53,10 +53,6 @@ internal final class WorkspacePanes { /// only way it gets the `sizingOptions` firewall below, which is applied here and nowhere else. internal let assistant: NSHostingController - /// Query history, on the same terms as the assistant: its own controller so the list's scroll - /// position, the selected entry and a half-typed search survive the reader looking at a row. - internal let history: NSHostingController - internal let sidebar: NSHostingController /// The editor tab strip. It is a pane like the other three, built and kept alive per /// connection, even though the window shows it in the titlebar accessory rather than in a @@ -76,7 +72,6 @@ internal final class WorkspacePanes { detail = NSHostingController(rootView: AnyView(Color.clear)) inspector = NSHostingController(rootView: AnyView(Color.clear)) assistant = NSHostingController(rootView: AnyView(Color.clear)) - history = NSHostingController(rootView: AnyView(Color.clear)) sidebar = NSHostingController(rootView: AnyView(Color.clear)) tabStrip = EditorTabStripPaneController() for pane in panes { @@ -85,7 +80,7 @@ internal final class WorkspacePanes { } private var panes: [NSHostingController] { - [detail, inspector, assistant, history, sidebar] + [detail, inspector, assistant, sidebar] } /// The controller a trailing surface is drawn by. One split item hosts whichever of these the @@ -94,7 +89,6 @@ internal final class WorkspacePanes { switch surface { case .inspector: inspector case .assistant: assistant - case .history: history } } diff --git a/TablePro/Models/UI/TrailingPaneState.swift b/TablePro/Models/UI/TrailingPaneState.swift index 0799b0b2e7..40854b2ead 100644 --- a/TablePro/Models/UI/TrailingPaneState.swift +++ b/TablePro/Models/UI/TrailingPaneState.swift @@ -43,15 +43,6 @@ import os let raw = defaults.string(forKey: Self.surfaceKey(connectionId)), let stored = TrailingPaneSurface(rawValue: raw) { self.surface = stored - } else if let connectionId, HistoryPanelPreferencesStorage.load(for: connectionId).isVisible { - /// This connection last had the query history drawer open, and the drawer is now a - /// surface of this pane. Adopting it here is what carries that reader across the move - /// instead of silently closing their history on the first launch after upgrading. - /// - /// Deterministic without a migration flag: it is reachable only while no surface has - /// ever been stored for the connection, and storing one is what this initializer's - /// `didSet` does the first time anything changes it. - self.surface = .history } else { self.surface = .inspector } diff --git a/TablePro/Models/UI/TrailingPaneSurface.swift b/TablePro/Models/UI/TrailingPaneSurface.swift index 7b31c4164e..c8288b8e4c 100644 --- a/TablePro/Models/UI/TrailingPaneSurface.swift +++ b/TablePro/Models/UI/TrailingPaneSurface.swift @@ -20,18 +20,11 @@ import Foundation internal enum TrailingPaneSurface: String, CaseIterable, Hashable { case inspector case assistant - /// Query history, which used to be a third band stacked under the editor and the results. - /// - /// It is a peer of the other two by the same argument they are peers of each other: it is a - /// task surface no selection owns. Its panel stacks its list over its detail rather than - /// beside it, so it reads at the shared 270pt floor without moving it. - case history internal var localizedTitle: String { switch self { case .inspector: String(localized: "Inspector") case .assistant: String(localized: "Assistant") - case .history: String(localized: "Query History") } } diff --git a/TablePro/Views/Connection/TrailingPaneUnavailableView.swift b/TablePro/Views/Connection/TrailingPaneUnavailableView.swift index 2da5e3846a..e14e3b016f 100644 --- a/TablePro/Views/Connection/TrailingPaneUnavailableView.swift +++ b/TablePro/Views/Connection/TrailingPaneUnavailableView.swift @@ -33,8 +33,6 @@ internal struct TrailingPaneUnavailableView: View { return String(localized: "Row fields appear once the connection is up") case .assistant: return String(localized: "The assistant answers once the connection is up") - case .history: - return String(localized: "Past queries appear once the connection is up") } } } diff --git a/TablePro/Views/Editor/History/HistoryPanelView.swift b/TablePro/Views/Editor/History/HistoryPanelView.swift index e3145a586b..afeeeb15f1 100644 --- a/TablePro/Views/Editor/History/HistoryPanelView.swift +++ b/TablePro/Views/Editor/History/HistoryPanelView.swift @@ -53,24 +53,11 @@ struct HistoryPanelView: View { } } - /// List above, detail below. - /// - /// It was list beside detail, which needed 260 + 280 of width and so could only ever live in a - /// band across the bottom of the window. The trailing pane it moved into is one shared - /// `NSSplitViewItem` with a 270pt floor, and raising that floor was measured and rejected: it - /// force-grows the pane past the width the reader chose and takes the difference from the - /// content. Rotating the split is what makes the same two panes fit, and it costs nothing, - /// because a query is a tall thing to read and a list of them is a narrow one. - /// - /// The autosave name moves with the orientation. The stored value is a divider offset along the - /// axis, so restoring a horizontal position into a vertical split puts the divider somewhere - /// the reader never put it. private func panel(_ viewModel: HistoryPanelViewModel) -> some View { AutosavingSplitView( - autosaveName: "com.TablePro.queryHistory.listOverDetail", - isVertical: false, - primaryMinimum: 150, - secondaryMinimum: 180, + autosaveName: "com.TablePro.queryHistory.listDetail", + primaryMinimum: 260, + secondaryMinimum: 280, collapsesPrimaryWhenTight: false ) { HistoryListPane( diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 8623688d1e..bff8fbd59f 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -21,6 +21,11 @@ private struct TabLoadKey: Hashable { } struct MainEditorContentView: View { + /// A query tab nests its own editor/results split, whose two minimums are required constraints. + /// The drawer's own minimum has to clear their sum, or dragging the drawer down asks AppKit to + /// satisfy a height the content it contains cannot reach. + static let tabContentMinimumHeight = VerticalCollapsibleSplitView.combinedMinimumThickness + // MARK: - Dependencies var tabManager: QueryTabManager @@ -89,13 +94,25 @@ struct MainEditorContentView: View { var body: some View { @Bindable var historyState = HistoryPanelState.forConnection(connectionId) - return Group { - if let tab = tabManager.selectedTab { - tabContent(for: tab) - } else { - emptyStateView + return VerticalCollapsibleSplitView( + isBottomCollapsed: Binding( + get: { !historyState.isVisible }, + set: { historyState.isVisible = !$0 } + ), + autosaveName: SplitViewAutosaveName.historyDrawer(connectionId: connectionId), + topMinimumThickness: Self.tabContentMinimumHeight, + bottomMinimumThickness: 180, + topContent: { + if let tab = tabManager.selectedTab { + tabContent(for: tab) + } else { + emptyStateView + } + }, + bottomContent: { + HistoryPanelView(coordinator: coordinator) } - } + ) .background(.background) .onChange(of: historyState.isVisible, initial: true) { _, isVisible in if isVisible { diff --git a/TablePro/Views/Main/MainContentCommandActions.swift b/TablePro/Views/Main/MainContentCommandActions.swift index 6cb86e1907..23723a67e6 100644 --- a/TablePro/Views/Main/MainContentCommandActions.swift +++ b/TablePro/Views/Main/MainContentCommandActions.swift @@ -1308,11 +1308,10 @@ final class MainContentCommandActions { // MARK: - UI Operations (Group A — Called Directly) - /// History is a trailing-pane surface, so the command that shows it is the same shape as the - /// two beside it. `HistoryPanelState.isVisible` is written by the pane rather than here, which - /// is what keeps the flag describing what the window shows instead of racing it. func toggleHistoryPanel() { - coordinator?.trailingPaneProxy?.toggleHistory() + guard let connectionId = coordinator?.connectionId else { return } + let state = HistoryPanelState.forConnection(connectionId) + state.isVisible.toggle() } func toggleRightSidebar() { diff --git a/docs/features/keyboard-shortcuts.mdx b/docs/features/keyboard-shortcuts.mdx index 983d2bba31..bc4d4ab34b 100644 --- a/docs/features/keyboard-shortcuts.mdx +++ b/docs/features/keyboard-shortcuts.mdx @@ -232,9 +232,9 @@ With the sidebar focused, typing the first letters of an object's name jumps to | Pin result | `Cmd+Option+P` | | Close result | `Cmd+Shift+W` | -### Query history pane +### Query history drawer -With the pane open (`Cmd+Y`) and its entry list focused, `Return` loads the selected entry into the editor, `Delete` removes it, and `Cmd+C` copies its query. `Cmd+Y` hides the pane again. +With the drawer open (`Cmd+Y`) and its entry list focused, `Return` loads the selected entry into the editor, `Delete` removes it, and `Cmd+C` copies its query. `Cmd+Y` hides the drawer again. ### Inspector fields diff --git a/docs/features/query-history.mdx b/docs/features/query-history.mdx index a14505bdf4..9bb5052fab 100644 --- a/docs/features/query-history.mdx +++ b/docs/features/query-history.mdx @@ -3,22 +3,22 @@ title: Query History description: Every executed query is saved to a local SQLite database with full-text search --- -By default the pane shows only the SQL you wrote yourself. The SELECTs behind table browsing, the UPDATEs behind grid edits, the DDL behind a structure change and everything an MCP client ran are all recorded too, waiting behind the **Source** menu. +By default the drawer shows only the SQL you wrote yourself. The SELECTs behind table browsing, the UPDATEs behind grid edits, the DDL behind a structure change and everything an MCP client ran are all recorded too, waiting behind the **Source** menu. -Open it with `Cmd+Y` or **View > Show Query History**. There is also a **History** toolbar button, added through **View > Customize Toolbar…**. +Open the drawer with `Cmd+Y` or **View > Show Query History**. There is also a **History** toolbar button, added through **View > Customize Toolbar…**. - - Query history pane - Query history pane + + Query history drawer + Query history drawer -## The pane +## The drawer -History shares the window's trailing pane with the inspector and the assistant, so opening it shows it in place of whichever of those was there. Its entry list sits above its details and a divider between them resizes both. Width, filters and whether it was open are remembered per connection. +The drawer opens under the editor and its divider resizes it. Height, filters and whether it was open are remembered per connection. Entries group by day, newest first, under **Today**, **Yesterday**, or the date. A row carries the outcome, the query text, the database, the time it ran and how long it took. Under a millisecond reads `<1 ms`; a step whose duration was never measured reads `–` rather than `0 ms`. -Select a row and the lower half shows the full query, highlighted for the database it ran against, with its connection, database and schema, timestamp, duration, row count, source, and the error when it failed. Where the driver separated execution from transfer, the duration is listed as its parts instead: see [How long it took](/features/query-results#how-long-it-took). The keyboard stays in the list, so arrow keys keep moving. +Select a row and the right pane shows the full query, highlighted for the database it ran against, with its connection, database and schema, timestamp, duration, row count, source, and the error when it failed. Where the driver separated execution from transfer, the duration is listed as its parts instead: see [How long it took](/features/query-results#how-long-it-took). The keyboard stays in the list, so arrow keys keep moving. Recent queries also appear in [Open Quickly](/features/open-quickly). For the summary rather than the list, see [Query Insights](/features/query-insights). @@ -34,7 +34,7 @@ Recent queries also appear in [Open Quickly](/features/open-quickly). For the su Search matches partial words, so `cust` finds `customers`. Several words must all match but need not be adjacent: `select customers` finds `SELECT id, name FROM customers`. -**All Connections** searches everything you have, and rows then name their connection, so two databases both called `app` stay apart. **Reset Filters** puts the pane back to its defaults. +**All Connections** searches everything you have, and rows then name their connection, so two databases both called `app` stay apart. **Reset Filters** puts the drawer back to its defaults. ### Sources @@ -74,11 +74,11 @@ An entry belonging to another connection loads into a new tab in that connection ## Pausing -The pause button stops recording on this Mac, from every source: row edits, structure changes, imports and AI clients included. The pane says so until you resume. Pausing is local to the Mac you press it on and survives relaunch. +The pause button stops recording on this Mac, from every source: row edits, structure changes, imports and AI clients included. The drawer says so until you resume. Pausing is local to the Mac you press it on and survives relaunch. ## Clearing history -The trash button deletes exactly the entries the pane is listing. Anything the source, date, outcome or search filters are hiding stays, which at the default **My Queries** source spares table browsing, row edits, imports and AI queries. The confirmation names what it is about to delete. There is no undo. +The trash button deletes exactly the entries the drawer is listing. Anything the source, date, outcome or search filters are hiding stays, which at the default **My Queries** source spares table browsing, row edits, imports and AI queries. The confirmation names what it is about to delete. There is no undo. **Settings > Data > Query History > Clear History…** clears everything, for every connection. diff --git a/docs/features/query-insights.mdx b/docs/features/query-insights.mdx index 255ab0e481..634a5d5f3c 100644 --- a/docs/features/query-insights.mdx +++ b/docs/features/query-insights.mdx @@ -82,7 +82,7 @@ Two things stay apart: table and column names keep their capitalization, since ` | Source | Which parts of the app the queries came from | | Date | Last hour, today, last 7 days, last 4 weeks, all time | -Source defaults to **My Queries**, the SQL you wrote yourself, and uses the same source list as [query history](/features/query-history#sources). Date defaults to **Last 4 Weeks** rather than All Time, since "got slower than before" needs a before to compare against. There is no outcome filter. +Source defaults to **My Queries**, the SQL you wrote yourself, and uses the same source list as the [history drawer](/features/query-history#sources). Date defaults to **Last 4 Weeks** rather than All Time, since "got slower than before" needs a before to compare against. There is no outcome filter. Insights refreshes as you run queries. The refresh button is for when you want it now.