Skip to content

Commit 0e8c394

Browse files
datlechinclaude
andcommitted
fix(linux): unified-tabs audit cleanup (B1-B5+B8)
Five fixes from the post-a8f573fc audit. All observable in normal use. B1 — Window title now updates on tab switch. WorkspaceTabsChanged previously only persisted state. The title shows "<table> · <connection> · <driver>" when a Browse tab is active, so the previous tab's table name lingered after switching. Fix: introduce on_workspace_tabs_changed that does persist_workspace_state + refresh_window_title + sync_sidebar_selection in one shot. Single dispatcher handler so the trio stays in lockstep. B2 — Drag-reorder now persists. Only connect_selected_page_notify was wired, so dragging tabs into a new order didn't write workspace_state.json until the next other event (e.g. selecting a tab). Restart-then the order was wrong. Fix: also wire connect_pages_notify → AppMsg::WorkspaceTabsChanged. Insert/remove/reorder all flow through the same persist path. B3 — Overview "+" tab labelling. The AdwTabOverview "+" button entry-point synthesised an editor slot inline with the hardcoded label "Empty query"; append_editor_tab labels new tabs "Query 1", "Query 2", … via default_editor_tab_label. Same widget kind, two different labels depending on entry point. Fix: count existing editor tabs in the overview closure and use default_editor_tab_label(count + 1). Both paths now produce the same label. B4 — Sidebar selection now follows the active Browse tab. Switching from `users` tab to `orders` tab left the sidebar's highlighted row on whichever was last *clicked*, regardless of the active tab. Visually misleading. Fix: sync_sidebar_selection (called from on_workspace_tabs_changed) walks the listbox children, finds the row whose title + paired schema matches the active Browse tab, and calls listbox.select_row. No-op when active is an Editor tab (sidebar selection persists, matching user intuition: you switch to editor and back, sidebar state shouldn't reshuffle). select_row doesn't fire row-activated (user gesture only) so this can't recurse into SelectTable. B5 — "Replace current tab" from history with a Browse tab active. HistoryDialogOutput::ReplaceCurrentTabQuery → AppMsg::ReplaceActiveTabQuery → on_replace_active_tab_query previously checked if the active tab was an Editor; if not, silent no-op. User clicked "Replace" while browsing → nothing happened, no feedback. Fix: when active is Browse (or no tab), fall back to opening a new Editor tab with the query. Mirrors "Open in new tab" semantics but keeps the user's intent of running this query right now. B8 — History "Open/Replace" while disconnected. History is app-wide (Ctrl+H works disconnected to view past queries across sessions). But OpenHistoryQuery / ReplaceActiveTabQuery without an active connection silently created an invisible tab in the workspace_root that was never displayed. Fix: gate both OpenHistoryQuery and ReplaceActiveTabQuery on self.connected; show a "Connect to a database first" toast when the user clicks while disconnected. Deferred from the audit: B6 (F5 / Ctrl+F context-sensitivity) — design choice, separate fix B7 (read-only fan-out) — no path triggers a live read-only flip today B9 (double persist on selection change) — minor I/O wastage B10 (monotonic editor labels) — cosmetic; user retypes anyway Verification: - cargo clippy --workspace --all-targets -- -D warnings: zero warnings - cargo test --workspace --lib: 43 pass, 1 ignored - cargo fmt --all -- --check: clean - cargo build -p tablepro-app: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a8f573f commit 0e8c394

2 files changed

Lines changed: 80 additions & 9 deletions

File tree

linux/crates/app/src/ui/app/mod.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ impl SimpleComponent for App {
690690
AppMsg::FetchBrowsePage(tab_id) => self.fetch_browse_page(tab_id, sender),
691691
AppMsg::FetchBrowseColumns(tab_id) => self.fetch_browse_columns(tab_id, sender),
692692
AppMsg::FetchBrowseRowCount(tab_id) => self.fetch_browse_row_count(tab_id, sender),
693-
AppMsg::WorkspaceTabsChanged => self.persist_workspace_state(),
693+
AppMsg::WorkspaceTabsChanged => self.on_workspace_tabs_changed(),
694694
AppMsg::WorkspaceSchemaWordsChanged => self.rebuild_schema_buffer(),
695695
AppMsg::WorkspaceTabClosed(id) => self.close_workspace_tab_by_id(id, sender),
696696
AppMsg::CloseActiveWorkspaceTab => self.close_active_workspace_tab(sender),
@@ -757,9 +757,19 @@ impl SimpleComponent for App {
757757
AppMsg::EditorTabQueryChanged(id, text) => self.on_editor_tab_query_changed(id, text),
758758
AppMsg::ShowHistory => self.on_show_history(sender),
759759
AppMsg::OpenHistoryQuery(text) => {
760-
self.append_editor_tab(Some(text), sender);
760+
if self.connected {
761+
self.append_editor_tab(Some(text), sender);
762+
} else {
763+
self.show_toast(&crate::tr!("Connect to a database first to run SQL."));
764+
}
765+
}
766+
AppMsg::ReplaceActiveTabQuery(text) => {
767+
if self.connected {
768+
self.on_replace_active_tab_query(text, sender);
769+
} else {
770+
self.show_toast(&crate::tr!("Connect to a database first to run SQL."));
771+
}
761772
}
762-
AppMsg::ReplaceActiveTabQuery(text) => self.on_replace_active_tab_query(text),
763773
AppMsg::PollHealth => self.on_poll_health(),
764774
AppMsg::RefreshPage => self.on_refresh_active_tab(),
765775
AppMsg::ShowShortcuts => self.on_show_shortcuts(),

linux/crates/app/src/ui/app/workspace_tabs.rs

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,18 @@ impl App {
6969
glib::Propagation::Stop
7070
});
7171

72+
// Both selection-change AND any pages-list change (insert /
73+
// remove / drag-reorder) trigger persist + title-refresh.
74+
// Without connect_pages_notify, drag-reorder doesn't persist
75+
// until the next other event.
7276
let pages_sender = sender.clone();
7377
tab_view.connect_selected_page_notify(move |_| {
7478
pages_sender.input(AppMsg::WorkspaceTabsChanged);
7579
});
80+
let reorder_sender = sender.clone();
81+
tab_view.connect_pages_notify(move |_| {
82+
reorder_sender.input(AppMsg::WorkspaceTabsChanged);
83+
});
7684

7785
let inner = gtk::Box::builder().orientation(gtk::Orientation::Vertical).build();
7886
inner.append(&tab_bar);
@@ -88,6 +96,10 @@ impl App {
8896
// we build an SqlEditor inline, register the slot, and return
8997
// the page. Browse tabs aren't creatable from the overview
9098
// (they need a sidebar table target).
99+
// Overview "+" must return a real TabPage synchronously. We
100+
// construct an editor slot inline using the same label scheme
101+
// as `append_editor_tab` ("Query 1", "Query 2", …) so the two
102+
// entry points are visually indistinguishable.
91103
let workspace_tabs_for_create = self.workspace_tabs.clone();
92104
let tab_view_for_create = tab_view.clone();
93105
let schema_buffer_for_create = self.schema_buffer.clone();
@@ -104,7 +116,12 @@ impl App {
104116
SqlEditorOutput::QueryChanged(text) => AppMsg::EditorTabQueryChanged(tab_id, text),
105117
});
106118
let page = tab_view_for_create.append(editor.widget());
107-
let label = crate::tr!("Empty query");
119+
let editor_count = workspace_tabs_for_create
120+
.borrow()
121+
.values()
122+
.filter(|t| matches!(t, WorkspaceTab::Editor(_)))
123+
.count();
124+
let label = default_editor_tab_label(editor_count + 1);
108125
page.set_title(&label);
109126
page.set_tooltip(&label);
110127
write_workspace_tab_id(&page, tab_id);
@@ -495,6 +512,44 @@ impl App {
495512
workspace_state::save_connection(connection_id, conn_state);
496513
}
497514

515+
/// Single handler for `WorkspaceTabsChanged`. Persists tab state,
516+
/// refreshes the window title (so tab switches update the subtitle),
517+
/// and syncs the sidebar selection to the active Browse tab's table.
518+
pub(super) fn on_workspace_tabs_changed(&self) {
519+
self.persist_workspace_state();
520+
self.refresh_window_title();
521+
self.sync_sidebar_selection();
522+
}
523+
524+
/// Highlight the sidebar row matching the active Browse tab's
525+
/// `(schema, table)`. No-op when active is an Editor tab (we leave
526+
/// the sidebar selection on whatever was last clicked, so switching
527+
/// to editor and back doesn't disorient).
528+
fn sync_sidebar_selection(&self) {
529+
let Some((schema, table)) = self.selected_browse_slot_table() else {
530+
return;
531+
};
532+
let listbox = self.sidebar_factory.widget();
533+
let schemas = self.sidebar_schemas.borrow();
534+
let mut idx = 0_i32;
535+
while let Some(row) = listbox.row_at_index(idx) {
536+
// The factory builds one row per TableInfo, in the same order
537+
// as `sidebar_schemas`, so we can pair each row with its
538+
// schema-Option by index.
539+
if let Some(action_row) = row.downcast_ref::<adw::ActionRow>() {
540+
let row_table = action_row.title().to_string();
541+
let row_schema = schemas.get(idx as usize).cloned().unwrap_or(None);
542+
if row_table == table && row_schema.as_deref() == schema.as_deref() {
543+
// select_row doesn't trigger row-activated (user-only
544+
// signal), so this won't recurse into SelectTable.
545+
listbox.select_row(Some(action_row));
546+
return;
547+
}
548+
}
549+
idx += 1;
550+
}
551+
}
552+
498553
pub(super) fn selected_workspace_tab_id(&self) -> Option<Uuid> {
499554
let tab_view = self.workspace_tab_view.as_ref()?;
500555
let page = tab_view.selected_page()?;
@@ -594,13 +649,19 @@ impl App {
594649
}
595650
}
596651

597-
pub(super) fn on_replace_active_tab_query(&self, text: String) {
598-
let Some(id) = self.selected_workspace_tab_id() else {
599-
return;
600-
};
601-
if let Some(WorkspaceTab::Editor(slot)) = self.workspace_tabs.borrow().get(&id) {
652+
pub(super) fn on_replace_active_tab_query(&mut self, text: String, sender: ComponentSender<Self>) {
653+
// If an editor tab is active, replace its buffer in-place. If a
654+
// browse tab is active (or no tab at all), fall back to opening
655+
// a new editor tab with the query — silent no-op was the prior
656+
// (annoying) behaviour for users invoking from history while
657+
// browsing.
658+
if let Some(id) = self.selected_workspace_tab_id()
659+
&& let Some(WorkspaceTab::Editor(slot)) = self.workspace_tabs.borrow().get(&id)
660+
{
602661
let _ = slot.controller.sender().send(SqlEditorInput::ReplaceQuery(text));
662+
return;
603663
}
664+
self.append_editor_tab(Some(text), sender);
604665
}
605666

606667
fn editor_tab_count(&self) -> usize {

0 commit comments

Comments
 (0)