fix(linux): keyboard sort, named fields, focus hand-off, announcements

linux/crates/app/src/application/accelerators.rs

@@ -209,6 +209,9 @@ pub const ACCELERATORS: &[Accelerator] = &[
         &["Escape"],
         Listed(ShortcutSection::SqlEditor),
     ),
+    // GtkPaned's own binding, a signal rather than an action. Tab indents
+    // in the editor and Ctrl+Tab switches tabs, so this is its way out.
+    stock("paned.cycle-child-focus", &["F6"], Listed(ShortcutSection::SqlEditor)),
     local(
         SqlEditor,
         CAPTURE,

linux/crates/app/src/persistence/column_width_store.rs

@@ -57,6 +57,18 @@ impl ColumnWidthStore {
         });
     }
 
+    /// Drop the column's width, so it opens at its default again.
+    pub fn forget(&self, connection_id: Uuid, schema: Option<&str>, table: &str, column: &str) {
+        self.file.update(|document| {
+            document
+                .connections
+                .get_mut(&connection_id.to_string())
+                .and_then(|schemas| schemas.get_mut(schema_key(schema)))
+                .and_then(|tables| tables.get_mut(table))
+                .is_some_and(|columns| columns.remove(column).is_some())
+        });
+    }
+
     /// Drop everything for a connection the user deleted, so the file
     /// does not grow with entries nothing can reach.
     pub fn forget_connection(&self, connection_id: Uuid) {
@@ -112,6 +124,21 @@ mod tests {
         assert_eq!(store.width(id, None, "users", "name"), Some(240));
     }
 
+    #[tokio::test]
+    async fn forget_drops_one_column() {
+        let root = tempfile::tempdir().expect("tempdir");
+        let store = store(&root);
+        let id = Uuid::new_v4();
+        store.record(id, Some("public"), "users", "name", 240);
+        store.record(id, Some("public"), "users", "email", 180);
+
+        store.forget(id, Some("public"), "users", "name");
+        store.forget(id, Some("public"), "orders", "name");
+
+        assert_eq!(store.width(id, Some("public"), "users", "name"), None);
+        assert_eq!(store.width(id, Some("public"), "users", "email"), Some(180));
+    }
+
     #[tokio::test]
     async fn forget_connection_removes_the_map() {
         let root = tempfile::tempdir().expect("tempdir");

linux/crates/app/src/test_support/accessible_label.rs

@@ -22,7 +22,8 @@ pub(crate) fn assert_labelled(widget: &impl IsA<gtk4::Widget>) -> Result<(), Unl
         };
     };
 
-    if row.title().is_empty() {
+    // A row whose controls are its content names itself instead.
+    if row.title().is_empty() && !gtk4::test_accessible_has_property(widget, AccessibleProperty::Label) {
         return Err(UnlabelledWidget::EmptyRowTitle { type_name });
     }
     if widget.is::<adw::EntryRow>() {

linux/crates/app/src/ui/a11y/announce.rs

@@ -0,0 +1,59 @@
+use gtk4 as gtk;
+use gtk4::prelude::*;
+
+/// Say `message` from `widget`, for a change a screen reader would not
+/// otherwise hear: one away from the focus, or text that changed under
+/// it.
+pub(crate) fn announce(widget: &impl IsA<gtk::Widget>, message: &str, priority: gtk::AccessibleAnnouncementPriority) {
+    if message.is_empty() {
+        return;
+    }
+    let widget = widget.as_ref();
+    #[cfg(test)]
+    said::record(widget, message, priority);
+    widget.announce(message, priority);
+}
+
+/// Show `text` on a status line, and say it when it changed: a screen
+/// reader does not read a label that changes away from the focus.
+pub(crate) fn show_status(label: &gtk::Label, text: &str) {
+    if label.label() == text {
+        return;
+    }
+    label.set_label(text);
+    announce(label, text, gtk::AccessibleAnnouncementPriority::Medium);
+}
+
+/// GTK's test backend keeps no announcement, so a test reads them here.
+#[cfg(test)]
+pub(crate) mod said {
+    use std::cell::RefCell;
+
+    use gtk4 as gtk;
+    use gtk4::glib;
+    use gtk4::prelude::*;
+
+    thread_local! {
+        static SAID: RefCell<Vec<(glib::WeakRef<gtk::Widget>, String, gtk::AccessibleAnnouncementPriority)>> =
+            const { RefCell::new(Vec::new()) };
+    }
+
+    pub(super) fn record(widget: &gtk::Widget, message: &str, priority: gtk::AccessibleAnnouncementPriority) {
+        SAID.with_borrow_mut(|said| said.push((widget.downgrade(), message.to_owned(), priority)));
+    }
+
+    /// What `root`, or a widget inside it, announced, oldest first.
+    pub(crate) fn announced_in(root: &impl IsA<gtk::Widget>) -> Vec<(String, gtk::AccessibleAnnouncementPriority)> {
+        let root = root.as_ref();
+        SAID.with_borrow(|said| {
+            said.iter()
+                .filter(|(widget, _, _)| {
+                    widget
+                        .upgrade()
+                        .is_some_and(|widget| &widget == root || widget.is_ancestor(root))
+                })
+                .map(|(_, message, priority)| (message.clone(), *priority))
+                .collect()
+        })
+    }
+}

linux/crates/app/src/ui/a11y/focus.rs

@@ -0,0 +1,40 @@
+use gtk4 as gtk;
+use gtk4::prelude::*;
+
+/// Whether the window's focus is `widget` or inside it.
+pub(crate) fn holds_focus(widget: &impl IsA<gtk::Widget>) -> bool {
+    let widget = widget.as_ref();
+    widget
+        .root()
+        .and_then(|root| root.focus())
+        .is_some_and(|focus| &focus == widget || focus.is_ancestor(widget))
+}
+
+/// Move the focus from inside `from` to `to`, before `from` is hidden or
+/// turned off. GTK would move it to the first control of the window.
+pub(crate) fn pass_focus(from: &impl IsA<gtk::Widget>, to: &impl IsA<gtk::Widget>) -> bool {
+    holds_focus(from) && to.as_ref().grab_focus()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_support::{OwnedWindow, drain_main_context};
+
+    #[gtk4::test]
+    fn the_focus_moves_only_from_inside() {
+        let first = gtk::Button::with_label("First");
+        let second = gtk::Button::with_label("Second");
+        let row = gtk::Box::new(gtk::Orientation::Horizontal, 0);
+        row.append(&first);
+        row.append(&second);
+        let window = OwnedWindow::presented(gtk::Window::builder().child(&row).build());
+        first.grab_focus();
+        drain_main_context();
+
+        assert!(!pass_focus(&second, &first), "the focus was not in Second");
+        assert!(pass_focus(&first, &second));
+        assert_eq!(GtkWindowExt::focus(&*window).as_ref(), Some(second.upcast_ref()));
+        assert!(holds_focus(&row));
+    }
+}

linux/crates/app/src/ui/a11y/invalid.rs

@@ -0,0 +1,62 @@
+use gtk4 as gtk;
+use gtk4::prelude::*;
+
+/// Flag `field` as holding a mistake, described by `problem`, or clear
+/// both. An entry row or a search entry hands the focus to the text
+/// field inside it, which is what a screen reader reads, so that field
+/// carries them too.
+pub(crate) fn show_invalid(field: &impl IsA<gtk::Widget>, problem: Option<&str>) {
+    let field = field.as_ref();
+    let delegate = field
+        .downcast_ref::<gtk::Editable>()
+        .and_then(|editable| editable.delegate())
+        .map(|delegate| delegate.upcast::<gtk::Widget>());
+    for widget in std::iter::once(field).chain(delegate.as_ref()) {
+        match problem {
+            Some(problem) => {
+                widget.update_state(&[gtk::accessible::State::Invalid(gtk::AccessibleInvalidState::True)]);
+                widget.update_property(&[gtk::accessible::Property::Description(problem)]);
+            }
+            None => {
+                widget.reset_state(gtk::AccessibleState::Invalid);
+                widget.reset_property(gtk::AccessibleProperty::Description);
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use gtk4::{AccessibleProperty, AccessibleState};
+    use libadwaita as adw;
+
+    use super::*;
+
+    #[gtk4::test]
+    fn an_entry_row_flags_its_text_field() {
+        adw::init().expect("libadwaita initialises under the test backend");
+        let row = adw::EntryRow::builder().title("Name").build();
+        let text = row
+            .delegate()
+            .and_downcast::<gtk::Text>()
+            .expect("the row's text field");
+
+        show_invalid(&row, Some("A name is required."));
+
+        for widget in [row.upcast_ref::<gtk::Widget>(), text.upcast_ref()] {
+            assert!(gtk::test_accessible_has_state(widget, AccessibleState::Invalid));
+            assert!(gtk::test_accessible_has_property(
+                widget,
+                AccessibleProperty::Description
+            ));
+        }
+
+        show_invalid(&row, None);
+
+        assert!(!gtk::test_accessible_has_state(&text, AccessibleState::Invalid));
+        assert!(!gtk::test_accessible_has_property(
+            &text,
+            AccessibleProperty::Description
+        ));
+    }
+}

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

@@ -1,9 +1,17 @@
 //! Accessibility properties the toolkit cannot set by itself.
 
+mod announce;
+mod focus;
 mod icon_button;
+mod invalid;
 mod key_shortcuts;
 #[cfg(test)]
 mod tests;
 
+#[cfg(test)]
+pub(crate) use announce::said::announced_in;
+pub(crate) use announce::{announce, show_status};
+pub(crate) use focus::{holds_focus, pass_focus};
 pub(crate) use icon_button::{announce_key_shortcuts, describe_icon_button};
+pub(crate) use invalid::show_invalid;
 pub(crate) use key_shortcuts::aria_key_shortcuts;

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

@@ -60,7 +60,8 @@ pub(super) fn install_browse_actions(tab: &BrowseTab, page_size: u64) -> BrowseA
         group.add_action(&action);
         action
     };
-    add("filter", || PagingInput::ToggleFilterStrip.into());
+    add("filter", || PagingInput::ShowFilterStrip.into());
+    add("toggle-filter", || PagingInput::ToggleFilterStrip.into());
     add("export", || ClipboardInput::Export(None).into());
     let page_size =
         gio::SimpleAction::new_stateful("page-size", Some(glib::VariantTy::UINT64), &page_size.to_variant());

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

@@ -58,6 +58,7 @@ impl From<GridMsg> for BrowseTabInput {
     fn from(msg: GridMsg) -> Self {
         match msg {
             GridMsg::SortChanged(col_idx, ascending) => PagingInput::SortChanged { col_idx, ascending }.into(),
+            GridMsg::SortCleared => PagingInput::SortCleared.into(),
             GridMsg::Edit { row, column, edit } => EditInput::GridEdit { row, column, edit }.into(),
             GridMsg::Copy(request) => ClipboardInput::GridCopy(request).into(),
             GridMsg::ShowRowJson(request) => ClipboardInput::GridShowRowJson(request).into(),

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

@@ -68,15 +68,31 @@ impl BrowseModel {
     /// error page, the banner over rows that could not be read again,
     /// and the tab bar's spinner.
     pub(super) fn present(&mut self, presentation: LoadPresentation, fx: &mut Effects) {
+        let content_had_focus =
+            std::mem::take(&mut self.content_had_focus) || crate::ui::a11y::holds_focus(&self.widgets.inner_stack);
         match &presentation.page {
-            ContentPage::Spinner => self.show_loading(),
+            ContentPage::Spinner => {
+                self.content_had_focus = content_had_focus;
+                self.show_loading();
+            }
             ContentPage::Content => self.widgets.inner_stack.set_visible_child_name("grid"),
             ContentPage::Error(failure) => {
+                let message = failure.message();
                 // The status page reads its description as markup.
                 self.widgets
                     .error_page
-                    .set_description(Some(&glib::markup_escape_text(&failure.message())));
+                    .set_description(Some(&glib::markup_escape_text(&message)));
+                // The page takes the place of the rows the focus was in, so
+                // the focus goes to Try Again and the reason is said.
                 self.widgets.inner_stack.set_visible_child_name("error");
+                if content_had_focus && let Some(retry) = self.widgets.error_page.child() {
+                    retry.grab_focus();
+                }
+                crate::ui::a11y::announce(
+                    &self.widgets.error_page,
+                    &format!("{}. {message}", self.widgets.error_page.title()),
+                    gtk::AccessibleAnnouncementPriority::High,
+                );
             }
         }
         self.widgets.stale_banner.show(presentation.stale.as_ref());
@@ -231,6 +247,8 @@ impl BrowseModel {
             },
             self.grid_context(),
         );
+        // A screen reader names the grid as the tab does.
+        column_view.update_property(&[gtk::accessible::Property::Label(&self.table_label())]);
         self.current_selection = Some(selection);
         self.current_column_view = Some(column_view.clone());
         self.view_columns = data_columns(&column_view);

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

@@ -41,6 +41,19 @@ fn keyed(ids: &[i64]) -> QueryResult {
     page
 }
 
+#[gtk4::test]
+fn the_grid_is_named_after_its_table() {
+    let mut script = serving_table();
+    script.query_bound.always(FakeReply::ok(rows(&["Ada"])));
+    let harness = Harness::open(script, []);
+    wait_until(WAIT, || harness.shown().len() == 1).expect("the first page");
+
+    assert!(gtk4::test_accessible_has_property(
+        &column_view(harness.tab()),
+        gtk4::AccessibleProperty::Label
+    ));
+}
+
 #[gtk4::test]
 fn refresh_keeps_grid_visible_and_sets_tab_loading() {
     let mut script = serving_table();
@@ -87,6 +100,17 @@ fn failed_refresh_keeps_rows_and_reveals_stale_banner() {
     assert!(!harness.tab().is_loading());
     let tooltip = banner(&harness).tooltip_text().map(|text| text.to_string());
     assert_eq!(tooltip.as_deref(), Some("The connection was closed. Try reconnecting."));
+    // The reason shows only on hover, away from the rows the focus is in.
+    assert_eq!(
+        crate::ui::a11y::announced_in(&banner(&harness)),
+        [(
+            format!(
+                "{} The connection was closed. Try reconnecting.",
+                banner(&harness).title()
+            ),
+            gtk4::AccessibleAnnouncementPriority::High
+        )]
+    );
 }
 
 #[gtk4::test]
@@ -130,6 +154,14 @@ fn first_load_failure_shows_error_page_with_try_again() {
     assert!(description.contains("relation <users> is gone"), "{description}");
     assert!(!banner(&harness).is_revealed());
     assert!(imp.retry_button.has_css_class("pill") && imp.retry_button.has_css_class("suggested-action"));
+    assert!(
+        crate::ui::a11y::announced_in(&imp.error_page.get())
+            .iter()
+            .any(|(said, priority)| said.starts_with("Couldn't Load Rows. ")
+                && said.contains("relation <users> is gone")
+                && *priority == gtk4::AccessibleAnnouncementPriority::High),
+        "the error page took the grid's place unsaid"
+    );
 
     imp.retry_button.emit_clicked();
 
@@ -209,6 +241,31 @@ fn a_selection_is_not_put_back_after_a_failed_refresh() {
     );
 }
 
+/// The loading page has nothing to focus, and the error page that
+/// follows it hands the focus the rows had to Try Again.
+#[gtk4::test]
+fn a_failed_page_turn_hands_the_focus_to_try_again() {
+    let mut script = super::tests::paging(full_page(0), rows(&["unused"]));
+    script.query_bound = Default::default();
+    script
+        .query_bound
+        .once(FakeReply::ok(full_page(0)))
+        .always(FakeReply::err(DriverError::server("denied")));
+    let harness = Harness::open(script, []);
+    wait_until(WAIT, || harness.shown().len() == 100).expect("the first page");
+    let shown = Shown::new(harness.tab());
+    let view = column_view(harness.tab());
+    view.grab_focus();
+    let focus = || gtk4::prelude::GtkWindowExt::focus(&shown.0);
+    wait_until(WAIT, || focus().is_some_and(|focus| focus.is_ancestor(&view))).expect("the grid takes the focus");
+
+    harness.tab().send(PagingInput::NextPage);
+
+    wait_until(WAIT, || harness.visible_page().as_deref() == Some("error")).expect("the error page");
+    let retry = harness.tab().imp().retry_button.get();
+    assert_eq!(focus().as_ref(), Some(retry.upcast_ref()));
+}
+
 fn error_text(harness: &Harness) -> String {
     harness
         .tab()