Skip to content

Commit 43b0b59

Browse files
datlechinclaude
andcommitted
fix(linux): UX bugs batch A — accidental edits, stale toast, Esc dismiss
Three bugs from the deep UI/UX audit, each with a real failure mode: - Cell edit was triggered by single click. In a database grid this is destructive — a misplaced click on the row a user wanted to select silently dropped them into an editable label. Switch to double-click, matching GNOME Files filename rename. Implementation: keep editable= false at rest so the EditableLabel's internal click→edit path is inert, flip to editable=true only on double-click via a GestureClick, and reset to false on editing-notify exit. - "Connecting…" was a fire-and-forget toast that auto-dismissed at 2 s, leaving remote / SSH-tunnelled connections with no visible feedback for the rest of their handshake. Replaced with a persistent toast (timeout 0) tracked on App and dismissed when the connect resolves (success in on_connected, failure in on_browse_load_failed/None). - Escape didn't reliably close the in-grid search bar. SearchBar's own Esc handler only fires while the SearchEntry has focus; clicking outside the bar (e.g. the page-size dropdown) put Esc out of reach. Added a Local-scope ShortcutController on the BrowseTab root that catches Escape anywhere in the tab and dismisses the bar when search-mode is enabled — matches Files / Text Editor. Audit also flagged sort-indicator restoration and Ctrl+F scoping as bugs; investigated both and they're false alarms (sort restore is guarded by columns.get + self-heals on later FetchPage; Ctrl+F is ShortcutScope::Global on the window so fires regardless of focus). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f8b4e62 commit 43b0b59

6 files changed

Lines changed: 82 additions & 9 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,14 @@ impl App {
175175
self.dispatch_to_tab(tab_id, BrowseTabInput::RowCountLoaded(count));
176176
}
177177

178-
pub(super) fn on_browse_load_failed(&self, tab_id: Option<Uuid>, msg: String) {
178+
pub(super) fn on_browse_load_failed(&mut self, tab_id: Option<Uuid>, msg: String) {
179179
match tab_id {
180180
Some(id) => self.dispatch_to_tab(id, BrowseTabInput::ShowError(msg)),
181181
None => {
182182
tracing::warn!(error = %msg, "app-level load failed");
183+
// Connect attempt failed → drop the in-progress toast so
184+
// the alert isn't competing with stale "Connecting…" UI.
185+
self.dismiss_loading_page();
183186
self.set_status_page(super::StatusKind::Error, &crate::tr!("Failed"), &msg);
184187
}
185188
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ impl App {
2626
}
2727

2828
pub(super) fn on_connected(&mut self, tables: Vec<TableInfo>, driver_id: String, sender: ComponentSender<Self>) {
29+
self.dismiss_loading_page();
2930
self.dialog = None;
3031
self.connected = true;
3132
self.current_driver_id = Some(driver_id.clone());
@@ -156,7 +157,7 @@ impl App {
156157
dialog.present(Some(&self.window));
157158
}
158159

159-
pub(super) fn on_open_saved(&self, saved: SavedConnection, sender: ComponentSender<Self>) {
160+
pub(super) fn on_open_saved(&mut self, saved: SavedConnection, sender: ComponentSender<Self>) {
160161
self.connections_popover.popdown();
161162
self.set_loading_page(
162163
&crate::tr!("Connecting…"),

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ pub struct App {
3939
sidebar_schemas: std::rc::Rc<std::cell::RefCell<Vec<Option<String>>>>,
4040
content_holder: adw::ToolbarView,
4141
toast_overlay: adw::ToastOverlay,
42+
/// Persistent "Connecting…" toast handle. Held so we can dismiss it
43+
/// when the connect attempt resolves (success or failure). Native
44+
/// alternative to a fire-and-forget 2 s toast that disappeared
45+
/// before the connection actually completed.
46+
connect_progress_toast: Option<adw::Toast>,
4247
reconnect_banner: adw::Banner,
4348
connections_factory: FactoryVecDeque<ConnectionRow>,
4449
connections_popover: gtk::Popover,
@@ -664,6 +669,7 @@ impl SimpleComponent for App {
664669
sidebar_schemas,
665670
content_holder: widgets.content_holder.clone(),
666671
toast_overlay: widgets.toast_overlay.clone(),
672+
connect_progress_toast: None,
667673
reconnect_banner: widgets.reconnect_banner.clone(),
668674
connections_factory,
669675
connections_popover: widgets.connections_popover.clone(),

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,29 @@ impl App {
1313
self.content_holder.set_content(Some(self.welcome_view.widget()));
1414
}
1515

16-
/// Used during connect to convey "Connecting…" — surfaces as a toast
17-
/// since the welcome view is still visible. Per-tab loading/error
18-
/// states live inside BrowseTab now (replace_status_child there).
19-
pub(super) fn set_loading_page(&self, title: &str, description: &str) {
20-
let _ = description;
21-
self.show_toast(title);
16+
/// Used during connect to convey "Connecting…". Persistent toast
17+
/// (timeout 0) — held in `connect_progress_toast` until the connect
18+
/// resolves, at which point `dismiss_loading_page` clears it. Replaces
19+
/// the prior fire-and-forget toast which auto-dismissed at 2 s, well
20+
/// before remote / SSH-tunnelled connections resolve.
21+
pub(super) fn set_loading_page(&mut self, title: &str, description: &str) {
22+
if let Some(prev) = self.connect_progress_toast.take() {
23+
prev.dismiss();
24+
}
25+
let body = if description.is_empty() {
26+
title.to_string()
27+
} else {
28+
format!("{title} {description}")
29+
};
30+
let toast = adw::Toast::builder().title(&body).timeout(0).build();
31+
self.toast_overlay.add_toast(toast.clone());
32+
self.connect_progress_toast = Some(toast);
33+
}
34+
35+
pub(super) fn dismiss_loading_page(&mut self) {
36+
if let Some(toast) = self.connect_progress_toast.take() {
37+
toast.dismiss();
38+
}
2239
}
2340

2441
/// Convenience for `set_status_page(Error, ...)` and similar; in the

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,30 @@ impl SimpleComponent for BrowseTab {
501501
root.add_bottom_bar(&paginator_bar);
502502
grid_search_bar.set_key_capture_widget(Some(&root));
503503

504+
// SearchBar's built-in Escape handler only fires while the
505+
// SearchEntry (or its close button) has focus. If the user opens
506+
// search and then clicks the page-size dropdown or any paginator
507+
// control, Esc lands outside the bar's reach. A Local-scope
508+
// shortcut on root catches Escape anywhere in this tab and
509+
// dismisses the bar — matching the GNOME Files / Text Editor
510+
// behaviour where Esc reliably closes search regardless of focus.
511+
let search_bar_for_esc = grid_search_bar.clone();
512+
let esc_shortcut = gtk::Shortcut::builder()
513+
.trigger(&gtk::ShortcutTrigger::parse_string("Escape").expect("valid trigger"))
514+
.action(&gtk::CallbackAction::new(move |_, _| {
515+
if search_bar_for_esc.is_search_mode() {
516+
search_bar_for_esc.set_search_mode(false);
517+
glib::Propagation::Stop
518+
} else {
519+
glib::Propagation::Proceed
520+
}
521+
}))
522+
.build();
523+
let esc_controller = gtk::ShortcutController::new();
524+
esc_controller.set_scope(gtk::ShortcutScope::Local);
525+
esc_controller.add_shortcut(esc_shortcut);
526+
root.add_controller(esc_controller);
527+
504528
// Per-tab GridMsg channel: events from this tab's grid (sort
505529
// change, cell edits, context-menu actions) flow into this tab's
506530
// own input queue, which then re-emits them as outputs to App

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,34 @@ fn build_column(
152152
return;
153153
};
154154
if editable_for_setup {
155+
// Double-click-to-edit, matching GNOME Files filename rename.
156+
// Single-click triggering edit is destructive in a database
157+
// grid: an accidental row-selection click would commit the
158+
// user into edit mode on the cell they happened to land on.
159+
// Pattern: keep editable=false at rest so the EditableLabel's
160+
// internal click→edit path is inert; flip true on double-click,
161+
// call start_editing(), and flip back to false on
162+
// editing-notify exit.
155163
let label = gtk::EditableLabel::builder()
156164
.xalign(0.0)
157165
.hexpand(true)
158166
.margin_start(8)
159167
.margin_end(8)
160168
.build();
161-
label.set_editable(true);
169+
label.set_editable(false);
162170
item.set_child(Some(&label));
163171

172+
let dbl_click = gtk::GestureClick::builder().button(gtk::gdk::BUTTON_PRIMARY).build();
173+
let label_for_dbl = label.clone();
174+
dbl_click.connect_pressed(move |gesture, n_press, _, _| {
175+
if n_press == 2 {
176+
gesture.set_state(gtk::EventSequenceState::Claimed);
177+
label_for_dbl.set_editable(true);
178+
label_for_dbl.start_editing();
179+
}
180+
});
181+
label.add_controller(dbl_click);
182+
164183
if let Some(ctx_sender) = context_sender.clone() {
165184
attach_context_menu(label.upcast_ref(), idx, context_table.clone(), ctx_sender);
166185
}
@@ -178,6 +197,9 @@ fn build_column(
178197
return;
179198
}
180199
label.remove_css_class("accent");
200+
// Reset to display-only so the next click won't enter
201+
// edit mode via EditableLabel's internal handler.
202+
label.set_editable(false);
181203
let Some(snap) = SNAPSHOT_SLOT.take(label) else {
182204
return;
183205
};

0 commit comments

Comments
 (0)