diff --git a/linux/crates/app/src/application/accelerators.rs b/linux/crates/app/src/application/accelerators.rs index 110bcd4b30..4a55e514d2 100644 --- a/linux/crates/app/src/application/accelerators.rs +++ b/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, diff --git a/linux/crates/app/src/persistence/column_width_store.rs b/linux/crates/app/src/persistence/column_width_store.rs index 6f9d0f2745..6311d3dfa9 100644 --- a/linux/crates/app/src/persistence/column_width_store.rs +++ b/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"); diff --git a/linux/crates/app/src/test_support/accessible_label.rs b/linux/crates/app/src/test_support/accessible_label.rs index a45f98aa48..f64d98c5c6 100644 --- a/linux/crates/app/src/test_support/accessible_label.rs +++ b/linux/crates/app/src/test_support/accessible_label.rs @@ -22,7 +22,8 @@ pub(crate) fn assert_labelled(widget: &impl IsA) -> 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::() { diff --git a/linux/crates/app/src/ui/a11y/announce.rs b/linux/crates/app/src/ui/a11y/announce.rs new file mode 100644 index 0000000000..2aafa4565d --- /dev/null +++ b/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, 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: >k::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, String, gtk::AccessibleAnnouncementPriority)>> = + const { RefCell::new(Vec::new()) }; + } + + pub(super) fn record(widget: >k::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) -> 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() + }) + } +} diff --git a/linux/crates/app/src/ui/a11y/focus.rs b/linux/crates/app/src/ui/a11y/focus.rs new file mode 100644 index 0000000000..8c33a373a0 --- /dev/null +++ b/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) -> 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, to: &impl IsA) -> 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)); + } +} diff --git a/linux/crates/app/src/ui/a11y/invalid.rs b/linux/crates/app/src/ui/a11y/invalid.rs new file mode 100644 index 0000000000..7aac20f5e8 --- /dev/null +++ b/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, problem: Option<&str>) { + let field = field.as_ref(); + let delegate = field + .downcast_ref::() + .and_then(|editable| editable.delegate()) + .map(|delegate| delegate.upcast::()); + 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::() + .expect("the row's text field"); + + show_invalid(&row, Some("A name is required.")); + + for widget in [row.upcast_ref::(), 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 + )); + } +} diff --git a/linux/crates/app/src/ui/a11y/mod.rs b/linux/crates/app/src/ui/a11y/mod.rs index 7d38bbdc2b..c7dbac9341 100644 --- a/linux/crates/app/src/ui/a11y/mod.rs +++ b/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; diff --git a/linux/crates/app/src/ui/browse_tab/browse_actions.rs b/linux/crates/app/src/ui/browse_tab/browse_actions.rs index 3540753a2e..7903a7f6cc 100644 --- a/linux/crates/app/src/ui/browse_tab/browse_actions.rs +++ b/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()); diff --git a/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs b/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs index a0eddfcdab..23cd1c4b17 100644 --- a/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs +++ b/linux/crates/app/src/ui/browse_tab/browse_tab_input.rs @@ -58,6 +58,7 @@ impl From 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(), diff --git a/linux/crates/app/src/ui/browse_tab/data_handlers.rs b/linux/crates/app/src/ui/browse_tab/data_handlers.rs index 0d5842bca8..51f7738e29 100644 --- a/linux/crates/app/src/ui/browse_tab/data_handlers.rs +++ b/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); diff --git a/linux/crates/app/src/ui/browse_tab/load_tests.rs b/linux/crates/app/src/ui/browse_tab/load_tests.rs index caaaa10542..bc2dcaa617 100644 --- a/linux/crates/app/src/ui/browse_tab/load_tests.rs +++ b/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 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() diff --git a/linux/crates/app/src/ui/browse_tab/mod.rs b/linux/crates/app/src/ui/browse_tab/mod.rs index 5558983dbd..a3aee7569e 100644 --- a/linux/crates/app/src/ui/browse_tab/mod.rs +++ b/linux/crates/app/src/ui/browse_tab/mod.rs @@ -192,6 +192,7 @@ impl BrowseTab { total: RowTotal::Unknown, load: ContentLoad::default(), tab_loading: false, + content_had_focus: false, current_column_view: None, view_columns: Vec::new(), row_index: GridRowIndex::default(), @@ -513,6 +514,9 @@ pub(crate) struct BrowseModel { load: ContentLoad, /// What the tab bar was last told, so it hears only changes. tab_loading: bool, + /// The rows held the focus when the loading page, which has nothing + /// to focus, took their place. + content_had_focus: bool, current_column_view: Option, /// The view's columns in the order of a row's cells, which a drag in diff --git a/linux/crates/app/src/ui/browse_tab/pager_bar.rs b/linux/crates/app/src/ui/browse_tab/pager_bar.rs index 8666f49be4..ce917f61c6 100644 --- a/linux/crates/app/src/ui/browse_tab/pager_bar.rs +++ b/linux/crates/app/src/ui/browse_tab/pager_bar.rs @@ -150,9 +150,56 @@ impl PagerBar { /// back goes from the pager, and Next waits for a page on screen. pub(super) fn show_position(&self, pager: &BrowsePager, rows_on_page: u64) { let imp = self.imp(); - imp.first_button.set_sensitive(pager.first().is_some()); - imp.prev_button.set_sensitive(pager.prev().is_some()); - imp.next_button.set_sensitive(pager.next(rows_on_page).is_some()); + self.turn(&[ + (imp.first_button.get(), pager.first().is_some()), + (imp.prev_button.get(), pager.prev().is_some()), + (imp.next_button.get(), pager.next(rows_on_page).is_some()), + ]); + } + + /// Turn each button on or off. The focus on a button turned off goes + /// to the nearest one left on, as Next at the last page hands it to + /// Previous; GTK would move it to the window's first control. + fn turn(&self, changes: &[(gtk::Button, bool)]) { + for (button, _) in changes.iter().filter(|(_, on)| *on) { + button.set_sensitive(true); + } + let usable = |button: >k::Button| { + changes + .iter() + .find(|(changed, _)| changed == button) + .map_or(button.is_sensitive(), |(_, on)| *on) + && button.is_visible() + }; + let buttons = self.buttons(); + if let Some(from) = buttons + .iter() + .position(|button| crate::ui::a11y::holds_focus(button) && !usable(button)) + { + let nearest = (1..buttons.len()) + .flat_map(|distance| [from.checked_sub(distance), Some(from + distance)]) + .flatten() + .filter_map(|index| buttons.get(index)) + .find(|button| usable(button)); + if let Some(nearest) = nearest { + nearest.grab_focus(); + } + } + for (button, _) in changes.iter().filter(|(_, on)| !*on) { + button.set_sensitive(false); + } + } + + /// The bar's buttons in the order they sit. + fn buttons(&self) -> [gtk::Button; 5] { + let imp = self.imp(); + [ + imp.first_button.get(), + imp.prev_button.get(), + imp.next_button.get(), + imp.last_button.get(), + imp.count_button.get(), + ] } /// The rows on the page, Count Rows and Last, which only an exact @@ -160,8 +207,7 @@ impl PagerBar { pub(super) fn show_total(&self, pager: &BrowsePager, shown: Option<&PageState>, total: &RowTotal) { let imp = self.imp(); let label = shown.map(|page| page_range_label(page, total)).unwrap_or_default(); - imp.paginator_label.set_label(&label); - imp.last_button.set_sensitive(pager.last(total).is_some()); + crate::ui::a11y::show_status(&imp.paginator_label, &label); let last_tooltip = match total.exact() { Some(_) => gettext("Last Page"), None => gettext("Count rows to go to the last page"), @@ -169,16 +215,43 @@ impl PagerBar { imp.last_button.set_tooltip_text(Some(&last_tooltip)); let counting = matches!(total, RowTotal::Counting { .. }); - imp.offer_count(shown.is_some() && total.exact().is_none()); - imp.count_button.set_sensitive(!counting); + let count_offered = shown.is_some() && total.exact().is_none(); + if count_offered { + imp.offer_count(true); + } + self.turn(&[ + (imp.last_button.get(), pager.last(total).is_some()), + (imp.count_button.get(), count_offered && !counting), + ]); + imp.offer_count(count_offered); imp.count_spinner.set_visible(counting); - let (label, tooltip) = match total { + let (label, reason) = match total { RowTotal::Counting { .. } => (gettext("Counting…"), None), RowTotal::Unavailable { reason, .. } => (gettext("Try Again"), Some(reason.as_str())), RowTotal::Unknown | RowTotal::Estimated(_) | RowTotal::Exact(_) => (gettext("Count Rows"), None), }; imp.count_label.set_label(&label); - imp.count_button.set_tooltip_text(tooltip); + self.show_count_failure(reason); + } + + /// A count asked for and refused says why, on Try Again as well as + /// in its tooltip, which no key shows. + fn show_count_failure(&self, reason: Option<&str>) { + let button = self.imp().count_button.get(); + let news = reason.is_some() && button.tooltip_text().as_deref() != reason; + button.set_tooltip_text(reason); + let widget = button.upcast_ref::(); + match reason { + Some(reason) => widget.update_property(&[gtk::accessible::Property::Description(reason)]), + None => widget.reset_property(gtk::AccessibleProperty::Description), + } + if let Some(reason) = reason.filter(|_| news) { + crate::ui::a11y::announce( + &button, + &crate::i18n::gettext_f("Couldn't count the rows. {reason}", &[("reason", reason)]), + gtk::AccessibleAnnouncementPriority::High, + ); + } } /// The keys for Previous and Next turn a page only where the button @@ -347,6 +420,88 @@ mod tests { assert_eq!(counts.count(), 1); } + /// The bar in a window, so its buttons can hold the focus. + fn shown_bar() -> (PagerBar, crate::test_support::OwnedWindow) { + let bar = bar(); + let window = crate::test_support::OwnedWindow::presented(gtk::Window::builder().child(&bar).build()); + (bar, window) + } + + fn focus(window: >k::Window) -> Option { + gtk::prelude::GtkWindowExt::focus(window) + } + + #[gtk4::test] + fn next_at_the_last_page_hands_the_focus_to_previous() { + let (bar, window) = shown_bar(); + let imp = bar.imp(); + bar.show_position(&BrowsePager::new(100, 0), 100); + assert!(imp.next_button.grab_focus()); + + bar.show_position(&BrowsePager::new(100, 100), 40); + + assert!(!imp.next_button.is_sensitive()); + assert_eq!(focus(&window).as_ref(), Some(imp.prev_button.upcast_ref())); + } + + #[gtk4::test] + fn a_count_keeps_the_focus_in_the_bar() { + let (bar, window) = shown_bar(); + let imp = bar.imp(); + let pager = BrowsePager::new(100, 0); + let shown = page(0, 100); + bar.show_position(&pager, 100); + bar.show_total(&pager, Some(&shown), &RowTotal::Estimated(5_000)); + assert!(imp.count_button.grab_focus()); + + bar.show_total(&pager, Some(&shown), &RowTotal::Counting { estimate: Some(5_000) }); + assert_eq!(focus(&window).as_ref(), Some(imp.next_button.upcast_ref())); + + let refused = RowTotal::Unavailable { + estimate: Some(5_000), + reason: "refused".to_owned(), + }; + bar.show_total(&pager, Some(&shown), &refused); + assert!(imp.count_button.grab_focus()); + bar.show_total(&pager, Some(&shown), &RowTotal::Exact(5_000)); + assert!(!imp.count_button.is_visible()); + assert_eq!(focus(&window).as_ref(), Some(imp.last_button.upcast_ref())); + } + + #[gtk4::test] + fn a_failed_count_says_why_on_try_again() { + let bar = bar(); + let pager = BrowsePager::new(100, 0); + let failed = RowTotal::Unavailable { + estimate: None, + reason: "Counting took longer than 45 seconds and was stopped.".to_owned(), + }; + + bar.show_total(&pager, Some(&page(0, 100)), &failed); + + let button = bar.imp().count_button.get(); + assert!(gtk::test_accessible_has_property( + &button, + AccessibleProperty::Description + )); + assert_eq!( + crate::ui::a11y::announced_in(&bar) + .last() + .map(|(said, priority)| (said.clone(), *priority)), + Some(( + "Couldn't count the rows. Counting took longer than 45 seconds and was stopped.".to_owned(), + gtk::AccessibleAnnouncementPriority::High + )) + ); + + bar.show_total(&pager, Some(&page(0, 100)), &RowTotal::Exact(500)); + + assert!(!gtk::test_accessible_has_property( + &button, + AccessibleProperty::Description + )); + } + #[gtk4::test] fn a_compact_bar_leaves_count_rows_to_the_menu() { let bar = bar(); @@ -375,5 +530,13 @@ mod tests { bar.show_total(&pager, Some(&page(0, 5)), &RowTotal::Exact(5)); assert_eq!(bar.imp().paginator_label.label(), "Rows 1 – 5 of 5"); + assert_eq!( + crate::ui::a11y::announced_in(&bar), + [( + "Rows 1 – 5 of 5".to_owned(), + gtk::AccessibleAnnouncementPriority::Medium + )], + "a page turn is said once" + ); } } diff --git a/linux/crates/app/src/ui/browse_tab/paging_handlers.rs b/linux/crates/app/src/ui/browse_tab/paging_handlers.rs index 8af417e6a4..072b915460 100644 --- a/linux/crates/app/src/ui/browse_tab/paging_handlers.rs +++ b/linux/crates/app/src/ui/browse_tab/paging_handlers.rs @@ -12,12 +12,14 @@ impl BrowseModel { PagingInput::Refresh => self.reload(fx), PagingInput::RequestRefresh => fx.output(BrowseTabOutput::RefreshRequested), PagingInput::ToggleFilterStrip => self.filter_strip.toggle(), + PagingInput::ShowFilterStrip => self.filter_strip.open(), PagingInput::FilterApplied(set) => self.apply_filter(set, fx), PagingInput::FirstPage => self.turn_page(self.pager.first(), fx), PagingInput::PrevPage => self.turn_page(self.pager.prev(), fx), PagingInput::NextPage => self.turn_page(self.pager.next(self.rows_on_page()), fx), PagingInput::LastPage => self.turn_page(self.pager.last(&self.total), fx), - PagingInput::SortChanged { col_idx, ascending } => self.sort_by((col_idx, ascending), fx), + PagingInput::SortChanged { col_idx, ascending } => self.sort_by(Some((col_idx, ascending)), fx), + PagingInput::SortCleared => self.sort_by(None, fx), PagingInput::PageSizeChanged(size) => { if self.suppress_combo_emit.get() || self.pager.page_size() == size { return; @@ -61,13 +63,13 @@ impl BrowseModel { fx.output(BrowseTabOutput::StateChanged); } - fn sort_by(&mut self, next: (usize, bool), fx: &mut Effects) { + fn sort_by(&mut self, next: Option<(usize, bool)>, fx: &mut Effects) { // The sorter notifies twice for one click on a new column // (column, then order), with the same final pair. - if self.current_sort == Some(next) { + if self.current_sort == next { return; } - self.current_sort = Some(next); + self.current_sort = next; self.pager.restart(); self.capture_selection(); self.fetch(BrowseRequestKind::Page, fx); diff --git a/linux/crates/app/src/ui/browse_tab/paging_input.rs b/linux/crates/app/src/ui/browse_tab/paging_input.rs index 2399f326d4..9ffb8efaa9 100644 --- a/linux/crates/app/src/ui/browse_tab/paging_input.rs +++ b/linux/crates/app/src/ui/browse_tab/paging_input.rs @@ -8,7 +8,10 @@ pub(crate) enum PagingInput { Refresh, /// F5: the window asks first when the tab has unsaved changes. RequestRefresh, + /// The Filter button. ToggleFilterStrip, + /// Ctrl+F, which opens the strip or takes the focus into it. + ShowFilterStrip, /// The strip's rules, or an empty set for Clear All. FilterApplied(FilterSet), FirstPage, @@ -19,6 +22,7 @@ pub(crate) enum PagingInput { col_idx: usize, ascending: bool, }, + SortCleared, PageSizeChanged(u64), /// Count Rows, or Try Again after a count failed. CountRows, diff --git a/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs b/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs index 863e98d099..3dcee46344 100644 --- a/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs +++ b/linux/crates/app/src/ui/browse_tab/pending_changes_bar.rs @@ -128,19 +128,37 @@ impl PendingChangesBar { &[("n", &count.to_string())], ) }; - imp.pending_label.set_label(&label); + crate::ui::a11y::show_status(&imp.pending_label, &label); } imp.revealer.set_reveal_child(visible); } /// While a save runs, Save and Discard are off, so a double click - /// cannot send two, and Stop Saving takes their place. + /// cannot send two, and Stop Saving takes their place, the focus + /// with it. pub(super) fn show_saving(&self, saving: bool) { let imp = self.imp(); - imp.save_button.set_sensitive(!saving); - imp.discard_button.set_sensitive(!saving); - imp.saving_spinner.set_visible(saving); - imp.stop_button.set_visible(saving); + let save = imp.save_button.get(); + let stop = imp.stop_button.get(); + if saving { + imp.saving_spinner.set_visible(true); + stop.set_visible(true); + for button in [&save, &imp.discard_button.get()] { + crate::ui::a11y::pass_focus(button, &stop); + button.set_sensitive(false); + } + crate::ui::a11y::announce( + &imp.saving_spinner.get(), + &crate::i18n::gettext("Saving…"), + gtk::AccessibleAnnouncementPriority::Medium, + ); + } else { + save.set_sensitive(true); + imp.discard_button.set_sensitive(true); + crate::ui::a11y::pass_focus(&stop, &save); + stop.set_visible(false); + imp.saving_spinner.set_visible(false); + } } fn connect_buttons(&self) { @@ -236,4 +254,40 @@ mod tests { assert!(!imp.stop_button.is_visible() && !imp.saving_spinner.is_visible()); assert!(imp.save_button.is_sensitive() && imp.discard_button.is_sensitive()); } + + #[gtk4::test] + fn the_focus_follows_save_to_stop_saving_and_back() { + let bar = bar(); + let window = crate::test_support::OwnedWindow::presented(gtk::Window::builder().child(&bar).build()); + let imp = bar.imp(); + bar.show_pending(2); + assert!(imp.save_button.grab_focus()); + let focus = || gtk::prelude::GtkWindowExt::focus(&*window); + + bar.show_saving(true); + assert_eq!(focus().as_ref(), Some(imp.stop_button.upcast_ref())); + assert!( + crate::ui::a11y::announced_in(&bar) + .iter() + .any(|(said, _)| said == "Saving…") + ); + + bar.show_saving(false); + assert_eq!(focus().as_ref(), Some(imp.save_button.upcast_ref())); + } + + #[gtk4::test] + fn a_new_count_of_changes_is_said() { + let bar = bar(); + + bar.show_pending(1); + bar.show_pending(1); + bar.show_pending(2); + + let said: Vec = crate::ui::a11y::announced_in(&bar) + .into_iter() + .map(|(said, _)| said) + .collect(); + assert_eq!(said, ["1 unsaved change", "2 unsaved changes"]); + } } diff --git a/linux/crates/app/src/ui/browse_tab/save_handlers.rs b/linux/crates/app/src/ui/browse_tab/save_handlers.rs index 6943465780..c534e060a8 100644 --- a/linux/crates/app/src/ui/browse_tab/save_handlers.rs +++ b/linux/crates/app/src/ui/browse_tab/save_handlers.rs @@ -87,6 +87,13 @@ impl BrowseModel { } pub(super) fn refresh_pending_bar(&self, count: usize) { + // The footer slides away once nothing is left to save, and the + // grid holds what the user was working on. + if count == 0 + && let Some(grid) = self.current_column_view.as_ref() + { + crate::ui::a11y::pass_focus(&self.widgets.pending_bar, grid); + } self.widgets.pending_bar.show_pending(count); self.refresh_banner_visibility(); } diff --git a/linux/crates/app/src/ui/browse_tab/tests.rs b/linux/crates/app/src/ui/browse_tab/tests.rs index 2decf0d096..0ee68ebc1e 100644 --- a/linux/crates/app/src/ui/browse_tab/tests.rs +++ b/linux/crates/app/src/ui/browse_tab/tests.rs @@ -1792,6 +1792,31 @@ fn pending_bar_shown(tab: &BrowseTab) -> bool { tab.imp().pending_bar.imp().revealer.reveals_child() } +/// Discard empties the footer, which slides away with the focus in it, +/// so the focus goes to the rows. +#[gtk4::test] +fn discard_hands_the_focus_to_the_grid() { + use gtk4::subclass::prelude::ObjectSubclassIsExt; + let mut script = serving_table(); + script.query_bound.always(FakeReply::ok(rows(&["Ada", "Grace"]))); + let harness = Harness::open(script, []); + wait_until(WAIT, || harness.shown().len() == 2).expect("the first page"); + let shown = Shown::new(harness.tab()); + harness.tab().send(EditInput::InsertRow); + wait_until(WAIT, || pending_bar_shown(harness.tab())).expect("the footer"); + let discard = harness.tab().imp().pending_bar.imp().discard_button.get(); + assert!(discard.grab_focus()); + + harness.tab().send(SaveInput::DiscardAll); + + let view = column_view(harness.tab()); + let focus = gtk4::prelude::GtkWindowExt::focus(&shown.0); + assert!( + focus.is_some_and(|focus| focus == *view.upcast_ref::() || focus.is_ancestor(&view)), + "the focus left with the footer" + ); +} + #[gtk4::test] fn deleting_a_draft_removes_its_row() { let mut script = serving_table(); diff --git a/linux/crates/app/src/ui/connect_dialog/row_problem.rs b/linux/crates/app/src/ui/connect_dialog/row_problem.rs index 16d78653fe..84592829f2 100644 --- a/linux/crates/app/src/ui/connect_dialog/row_problem.rs +++ b/linux/crates/app/src/ui/connect_dialog/row_problem.rs @@ -2,20 +2,20 @@ use gtk4::prelude::*; /// Mark a row as holding a mistake, or clear the mark. /// -/// The red style alone says nothing to a screen reader, so the row is -/// also flagged invalid and carries the sentence as its tooltip. +/// The red style alone says nothing to a screen reader, so the row, and +/// the field inside it that takes the focus, are flagged invalid and +/// carry the sentence. pub fn show_problem(row: &impl IsA, problem: Option<&str>) { let row = row.as_ref(); match problem { Some(message) => { row.add_css_class("error"); row.set_tooltip_text(Some(message)); - row.update_state(&[gtk4::accessible::State::Invalid(gtk4::AccessibleInvalidState::True)]); } None => { row.remove_css_class("error"); row.set_tooltip_text(None); - row.reset_state(gtk4::AccessibleState::Invalid); } } + crate::ui::a11y::show_invalid(row, problem); } diff --git a/linux/crates/app/src/ui/connect_dialog/tests.rs b/linux/crates/app/src/ui/connect_dialog/tests.rs index 15d7c8ae29..38d430656d 100644 --- a/linux/crates/app/src/ui/connect_dialog/tests.rs +++ b/linux/crates/app/src/ui/connect_dialog/tests.rs @@ -278,22 +278,27 @@ fn a_bad_host_is_marked_for_sight_and_for_screen_readers() { fill_network(&harness.dialog); let host = harness.dialog.imp().host_row.get(); + // The focus is in the row's text field, which is what a reader reads. + let field = host.delegate().expect("the row's text field"); + host.set_text("db example.com"); assert!(host.has_css_class("error")); - assert!(gtk4::test_accessible_has_state( - host.upcast_ref::(), - AccessibleState::Invalid + for widget in [host.upcast_ref::(), field.upcast_ref()] { + assert!(gtk4::test_accessible_has_state(widget, AccessibleState::Invalid)); + } + assert!(gtk4::test_accessible_has_property( + &field, + gtk4::AccessibleProperty::Description )); assert!(host.tooltip_text().is_some()); host.set_text("db.example.com"); assert!(!host.has_css_class("error")); - assert!(!gtk4::test_accessible_has_state( - host.upcast_ref::(), - AccessibleState::Invalid - )); + for widget in [host.upcast_ref::(), field.upcast_ref()] { + assert!(!gtk4::test_accessible_has_state(widget, AccessibleState::Invalid)); + } } #[gtk4::test] diff --git a/linux/crates/app/src/ui/editor/editor_actions.rs b/linux/crates/app/src/ui/editor/editor_actions.rs index 850cd96317..a5144c547f 100644 --- a/linux/crates/app/src/ui/editor/editor_actions.rs +++ b/linux/crates/app/src/ui/editor/editor_actions.rs @@ -25,7 +25,7 @@ impl SqlEditor { entry("open-file", SqlEditor::open_file), entry("run", SqlEditor::run), entry("run-statement", SqlEditor::run_at_cursor), - entry("cancel", SqlEditor::stop), + entry("cancel", SqlEditor::close_popup_or_stop), entry("format", SqlEditor::format), entry("toggle-comment", |editor| toggle_line_comment(&editor.buffer())), entry("find", |editor| editor.imp().find_bar.open(false)), @@ -46,9 +46,39 @@ impl SqlEditor { self.add_controller(local_shortcut_controller(ShortcutOwner::SqlEditorTab)); crate::ui::a11y::announce_key_shortcuts(&imp.run_button.get(), "editor.run"); crate::ui::a11y::announce_key_shortcuts(&imp.stop_button.get(), "editor.cancel"); + crate::ui::a11y::announce_key_shortcuts( + imp.source_view.upcast_ref::(), + "paned.cycle-child-focus", + ); } } +impl SqlEditor { + /// Escape while a query runs. The stop key's controller runs before + /// the view's own, so a popup the view has open, such as the + /// completion list, is closed here first, as the view's Escape would. + fn close_popup_or_stop(&self) { + if !close_view_popups(&self.imp().source_view.get()) { + self.stop(); + } + } +} + +/// GtkSourceView parents its completion list and other assistants to +/// the view as popovers, and its Escape hides every visible one. +fn close_view_popups(view: &impl IsA) -> bool { + let mut closed = false; + let mut child = view.as_ref().first_child(); + while let Some(widget) = child { + child = widget.next_sibling(); + if widget.is::() && widget.is_visible() { + widget.set_visible(false); + closed = true; + } + } + closed +} + /// Off while nothing runs, so Escape goes on to the view and its /// completion popup. fn set_cancel_enabled(actions: &gio::SimpleActionGroup, enabled: bool) { @@ -56,3 +86,26 @@ fn set_cancel_enabled(actions: &gio::SimpleActionGroup, enabled: bool) { cancel.set_enabled(enabled); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// GtkSourceView's completion list is a popover child of the view, + /// which GtkTextView will not take from a test; any widget holds + /// one the same way. + #[gtk4::test] + fn escape_closes_a_shown_popup_and_says_so() { + let parent = gtk4::Box::new(gtk4::Orientation::Vertical, 0); + let _window = crate::test_support::OwnedWindow::presented(gtk4::Window::builder().child(&parent).build()); + let popup = gtk4::Popover::new(); + popup.set_parent(&parent); + assert!(!close_view_popups(&parent), "nothing was open"); + + popup.set_visible(true); + + assert!(close_view_popups(&parent)); + assert!(!popup.is_visible()); + popup.unparent(); + } +} diff --git a/linux/crates/app/src/ui/editor/find_bar.rs b/linux/crates/app/src/ui/editor/find_bar.rs index 91db83a412..2d4b1f9c48 100644 --- a/linux/crates/app/src/ui/editor/find_bar.rs +++ b/linux/crates/app/src/ui/editor/find_bar.rs @@ -11,6 +11,9 @@ use sourceview5::prelude::*; use super::find_state::FindStatus; +/// Long enough for a typed word, short enough to answer the last key. +const ANNOUNCE_AFTER: std::time::Duration = std::time::Duration::from_millis(600); + mod imp { use libadwaita::subclass::prelude::*; @@ -43,6 +46,11 @@ mod imp { /// A move asked for while the search was still counting, made /// once it has counted. pub(super) pending: Cell>, + /// The count said once typing pauses, rather than at each key. + pub(super) announcing: std::cell::RefCell>, + /// What was last said or is about to be, so a refresh that changes + /// nothing says nothing. + pub(super) announcement: std::cell::RefCell>, } #[glib::object_subclass] @@ -388,6 +396,7 @@ impl FindBar { let imp = self.imp(); let status = self.status(); imp.count_label.set_label(&status.label()); + self.announce_when_settled(status.announcement()); if let Some(actions) = imp.actions.get() { for name in ["replace", "replace-all"] { if let Some(action) = actions.lookup_action(name).and_downcast::() { @@ -397,8 +406,38 @@ impl FindBar { } } + /// The count and the pattern's error show only in a label and on the + /// entry, away from the text the reader is on. + fn announce_when_settled(&self, announcement: Option) { + let imp = self.imp(); + if *imp.announcement.borrow() == announcement { + return; + } + imp.announcement.replace(announcement.clone()); + if let Some(pending) = imp.announcing.take() { + pending.remove(); + } + let Some(announcement) = announcement else { + return; + }; + let bar = self.downgrade(); + let pending = glib::timeout_add_local_once(ANNOUNCE_AFTER, move || { + let Some(bar) = bar.upgrade() else { + return; + }; + bar.imp().announcing.take(); + crate::ui::a11y::announce( + &bar.imp().search_entry.get(), + &announcement, + gtk::AccessibleAnnouncementPriority::Medium, + ); + }); + imp.announcing.replace(Some(pending)); + } + fn show_regex_error(&self, error: Option<&glib::Error>) { let entry = self.imp().search_entry.get(); + crate::ui::a11y::show_invalid(&entry, error.map(glib::Error::message)); match error { Some(error) => { entry.add_css_class("error"); diff --git a/linux/crates/app/src/ui/editor/find_state.rs b/linux/crates/app/src/ui/editor/find_state.rs index faaab59fd1..4148413576 100644 --- a/linux/crates/app/src/ui/editor/find_state.rs +++ b/linux/crates/app/src/ui/editor/find_state.rs @@ -39,6 +39,15 @@ impl FindStatus { } } + /// What a screen reader hears once the search settles: the count, or + /// what is wrong with the pattern, which the label leaves out. + pub(crate) fn announcement(&self) -> Option { + if let Some(error) = &self.regex_error { + return Some(error.clone()); + } + Some(self.label()).filter(|label| !label.is_empty()) + } + /// Whether there is a match to replace, with its count known. pub(crate) fn has_matches(&self) -> bool { self.regex_error.is_none() && self.occurrences.is_some_and(|count| count > 0) @@ -67,6 +76,29 @@ mod tests { ); } + #[test] + fn the_announcement_is_the_count_or_the_pattern_error() { + assert_eq!( + FindStatus::from_counts(3, 2, None).announcement().as_deref(), + Some("2 of 3") + ); + assert_eq!( + FindStatus::from_counts(0, 0, None).announcement().as_deref(), + Some("No results") + ); + assert_eq!( + FindStatus::from_counts(-1, -1, None).announcement(), + None, + "while counting" + ); + assert_eq!( + FindStatus::from_counts(0, 0, Some("missing )".to_owned())) + .announcement() + .as_deref(), + Some("missing )") + ); + } + #[test] fn only_a_counted_match_can_be_replaced() { assert!(FindStatus::from_counts(2, 1, None).has_matches()); diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index e7743b3a61..2e8ede254b 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -53,6 +53,7 @@ use super::grid::{GridMsg, GridSink, present_row_json}; use super::handler_guard::HandlerGuard; use crate::services::active_session::ActiveSession; use crate::services::history_service::HistoryService; +use crate::workspace::WorkOutcome; glib::wrapper! { pub struct SqlEditor(ObjectSubclass) @@ -473,7 +474,12 @@ impl SqlEditor { } }, }; - Some(result_widgets::entry_widget(entry, grid_sink, actions)) + Some(result_widgets::entry_widget( + usize::try_from(index).ok()?, + entry, + grid_sink, + actions, + )) } fn finish_run( @@ -503,6 +509,12 @@ impl SqlEditor { ) }; imp.results.set_summary(&summary); + // The focus stays in the editor, where the summary does not show. + let priority = match outcome { + WorkOutcome::Failed => gtk4::AccessibleAnnouncementPriority::High, + WorkOutcome::Succeeded | WorkOutcome::Cancelled => gtk4::AccessibleAnnouncementPriority::Medium, + }; + crate::ui::a11y::announce(&*imp.results, &summary, priority); imp.results.set_transaction(label.as_deref()); if let Some(first) = first.and_then(|first| u32::try_from(first).ok()) { imp.results.select(first); diff --git a/linux/crates/app/src/ui/editor/result_widgets.rs b/linux/crates/app/src/ui/editor/result_widgets.rs index 3f6533d7fd..82783c34ea 100644 --- a/linux/crates/app/src/ui/editor/result_widgets.rs +++ b/linux/crates/app/src/ui/editor/result_widgets.rs @@ -17,8 +17,13 @@ pub struct PageActions { pub export_all: E, } -/// The widget a results list line shows. -pub fn entry_widget(entry: &RunLogEntry, grid_sink: &GridSink, actions: PageActions) -> gtk4::Widget +/// The widget the results list's `index`th line shows. +pub fn entry_widget( + index: usize, + entry: &RunLogEntry, + grid_sink: &GridSink, + actions: PageActions, +) -> gtk4::Widget where G: Fn(usize) + 'static, E: Fn(ExportSource) + 'static, @@ -28,7 +33,9 @@ where export_all, } = actions; match &entry.kind { - EntryKind::ResultSet(result) => result_set_page(result, grid_sink.clone(), export_all), + EntryKind::ResultSet(result) => { + result_set_page(result, &result.summary.tab_label(index), grid_sink.clone(), export_all) + } EntryKind::Command { rows_affected } => { let title = match rows_affected { Some(rows) => crate::i18n::ngettext_f( @@ -122,6 +129,31 @@ mod tests { } } + #[gtk4::test] + fn a_result_grid_is_named_as_its_line_in_the_list() { + let columns: std::sync::Arc<[tablepro_core::column::ResultColumn]> = + std::sync::Arc::from([tablepro_core::column::ResultColumn::new( + "a", + tablepro_core::column::ColumnType::untyped_text("text"), + )]); + let view = crate::ui::result_pane::ResultSetView::new(columns, tablepro_core::UserSql::new("SELECT a FROM t")); + let name = view.summary.tab_label(1); + let entry = RunLogEntry { + kind: EntryKind::ResultSet(view), + batch: Some(0), + preview: String::new(), + }; + + let widget = entry_widget(1, &entry, &sink(), actions(|_| {})); + + let grid = crate::test_support::first_descendant_of_type::(&widget).expect("the grid"); + assert_eq!(name, "Result 2 (0)"); + assert!(gtk4::test_accessible_has_property( + &grid, + gtk4::AccessibleProperty::Label + )); + } + #[gtk4::test] fn markup_in_error_is_rendered_literally() { let entry = RunLogEntry { @@ -133,7 +165,7 @@ mod tests { preview: String::new(), }; - let widget = entry_widget(&entry, &sink(), actions(|_| {})); + let widget = entry_widget(0, &entry, &sink(), actions(|_| {})); let page = widget.downcast::().expect("a status page"); assert_eq!( @@ -155,7 +187,7 @@ mod tests { let reached = std::rc::Rc::new(std::cell::Cell::new(None)); let seen = reached.clone(); - let widget = entry_widget(&entry, &sink(), actions(move |position| seen.set(Some(position)))); + let widget = entry_widget(0, &entry, &sink(), actions(move |position| seen.set(Some(position)))); let page = widget.downcast::().expect("a status page"); let button = page.child().and_downcast::().expect("Go to Error"); button.emit_clicked(); diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 2ecfe3921d..0446a9207d 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -184,6 +184,34 @@ fn a_script_fills_the_results_list_one_line_per_statement() { assert!(harness.editor.imp().run_button.is_sensitive()); } +/// Ctrl+Return leaves the focus in the editor, so the summary is said. +#[gtk4::test] +fn a_finished_run_says_how_it_went() { + let mut runs = Replies::default(); + runs.when_sql_contains( + "boom", + FakeRun::failed(DriverError::server("relation \"boom\" does not exist")), + ); + runs.always(FakeRun::completed(Vec::new())); + let harness = editor_with(runs, "SELECT 1"); + + run_and_wait(&harness); + harness.editor.buffer().set_text("SELECT * FROM boom"); + run_and_wait(&harness); + + let said: Vec<(bool, gtk4::AccessibleAnnouncementPriority)> = crate::ui::a11y::announced_in(&harness.editor) + .into_iter() + .map(|(message, priority)| (message.contains("failed"), priority)) + .collect(); + assert_eq!( + said, + [ + (false, gtk4::AccessibleAnnouncementPriority::Medium), + (true, gtk4::AccessibleAnnouncementPriority::High), + ] + ); +} + #[gtk4::test] fn a_long_script_that_fails_early_builds_one_result_page() { let mut runs = Replies::default(); @@ -396,6 +424,22 @@ fn the_icon_buttons_have_accessible_names() { ); } +/// Tab indents and Ctrl+Tab switches tabs, so the editor names F6, the +/// key its pane binds to move on to the results. +#[gtk4::test] +fn the_editor_names_its_way_out() { + use crate::test_support::shortcut_keys::{class_binding_keys, keystroke}; + let harness = editor_with(answering(), "SELECT 1"); + let view = harness.editor.imp().source_view.get(); + let paned = view.ancestor(gtk4::Paned::static_type()).expect("the editor's pane"); + + assert!(class_binding_keys(&paned).contains(&keystroke("F6"))); + assert!(gtk4::test_accessible_has_property( + view.upcast_ref::(), + gtk4::AccessibleProperty::KeyShortcuts + )); +} + #[gtk4::test] fn a_run_is_recorded_unless_it_carries_a_credential() { let root = tempfile::tempdir().expect("tempdir"); @@ -1047,6 +1091,37 @@ fn invalid_regex_marks_entry_error() { assert_eq!(entry.tooltip_text(), None); } +/// The count and a pattern's error show away from the text the reader +/// is on, so they are said, and the error marks the field. +#[gtk4::test] +fn the_match_count_and_a_bad_pattern_are_said() { + let harness = editor_with(answering(), "SELECT 1; SELECT 2"); + let _window = InWindow::new(&harness.editor); + let bar = search(&harness, "SELECT"); + let said = |text: &str| { + crate::ui::a11y::announced_in(&bar) + .iter() + .any(|(message, _)| message == text) + }; + wait_until(WAIT, || said(&bar.count_text())).expect("the count to be said"); + + WidgetExt::activate_action(&bar, "find.regex-enabled", None).expect("find.regex-enabled"); + let entry = bar.search_entry(); + entry.set_text("("); + let text = entry.delegate().expect("the entry's text field"); + wait_until(WAIT, || { + gtk4::test_accessible_has_state(&text, gtk4::AccessibleState::Invalid) + && entry.tooltip_text().is_some_and(|error| said(&error)) + }) + .expect("the pattern's error to be said and marked"); + + entry.set_text("SEL+"); + wait_until(WAIT, || { + !gtk4::test_accessible_has_state(&text, gtk4::AccessibleState::Invalid) + }) + .expect("the mark to clear"); +} + #[gtk4::test] fn case_sensitive_action_toggles_settings() { let harness = editor_with(answering(), "SELECT 1"); diff --git a/linux/crates/app/src/ui/export/export_ledger/tests.rs b/linux/crates/app/src/ui/export/export_ledger/tests.rs index 7ef6343470..00532ff00e 100644 --- a/linux/crates/app/src/ui/export/export_ledger/tests.rs +++ b/linux/crates/app/src/ui/export/export_ledger/tests.rs @@ -526,6 +526,15 @@ fn stop_button_cancels_job_and_removes_row() { assert!(button.is_visible()); let row = button.job_list().row_at_index(0).expect("the job's row"); let stop = first_descendant_of_type::(&row).expect("the stop button"); + let progress = first_descendant_of_type::(&row).expect("the job's bar"); + assert!(gtk::test_accessible_has_relation( + &stop, + gtk::AccessibleRelation::DescribedBy + )); + assert!(gtk::test_accessible_has_relation( + &progress, + gtk::AccessibleRelation::LabelledBy + )); stop.emit_clicked(); let outcome = harness.finish(handle); diff --git a/linux/crates/app/src/ui/export/export_operations_button.rs b/linux/crates/app/src/ui/export/export_operations_button.rs index d4d0ec8b47..f7a06880a4 100644 --- a/linux/crates/app/src/ui/export/export_operations_button.rs +++ b/linux/crates/app/src/ui/export/export_operations_button.rs @@ -103,6 +103,9 @@ fn job_row(item: &ExportJobItem) -> gtk::Widget { let stop = gtk::Button::from_icon_name(crate::ui::icons::PROCESS_STOP); crate::ui::a11y::describe_icon_button(&stop, &gettext("Stop Export"), None); + // Every job has a bar and a Stop Export, so each says which file. + progress.update_relation(&[gtk::accessible::Relation::LabelledBy(&[title.upcast_ref()])]); + stop.update_relation(&[gtk::accessible::Relation::DescribedBy(&[title.upcast_ref()])]); stop.set_valign(gtk::Align::Center); stop.add_css_class("flat"); stop.add_css_class("circular"); diff --git a/linux/crates/app/src/ui/filter_strip/imp.rs b/linux/crates/app/src/ui/filter_strip/imp.rs index 1c757320a2..a70cbda66b 100644 --- a/linux/crates/app/src/ui/filter_strip/imp.rs +++ b/linux/crates/app/src/ui/filter_strip/imp.rs @@ -45,6 +45,9 @@ pub struct FilterStrip { pub(super) rules: OnceCell, /// The columns a rule can name, blobs left out. pub(super) columns: RefCell>, + /// Where the focus was when it came into the strip, and goes back to + /// when the strip closes. + pub(super) focus_before: glib::WeakRef, } #[glib::object_subclass] @@ -55,7 +58,7 @@ impl ObjectSubclass for FilterStrip { fn class_init(klass: &mut Self::Class) { klass.bind_template(); - klass.install_action("filter.close", None, |strip, _, _| strip.set_revealed(false)); + klass.install_action("filter.close", None, |strip, _, _| strip.close()); klass.install_action("filter.apply", None, |strip, _, _| strip.apply()); klass.install_action("filter.clear", None, |strip, _, _| strip.clear()); klass.install_action("filter.add-rule", None, |strip, _, _| strip.add_rule()); diff --git a/linux/crates/app/src/ui/filter_strip/mod.rs b/linux/crates/app/src/ui/filter_strip/mod.rs index 406c674463..a3a6ab1f09 100644 --- a/linux/crates/app/src/ui/filter_strip/mod.rs +++ b/linux/crates/app/src/ui/filter_strip/mod.rs @@ -46,16 +46,39 @@ impl FilterStrip { strip } - /// Slide in, or out. Opening puts the cursor in the SQL field, the - /// strip's main input. + /// Slide in, or out, as the Filter button does. pub fn toggle(&self) { - let opening = !self.revealed(); - self.set_revealed(opening); - if opening { - self.imp().raw_entry.grab_focus(); + match self.revealed() { + true => self.close(), + false => self.open(), } } + /// Slide in with the cursor in the SQL field, the strip's main input, + /// or, already in, take the focus there, as Ctrl+F does in a search + /// bar that shows. + pub fn open(&self) { + if crate::ui::a11y::holds_focus(self) { + return; + } + let focus = self.root().and_then(|root| root.focus()); + self.imp().focus_before.set(focus.as_ref()); + self.set_revealed(true); + self.imp().raw_entry.grab_focus(); + } + + /// Slide out. The focus inside goes back where it came from, before + /// GTK moves it to the window's first control. + fn close(&self) { + if crate::ui::a11y::holds_focus(self) + && let Some(before) = self.imp().focus_before.upgrade() + && before.root() == self.root() + { + before.grab_focus(); + } + self.set_revealed(false); + } + /// Rebuild the rows against new columns: each row's column and /// operator choices come from them. pub fn set_columns(&self, columns: Vec) { @@ -129,7 +152,7 @@ impl FilterStrip { fn emit_applied(&self, set: FilterSet) { self.emit_by_name::<()>("applied", &[&glib::BoxedAnyObject::new(set)]); - self.set_revealed(false); + self.close(); } fn add_rule(&self) { diff --git a/linux/crates/app/src/ui/filter_strip/rule_row.rs b/linux/crates/app/src/ui/filter_strip/rule_row.rs index 0e76f37fd1..4481e283f6 100644 --- a/linux/crates/app/src/ui/filter_strip/rule_row.rs +++ b/linux/crates/app/src/ui/filter_strip/rule_row.rs @@ -68,6 +68,12 @@ mod imp { let remove = self.remove_button.get(); remove.set_icon_name(crate::ui::icons::USER_TRASH); crate::ui::a11y::describe_icon_button(&remove, &crate::i18n::gettext("Remove Rule"), None); + // The row has no title, its controls being the rule, and + // libadwaita names a row by its title label alone. + let row = self.obj(); + let row = row.upcast_ref::(); + row.reset_relation(gtk::AccessibleRelation::LabelledBy); + row.update_property(&[gtk::accessible::Property::Label(&crate::i18n::gettext("Filter Rule"))]); } } diff --git a/linux/crates/app/src/ui/filter_strip/rule_value_editor.rs b/linux/crates/app/src/ui/filter_strip/rule_value_editor.rs index 928d0829bc..7daced66c9 100644 --- a/linux/crates/app/src/ui/filter_strip/rule_value_editor.rs +++ b/linux/crates/app/src/ui/filter_strip/rule_value_editor.rs @@ -19,12 +19,14 @@ pub(super) fn value_editor(rule: &FilterRuleObject, column: &ColumnInfo, shape: } fn single_editor(rule: &FilterRuleObject, column: &ColumnInfo) -> gtk::Entry { + let label = crate::i18n::gettext("Value"); let entry = gtk::Entry::builder() - .placeholder_text(crate::i18n::gettext("Value")) + .placeholder_text(&label) .valign(gtk::Align::Center) .hexpand(true) .input_purpose(input_purpose_for(column)) .build(); + name(&entry, &label); if let Some(FilterValue::Single(text)) = rule.rule().value { entry.set_text(&text); } @@ -37,11 +39,13 @@ fn single_editor(rule: &FilterRuleObject, column: &ColumnInfo) -> gtk::Entry { fn pair_editor(rule: &FilterRuleObject, column: &ColumnInfo) -> gtk::Box { let bound = |placeholder: &str| { - gtk::Entry::builder() + let entry = gtk::Entry::builder() .placeholder_text(placeholder) .valign(gtk::Align::Center) .input_purpose(input_purpose_for(column)) - .build() + .build(); + name(&entry, placeholder); + entry }; let low = bound(&crate::i18n::gettext("From")); let high = bound(&crate::i18n::gettext("To")); @@ -87,6 +91,7 @@ fn list_editor(rule: &FilterRuleObject) -> gtk::Entry { .valign(gtk::Align::Center) .hexpand(true) .build(); + name(&entry, &crate::i18n::gettext("Values")); if let Some(FilterValue::List(items)) = rule.rule().value { entry.set_text(&items.join(", ")); } @@ -97,6 +102,11 @@ fn list_editor(rule: &FilterRuleObject) -> gtk::Entry { entry } +/// A placeholder is gone once the field holds text, so it names nothing. +fn name(entry: >k::Entry, label: &str) { + entry.update_property(&[gtk::accessible::Property::Label(label)]); +} + /// The comma-separated items of a list value, with blanks dropped. fn list_items(text: &str) -> Vec { text.split(',') diff --git a/linux/crates/app/src/ui/filter_strip/tests.rs b/linux/crates/app/src/ui/filter_strip/tests.rs index 2c7dffd113..dc7d507c0c 100644 --- a/linux/crates/app/src/ui/filter_strip/tests.rs +++ b/linux/crates/app/src/ui/filter_strip/tests.rs @@ -4,7 +4,8 @@ use super::rule_operators::placeholder_column; use super::*; use crate::application::ShortcutOwner; use crate::test_support::{ - SignalLog, descendants, drain_main_context, first_descendant_of_type, tooltips_with_keys, unlabelled_icon_buttons, + SignalLog, assert_labelled, descendants, drain_main_context, first_descendant_of_type, tooltips_with_keys, + unlabelled_icon_buttons, }; fn rule(column: &str) -> FilterRule { @@ -135,6 +136,33 @@ fn toggle_opens_onto_the_sql_field_and_closes_again() { assert!(!strip.imp().revealer.reveals_child()); } +/// Ctrl+F on an open strip takes the focus into it rather than closing +/// it, and closing gives the focus back to where it came from. +#[gtk4::test] +fn ctrl_f_focuses_an_open_strip_and_close_gives_the_focus_back() { + let strip = strip_with(vec![rule("a")]); + let outside = gtk::Button::with_label("Rows"); + let column = gtk::Box::new(gtk::Orientation::Vertical, 0); + column.append(&strip); + column.append(&outside); + let window = crate::test_support::OwnedWindow::presented(gtk::Window::builder().child(&column).build()); + let focus = || gtk::prelude::GtkWindowExt::focus(&*window); + assert!(outside.grab_focus()); + + strip.open(); + assert!(strip.revealed()); + assert!(crate::ui::a11y::holds_focus(&strip)); + assert!(outside.grab_focus()); + strip.open(); + assert!(strip.revealed(), "Ctrl+F closed the open strip"); + assert!(crate::ui::a11y::holds_focus(&strip)); + + strip.activate_action("filter.close", None).expect("filter.close"); + + assert!(!strip.revealed()); + assert_eq!(focus().as_ref(), Some(outside.upcast_ref())); +} + #[gtk4::test] fn strip_freed_after_drop() { let strip = strip_with(vec![rule("a"), rule("b")]); @@ -218,6 +246,42 @@ fn icon_buttons_are_labelled_without_accelerator_hints() { assert_eq!(tooltips_with_keys(&strip), Vec::::new()); } +/// Placeholders go once text is typed, so each field, and the row that +/// holds a rule, has a name of its own. +#[gtk4::test] +fn every_field_of_the_strip_is_named() { + let rule = |column: &str, op: FilterOp, value: Option| FilterRule { + column: column.to_owned(), + op, + value, + }; + let strip = new_strip( + vec![integer("amount"), placeholder_column("name")], + &FilterSet { + rules: vec![ + rule("name", FilterOp::Eq, Some(FilterValue::Single("x".to_owned()))), + rule( + "amount", + FilterOp::Between, + Some(FilterValue::Pair("1".to_owned(), "2".to_owned())), + ), + rule("name", FilterOp::In, Some(FilterValue::List(vec!["a".to_owned()]))), + ], + ..FilterSet::default() + }, + ); + strip.set_revealed(true); + let fields: Vec = descendants(&strip) + .into_iter() + .filter(|widget| widget.is::() || widget.is::() || widget.is::()) + .collect(); + assert_eq!(fields.len(), 1 + 1 + 3 * 3 + 1 + 2 + 1, "the strip's fields"); + + for field in fields { + assert_eq!(assert_labelled(&field), Ok(()), "{}", field.type_().name()); + } +} + #[gtk4::test] fn an_applied_filter_rebuilds_the_rule_list() { let strip = strip_with(vec![rule("a"), rule("b")]); diff --git a/linux/crates/app/src/ui/grid/calendar_editor.rs b/linux/crates/app/src/ui/grid/calendar_editor.rs index 7412602bc5..c57e4d5493 100644 --- a/linux/crates/app/src/ui/grid/calendar_editor.rs +++ b/linux/crates/app/src/ui/grid/calendar_editor.rs @@ -5,6 +5,7 @@ use tablepro_core::edit::CellInput; use tablepro_core::value::Temporal; use tablepro_core::{ColumnInfo, Value}; +use super::cell_accessibility::name_editor; use super::edit_commit::{commit_edit, send_commit}; use super::inline_editor::{InlineEditor, InlineOptions}; use super::{EditSnapshot, GridCell}; @@ -36,6 +37,7 @@ impl CalendarEditor { }; cell.cancel_edit(); let calendar = gtk::Calendar::new(); + name_editor(&calendar, cell); // Before the handler: set_date emits day-selected when the day // changes, and that is not a pick. if let CalendarStart::On(date) = &start { diff --git a/linux/crates/app/src/ui/grid/cell_accessibility.rs b/linux/crates/app/src/ui/grid/cell_accessibility.rs index b7179409e2..635270877c 100644 --- a/linux/crates/app/src/ui/grid/cell_accessibility.rs +++ b/linux/crates/app/src/ui/grid/cell_accessibility.rs @@ -1,6 +1,23 @@ +use gtk4 as gtk; +use gtk4::prelude::*; + +use super::GridCell; use super::row_state_icon::RowPendingState; use crate::services::change_tracker::CellState; +/// An editor opened over a cell takes its column's name: Tab moves the +/// edit across columns, and the header that names them is never focused. +pub(super) fn name_editor(editor: &impl IsA, cell: &GridCell) { + if let Some(name) = cell + .drawing() + .and_then(|drawing| drawing.catalog.as_ref().map(|column| column.name.clone())) + { + editor + .as_ref() + .update_property(&[gtk::accessible::Property::Label(&name)]); + } +} + /// What a screen reader adds to a cell's value. A new row's cells say /// nothing of their own: the row says it is new. A NULL is named, since /// it reads as the text "NULL" would. diff --git a/linux/crates/app/src/ui/grid/cell_focus.rs b/linux/crates/app/src/ui/grid/cell_focus.rs index 26938c863d..e385abcbb4 100644 --- a/linux/crates/app/src/ui/grid/cell_focus.rs +++ b/linux/crates/app/src/ui/grid/cell_focus.rs @@ -144,7 +144,9 @@ mod tests { use tablepro_core::value::Temporal; use super::edit_row; - use crate::test_support::{FocusThief, after_next_frame, drain_main_context, first_descendant_of_type, wait_until}; + use crate::test_support::{ + FocusThief, after_next_frame, assert_labelled, drain_main_context, first_descendant_of_type, wait_until, + }; use crate::ui::grid::test_grid::{Grid, WAIT, edited_rows, key, named, text}; use crate::ui::grid::{GridCell, begin_edit}; @@ -438,6 +440,50 @@ mod tests { ); } + /// Tab moves an edit across the columns, and no header is focused on + /// the way, so each editor names the column it edits. + #[gtk4::test] + fn every_cell_editor_is_named_by_its_column() { + let grid = Grid::open( + vec![ + named("id", "integer", true), + named("note", "text", false), + named("day", "date", false), + ], + vec![ + vec![Value::Int(1), Value::Text("one\ntwo".into()), day(15)], + vec![Value::Int(2), Value::Text("plain".into()), day(16)], + ], + ); + + let one_line = grid.cell_of(2, 1); + begin_edit(&one_line); + let field = one_line.inline_text().expect("the one-line editor"); + assert_eq!(assert_labelled(&field), Ok(())); + one_line.cancel_edit(); + + let lines = grid.cell_of(1, 1); + begin_edit(&lines); + let view = first_descendant_of_type::(&lines).expect("the multi-line editor"); + assert_eq!(assert_labelled(&view), Ok(())); + let save = crate::test_support::descendants(&lines) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .find(|button| button.label().as_deref() == Some("Save")) + .expect("the Save button"); + assert!(gtk::test_accessible_has_property( + &save, + gtk::AccessibleProperty::KeyShortcuts + )); + lines.cancel_edit(); + + let dated = grid.cell_of(1, 2); + begin_edit(&dated); + let calendar = first_descendant_of_type::(&dated).expect("the calendar"); + assert_eq!(assert_labelled(&calendar), Ok(())); + dated.cancel_edit(); + } + #[gtk4::test] fn the_multi_line_editor_returns_focus_to_cell() { let grid = Grid::open( diff --git a/linux/crates/app/src/ui/grid/column_factory.rs b/linux/crates/app/src/ui/grid/column_factory.rs index dc18329692..113c5841bd 100644 --- a/linux/crates/app/src/ui/grid/column_factory.rs +++ b/linux/crates/app/src/ui/grid/column_factory.rs @@ -288,7 +288,7 @@ fn build_column(init: ColumnInit<'_>) -> (gtk::ColumnViewColumn, Option Vec> { + let mut sorts: Vec> = grid + .take_messages() + .into_iter() + .filter_map(|msg| match msg { + GridMsg::SortChanged(column, ascending) => Some(Some((column, ascending))), + GridMsg::SortCleared => Some(None), + _ => None, + }) + .collect(); + // A sort on a new column notifies the column, then the order. + sorts.dedup(); + sorts + } + + /// A header's click and drag take a pointer, so the menu, which the + /// Menu key opens, sorts and resets the width too. + #[gtk4::test] + fn the_menu_sorts_by_its_column_and_clears_the_sort() { + let grid = two_text_rows(); + open_menu(&grid.cell_of(1, 1)); + choose(&grid, &["cell.clear-sort"]); + assert!(sorts(&grid).is_empty(), "Clear Sort was on with no sort"); + + choose(&grid, &["cell.sort-descending"]); + open_menu(&grid.cell_of(1, 1)); + choose(&grid, &["cell.clear-sort"]); + + assert_eq!(sorts(&grid), [Some((1, false)), None]); + let sorter = grid + .view + .sorter() + .and_downcast::() + .expect("the view's sorter"); + assert!(sorter.primary_sort_column().is_none()); + } + + #[gtk4::test] + fn reset_width_forgets_the_dragged_width() { + let runtime = crate::test_support::test_runtime(); + let root = tempfile::tempdir().expect("tempdir"); + let widths = ColumnWidthBinding { + store: crate::persistence::ColumnWidthStore::load(root.path().join("column-widths.json"), &runtime.tasks()), + connection_id: uuid::Uuid::new_v4(), + schema: None, + table: "t".to_owned(), + }; + let grid = Grid::open_with_widths(two_text_columns(), two_text_values(), widths.clone()); + grid.column(1).set_fixed_width(300); + assert_eq!(widths.width("name"), Some(300)); + + open_menu(&grid.cell_of(1, 1)); + choose(&grid, &["cell.reset-width"]); + + assert_eq!(grid.column(1).fixed_width(), -1); + assert_eq!(widths.width("name"), None); + } + fn shown_menu(grid: &Grid) -> Option { descendants(&grid.view) .into_iter() diff --git a/linux/crates/app/src/ui/grid/grid_columns.rs b/linux/crates/app/src/ui/grid/grid_columns.rs index 80484d5952..ec3085bdd0 100644 --- a/linux/crates/app/src/ui/grid/grid_columns.rs +++ b/linux/crates/app/src/ui/grid/grid_columns.rs @@ -21,6 +21,7 @@ struct GridColumn { /// The view owns its columns, and their cells hold this. view_column: glib::WeakRef, drawing: Rc, + default_width: Option, } #[derive(Clone)] @@ -29,6 +30,8 @@ pub(super) struct ShownColumn { /// The column's place in the result, which is what its cells hold. pub(super) index: usize, drawing: Rc, + /// The width it opens at with none saved, `None` for its natural one. + pub(super) default_width: Option, } impl ShownColumn { @@ -47,11 +50,18 @@ impl ShownColumn { } impl GridColumns { - pub(super) fn push(&self, index: usize, view_column: >k::ColumnViewColumn, drawing: Rc) { + pub(super) fn push( + &self, + index: usize, + view_column: >k::ColumnViewColumn, + drawing: Rc, + default_width: Option, + ) { self.columns.borrow_mut().push(GridColumn { index, view_column: view_column.downgrade(), drawing, + default_width, }); } @@ -89,6 +99,7 @@ impl GridColumns { view_column, index: column.index, drawing: column.drawing.clone(), + default_width: column.default_width, }) }) .collect() diff --git a/linux/crates/app/src/ui/grid/grid_menus.rs b/linux/crates/app/src/ui/grid/grid_menus.rs index 9556f4b0cf..3fb2784762 100644 --- a/linux/crates/app/src/ui/grid/grid_menus.rs +++ b/linux/crates/app/src/ui/grid/grid_menus.rs @@ -10,6 +10,7 @@ use super::cell_edit::begin_edit; use super::cell_editor_kind::CellEditorKind; use super::cell_preset::cell_presets; use super::context_menu::{CellContext, MenuTarget}; +use super::grid_columns::ShownColumn; use super::row_json_dialog::RowJsonRequest; use super::selection::export_snapshot; use super::{ @@ -31,6 +32,8 @@ pub(super) struct CellActions { /// Shown insensitive while disabled, since a menu item cannot carry /// a tooltip to say why. delete_row: Option, + /// On while the rows are sorted. + clear_sort: Option, } impl CellActions { @@ -64,6 +67,12 @@ impl CellActions { let availability = delete_availability(rows.iter().map(RowObject::id), false); delete_row.set_enabled(availability == DeleteAvailability::Available); } + let sorted = cell + .column_view() + .and_then(|view| view.sorter()) + .and_downcast::() + .is_some_and(|sorter| sorter.primary_sort_column().is_some()); + enable(&self.clear_sort, sorted); } } @@ -160,6 +169,23 @@ fn build_cell_menu(shape: MenuShape) -> gio::Menu { ); menu.append_section(None, &json_section); + // A header's click and drag, which no key reaches. + let column_section = gio::Menu::new(); + column_section.append( + Some(&crate::i18n::gettext("Sort Ascending")), + Some("cell.sort-ascending"), + ); + column_section.append( + Some(&crate::i18n::gettext("Sort Descending")), + Some("cell.sort-descending"), + ); + column_section.append(Some(&crate::i18n::gettext("Clear Sort")), Some("cell.clear-sort")); + column_section.append( + Some(&crate::i18n::gettext("Reset Column Width")), + Some("cell.reset-width"), + ); + menu.append_section(None, &column_section); + let action_section = gio::Menu::new(); action_section.append(Some(&crate::i18n::gettext("Export…")), Some("cell.export")); if shape.row_ops { @@ -211,6 +237,7 @@ pub(super) fn install_grid_context_menus(init: GridMenuInit<'_>) -> GridMenus { set_null: editable.then(|| simple_action(&group, "set-null")).flatten(), set_default: editable.then(|| simple_action(&group, "set-default")).flatten(), delete_row: editable.then(|| simple_action(&group, "delete-row")).flatten(), + clear_sort: simple_action(&group, "clear-sort"), }; let (editable_popover, readonly_popover) = cell_popovers(column_view, editable); @@ -290,9 +317,55 @@ impl CellActionKit { fn reading_entries(&self) -> Vec { let mut entries = self.copy_entries(); entries.extend([self.copy_column_name(), self.show_row_json(), self.export()]); + entries.extend(self.column_entries()); entries } + /// Sort by the cell's column, as a header click does, or reset the + /// width a header drag set. + fn column_entries(&self) -> Vec { + let sort = |name: &str, order: gtk::SortType| { + let view = self.view.clone(); + self.on_target(name, move |cell, _row, _rows| { + let Some(cv) = view.upgrade() else { return }; + if let Some(column) = shown_column(&cv, cell) { + cv.sort_by_column(Some(&column.view_column), order); + } + }) + }; + let clear_sort = { + let view = self.view.clone(); + let sink = self.sink.clone(); + self.on_target("clear-sort", move |_cell, _row, _rows| { + let Some(cv) = view.upgrade() else { return }; + cv.sort_by_column(None, gtk::SortType::Ascending); + // The server sorts a browse page, and an unsorted view + // tells it nothing by itself. + sink(GridMsg::SortCleared); + }) + }; + let reset_width = { + let view = self.view.clone(); + let tab = self.tab.clone(); + let columns = self.columns.clone(); + self.on_target("reset-width", move |cell, _row, _rows| { + let Some(column) = view.upgrade().and_then(|cv| shown_column(&cv, cell)) else { + return; + }; + column.view_column.set_fixed_width(column.default_width.unwrap_or(-1)); + if let (Some(widths), Some(info)) = (&tab.column_widths, columns.get(column.index)) { + widths.forget(&info.name); + } + }) + }; + vec![ + sort("sort-ascending", gtk::SortType::Ascending), + sort("sort-descending", gtk::SortType::Descending), + clear_sort, + reset_width, + ] + } + fn copy_entries(&self) -> Vec { vec![ // Plain Copy reads the cell's value, never the widget's text: @@ -450,6 +523,14 @@ fn install_empty_space_menu(column_view: >k::ColumnView) -> gtk::PopoverMenu { empty_popover } +/// The view's column that `cell` sits in. +fn shown_column(view: >k::ColumnView, cell: &GridCell) -> Option { + cell.columns()? + .shown(view) + .into_iter() + .find(|column| column.index == cell.column()) +} + /// The rows a menu item copies, as the grid shows them. fn snapshots(tab: &TabGridContext, rows: &[RowObject]) -> Vec { rows.iter().map(|row| tab.snapshot(row)).collect() diff --git a/linux/crates/app/src/ui/grid/grid_msg.rs b/linux/crates/app/src/ui/grid/grid_msg.rs index 01708c78c6..62eb484e8a 100644 --- a/linux/crates/app/src/ui/grid/grid_msg.rs +++ b/linux/crates/app/src/ui/grid/grid_msg.rs @@ -14,6 +14,8 @@ pub enum GridMsg { /// The sorter's state after the click, not a toggle hint: a click on /// another column fires both sort notifies, and a hint would flip twice. SortChanged(usize, bool), + /// Clear Sort: the rows go back to the order the server sends. + SortCleared, /// An edit of `column` in `row`, already parsed for the column. Edit { row: RowObject, diff --git a/linux/crates/app/src/ui/grid/inline_editor.rs b/linux/crates/app/src/ui/grid/inline_editor.rs index 67d097dc90..911d4b5faa 100644 --- a/linux/crates/app/src/ui/grid/inline_editor.rs +++ b/linux/crates/app/src/ui/grid/inline_editor.rs @@ -4,6 +4,7 @@ use std::rc::Rc; use gtk4::prelude::*; use gtk4::{self as gtk, glib}; +use super::cell_accessibility::name_editor; use super::cell_focus::{edit_next, return_focus}; use super::edit_commit::{commit_edit, send_commit}; use super::edit_navigation::EditDirection; @@ -40,6 +41,7 @@ impl InlineEditor { text.set_input_purpose(options.purpose); text.set_placeholder_text(options.placeholder.as_deref()); text.set_text(initial); + name_editor(&text, cell); connect_commit(cell, &text); if let Some(display) = cell.content() { display.set_visible(false); diff --git a/linux/crates/app/src/ui/grid/multiline_editor.rs b/linux/crates/app/src/ui/grid/multiline_editor.rs index 38f9a3eb62..c6128de9b1 100644 --- a/linux/crates/app/src/ui/grid/multiline_editor.rs +++ b/linux/crates/app/src/ui/grid/multiline_editor.rs @@ -2,6 +2,7 @@ use gtk4::prelude::*; use gtk4::{self as gtk, glib}; use sourceview5::prelude::*; +use super::cell_accessibility::name_editor; use super::edit_commit::{commit_edit, send_commit}; use super::{EditSnapshot, GridCell}; use crate::application::{ShortcutOwner, component_trigger, controller_name}; @@ -24,6 +25,7 @@ impl MultilineEditor { view.set_wrap_mode(gtk::WrapMode::WordChar); view.set_auto_indent(true); crate::ui::style_scheme_resolver::apply(&view, "Adwaita"); + name_editor(&view, cell); let scrolled = gtk::ScrolledWindow::builder() .child(&view) @@ -33,6 +35,7 @@ impl MultilineEditor { .build(); let save = gtk::Button::with_label(&crate::i18n::gettext("Save")); save.add_css_class("suggested-action"); + crate::ui::a11y::announce_key_shortcuts(&save, "cell-editor.save-text"); let cancel = gtk::Button::with_label(&crate::i18n::gettext("Cancel")); let buttons = gtk::Box::builder() .orientation(gtk::Orientation::Horizontal) diff --git a/linux/crates/app/src/ui/grid/result_grid.rs b/linux/crates/app/src/ui/grid/result_grid.rs index b0de508e93..a9531ac8c2 100644 --- a/linux/crates/app/src/ui/grid/result_grid.rs +++ b/linux/crates/app/src/ui/grid/result_grid.rs @@ -270,6 +270,51 @@ mod tests { window.close(); } + #[gtk4::test] + fn the_cell_menu_sorts_the_results_and_restores_their_order() { + let columns = [named("id", "integer", false), named("name", "text", false)]; + let (view, selection) = grid( + &columns, + vec![ + vec![Value::Int(2), text("b")], + vec![Value::Int(3), text("c")], + vec![Value::Int(1), text("a")], + ], + recorder().0, + ); + let window = gtk::Window::new(); + window.set_default_size(600, 400); + window.set_child(Some(&view)); + window.present(); + let cell = || { + descendants(&view) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .find(|cell| cell.position() == Some(0) && cell.column() == 0 && cell.row().is_some()) + }; + wait_until(WAIT, || cell().is_some()).expect("the cells are drawn"); + let open_menu = || { + let cell = cell().expect("the first row's cell"); + let controllers = cell.observe_controllers(); + let gesture = (0..controllers.n_items()) + .filter_map(|index| controllers.item(index).and_downcast::()) + .find(|gesture| gesture.button() == 3) + .expect("a cell answers a right-click"); + gesture.emit_by_name::<()>("pressed", &[&1i32, &1.0f64, &1.0f64]); + }; + + open_menu(); + view.activate_action("cell.sort-descending", None) + .expect("the menu's Sort Descending"); + assert_eq!(shown(&selection, 0), [Value::Int(3), Value::Int(2), Value::Int(1)]); + + open_menu(); + view.activate_action("cell.clear-sort", None) + .expect("the menu's Clear Sort"); + assert_eq!(shown(&selection, 0), [Value::Int(2), Value::Int(3), Value::Int(1)]); + window.close(); + } + #[gtk4::test] fn copy_after_sort_copies_displayed_row() { let columns = [named("id", "integer", false), named("name", "text", false)]; diff --git a/linux/crates/app/src/ui/grid/test_grid.rs b/linux/crates/app/src/ui/grid/test_grid.rs index de69a97fd2..ee98a3b8b8 100644 --- a/linux/crates/app/src/ui/grid/test_grid.rs +++ b/linux/crates/app/src/ui/grid/test_grid.rs @@ -10,8 +10,8 @@ use tablepro_core::meta::RowIdentity; use tablepro_core::{ColumnInfo, Value}; use super::{ - GridCell, GridContent, GridMsg, GridSink, GridSorting, RowObject, TabGridContext, begin_edit, build_column_view, - data_columns, + ColumnWidthBinding, GridCell, GridContent, GridMsg, GridSink, GridSorting, RowObject, TabGridContext, begin_edit, + build_column_view, data_columns, }; use crate::services::change_tracker::RowKey; use crate::services::tab_tracker_handle::TabTrackerHandle; @@ -109,6 +109,19 @@ pub(super) struct Grid { impl Grid { /// `rows` keyed by their first cell, in a grid that edits them. pub(super) fn open(columns: Vec, rows: Vec>) -> Self { + Self::open_in(columns, rows, None) + } + + /// As `open`, with the column widths saved through `widths`. + pub(super) fn open_with_widths( + columns: Vec, + rows: Vec>, + widths: ColumnWidthBinding, + ) -> Self { + Self::open_in(columns, rows, Some(widths)) + } + + fn open_in(columns: Vec, rows: Vec>, widths: Option) -> Self { let tracker = TabTrackerHandle::default(); let (sink, messages) = recorder(); let (view, selection) = build_column_view( @@ -116,7 +129,10 @@ impl Grid { &columns, sink, GridSorting::Server { initial: None }, - tracked(&tracker), + TabGridContext { + column_widths: widths, + ..tracked(&tracker) + }, ); let store = selection .model() @@ -233,13 +249,18 @@ pub(super) fn focus_in_grid(grid: &Grid) -> bool { } pub(super) fn two_text_rows() -> Grid { - Grid::open( - vec![named("id", "integer", true), named("name", "text", false)], - vec![ - vec![Value::Int(1), Value::Text("A".into())], - vec![Value::Int(2), Value::Text("B".into())], - ], - ) + Grid::open(two_text_columns(), two_text_values()) +} + +pub(super) fn two_text_columns() -> Vec { + vec![named("id", "integer", true), named("name", "text", false)] +} + +pub(super) fn two_text_values() -> Vec> { + vec![ + vec![Value::Int(1), Value::Text("A".into())], + vec![Value::Int(2), Value::Text("B".into())], + ] } /// Open the cell's one-line editor and type `text` over its value. diff --git a/linux/crates/app/src/ui/history_dialog/tests.rs b/linux/crates/app/src/ui/history_dialog/tests.rs index e7c47cf10e..c59e5648a4 100644 --- a/linux/crates/app/src/ui/history_dialog/tests.rs +++ b/linux/crates/app/src/ui/history_dialog/tests.rs @@ -212,6 +212,30 @@ pub(super) fn menu_item(menu: &gio::MenuModel, label: &str) -> Option<(String, O None } +/// Every row has the same check box, Pin and Delete, so each says which +/// query it acts on. +#[gtk4::test] +fn each_rows_controls_name_their_query() { + let harness = Harness::new(); + harness.record("SELECT 1"); + let dialog = harness.presented_dialog(); + wait_until(WAIT, || !rows(&dialog).is_empty()).expect("the row"); + let row = rows(&dialog).remove(0); + + let controls: Vec = descendants(&row) + .into_iter() + .filter(|widget| widget.is::() || widget.is::()) + .collect(); + assert_eq!(controls.len(), 3, "the check box, Pin and Delete"); + for control in controls { + assert!( + gtk::test_accessible_has_relation(&control, gtk::AccessibleRelation::DescribedBy), + "{} does not name its query", + control.type_().name() + ); + } +} + #[gtk4::test] fn list_view_shows_pinned_section_header() { let harness = Harness::new(); diff --git a/linux/crates/app/src/ui/result_pane/mod.rs b/linux/crates/app/src/ui/result_pane/mod.rs index ed8be63dd5..b9a406d42d 100644 --- a/linux/crates/app/src/ui/result_pane/mod.rs +++ b/linux/crates/app/src/ui/result_pane/mod.rs @@ -17,16 +17,18 @@ use crate::ui::export::ExportSource; use crate::ui::format::{format_thousands, plural_count}; use crate::ui::grid::{GridSink, RowObject, TabGridContext, build_result_grid, export_snapshot}; -/// The page for `view`. Export All on its banner hands `export_all` the -/// rows the grid shows, in the order it shows them, for the dialog's -/// Shown Rows. +/// The page for `view`, its grid named `name` as the results list names +/// it. Export All on its banner hands `export_all` the rows the grid +/// shows, in the order it shows them, for the dialog's Shown Rows. pub(crate) fn result_set_page( view: &ResultSetView, + name: &str, sink: GridSink, export_all: impl Fn(ExportSource) + 'static, ) -> gtk::Widget { let rows = view.rows.iter().cloned().map(RowObject::result).collect(); let (column_view, _selection) = build_result_grid(view.columns.clone(), rows, sink); + column_view.update_property(&[gtk::accessible::Property::Label(name)]); let banner = ResultLimitBanner::new(&view.summary); let columns = view.columns.clone(); let grid = column_view.downgrade(); diff --git a/linux/crates/app/src/ui/saved_queries_dialog.rs b/linux/crates/app/src/ui/saved_queries_dialog.rs index 5cf5faab47..65e1fe0a6d 100644 --- a/linux/crates/app/src/ui/saved_queries_dialog.rs +++ b/linux/crates/app/src/ui/saved_queries_dialog.rs @@ -82,10 +82,13 @@ impl Component for SavedQueriesDialog { header.set_title_widget(Some(&adw::WindowTitle::new(&crate::i18n::gettext("Saved Queries"), ""))); toolbar.add_top_bar(&header); + let search_label = crate::i18n::gettext("Search saved queries"); let search = gtk::SearchEntry::builder() - .placeholder_text(crate::i18n::gettext("Search saved queries")) + .placeholder_text(&search_label) .hexpand(true) .build(); + // A placeholder is gone once text is typed, so it names nothing. + search.update_property(&[gtk::accessible::Property::Label(&search_label)]); let search_bar = gtk::Box::builder() .orientation(gtk::Orientation::Horizontal) .margin_start(12) @@ -608,6 +611,18 @@ mod dialog_tests { .expect("the search did not narrow the list"); } + /// No scenario opens the dialog, so its one field is checked here. + #[gtk4::test] + fn the_search_field_is_named() { + let (_root, store, local, _staging) = seeded(); + let dialog = SavedQueriesDialog::builder().launch(SavedQueriesDialogInit { + store: Some(store), + active_connection: Some(local), + }); + + assert_eq!(crate::test_support::assert_labelled(&dialog.model().search), Ok(())); + } + #[gtk4::test] fn deleting_a_query_takes_it_out_of_the_list() { let (_root, store, local, _staging) = seeded(); diff --git a/linux/crates/app/src/ui/structure_tab/columns.rs b/linux/crates/app/src/ui/structure_tab/columns.rs index 4ebd23b281..23b8e09c2c 100644 --- a/linux/crates/app/src/ui/structure_tab/columns.rs +++ b/linux/crates/app/src/ui/structure_tab/columns.rs @@ -65,16 +65,13 @@ fn mark_invalid(row: &adw::EntryRow, error: Option<&str>) { Some(message) => { row.add_css_class("error"); row.set_tooltip_text(Some(message)); - row.upcast_ref::() - .update_property(&[gtk::accessible::Property::Description(message)]); } None => { row.remove_css_class("error"); row.set_tooltip_text(None); - row.upcast_ref::() - .reset_property(gtk::AccessibleProperty::Description); } } + crate::ui::a11y::show_invalid(row, error); } /// Whether the engine's parser takes `text` as one type. The diff --git a/linux/crates/app/src/ui/structure_tab/foreign_key_dialog.rs b/linux/crates/app/src/ui/structure_tab/foreign_key_dialog.rs index 5171ffb83d..a6cba60df3 100644 --- a/linux/crates/app/src/ui/structure_tab/foreign_key_dialog.rs +++ b/linux/crates/app/src/ui/structure_tab/foreign_key_dialog.rs @@ -393,24 +393,28 @@ impl Inner { ForeignKeyDraftError::UnpairedColumn(_) => loaded, }) .collect(); + let problem_of = |about: &dyn Fn(&ForeignKeyDraftError) -> bool| { + shown.iter().find(|error| about(error)).map(|error| error.message()) + }; mark_invalid( &self.name_row, - shown.iter().any(|error| { + problem_of(&|error| { matches!( error, ForeignKeyDraftError::NameMissing | ForeignKeyDraftError::NameTaken ) - }), + }) + .as_deref(), ); mark_invalid( &self.table_row, - shown.iter().any(|error| matches!(error, ForeignKeyDraftError::NoTable)), + problem_of(&|error| matches!(error, ForeignKeyDraftError::NoTable)).as_deref(), ); for (column, row) in self.pair_rows.borrow().iter() { - let unpaired = shown - .iter() - .any(|error| matches!(error, ForeignKeyDraftError::UnpairedColumn(unpaired) if unpaired == column)); - mark_invalid(row, unpaired); + let unpaired = problem_of( + &|error| matches!(error, ForeignKeyDraftError::UnpairedColumn(unpaired) if unpaired == column), + ); + mark_invalid(row, unpaired.as_deref()); } let problem = self .load_error @@ -713,6 +717,18 @@ mod tests { let pair = harness.pair_row("account_id"); assert_eq!(pair.selected(), 0); assert!(pair.has_css_class("error")); + let field = pair.upcast_ref::(); + assert!(gtk::test_accessible_has_state(field, gtk::AccessibleState::Invalid)); + assert!(gtk::test_accessible_has_property( + field, + gtk::AccessibleProperty::Description + )); + assert_eq!( + crate::ui::a11y::announced_in(&*harness.window) + .last() + .map(|(said, _)| said.as_str()), + Some("Choose the column “account_id” references.") + ); assert!(!inner.form.submit.is_sensitive()); assert_eq!( inner.form.caption().as_deref(), @@ -722,6 +738,10 @@ mod tests { pair.set_selected(1); assert!(!pair.has_css_class("error")); + assert!(!gtk::test_accessible_has_state( + pair.upcast_ref::(), + gtk::AccessibleState::Invalid + )); assert!(inner.form.submit.is_sensitive()); assert_eq!( inner.draft.borrow().pairs, diff --git a/linux/crates/app/src/ui/structure_tab/form_dialog.rs b/linux/crates/app/src/ui/structure_tab/form_dialog.rs index c0957786cc..7d53c77244 100644 --- a/linux/crates/app/src/ui/structure_tab/form_dialog.rs +++ b/linux/crates/app/src/ui/structure_tab/form_dialog.rs @@ -93,12 +93,18 @@ impl FormDialog { } /// Submit on only for a valid form, and `problem` under the fields. + /// A new problem is said: Enter does nothing until it is fixed, and + /// the caption sits below the field that has the focus. pub(super) fn show_state(&self, valid: bool, problem: Option) { self.submit.set_sensitive(valid); match problem { Some(problem) => { + let news = !self.caption.is_visible() || self.caption.label() != problem; self.caption.set_label(&problem); self.caption.set_visible(true); + if news { + crate::ui::a11y::announce(&self.caption, &problem, gtk::AccessibleAnnouncementPriority::Medium); + } } None => self.caption.set_visible(false), } @@ -125,12 +131,14 @@ pub(super) fn give_state(dialog: &adw::Dialog, state: Rc) { let _owner = dialog.add_weak_ref_notify_local(move || drop(state)); } -/// The `error` style on a field the problem shown is about. -pub(super) fn mark_invalid(widget: &impl IsA, invalid: bool) { - match invalid { - true => widget.add_css_class("error"), - false => widget.remove_css_class("error"), +/// The `error` style on a field the problem shown is about, and the +/// problem on the field a screen reader is on. +pub(super) fn mark_invalid(widget: &impl IsA, problem: Option<&str>) { + match problem { + Some(_) => widget.add_css_class("error"), + None => widget.remove_css_class("error"), } + crate::ui::a11y::show_invalid(widget, problem); } /// A GtkPopoverMenu reads an underscore in a label as a mnemonic, so a diff --git a/linux/crates/app/src/ui/structure_tab/index_dialog.rs b/linux/crates/app/src/ui/structure_tab/index_dialog.rs index 9373de8f1b..54df15c48b 100644 --- a/linux/crates/app/src/ui/structure_tab/index_dialog.rs +++ b/linux/crates/app/src/ui/structure_tab/index_dialog.rs @@ -154,10 +154,10 @@ impl Inner { .copied() .find(|error| matches!(error, IndexDraftError::NameMissing | IndexDraftError::NameTaken)) .filter(|_| self.name_edited.get()); - mark_invalid(&self.name_row, name_error.is_some()); + let problem = name_error.map(IndexDraftError::message); + mark_invalid(&self.name_row, problem.as_deref()); // An empty key list says so in the group's description. - self.form - .show_state(errors.is_empty(), name_error.map(IndexDraftError::message)); + self.form.show_state(errors.is_empty(), problem); self.show_keys(); } @@ -358,8 +358,16 @@ mod tests { assert!(name_row.has_css_class("error")); assert_eq!(dialog.inner.form.caption().as_deref(), Some("Index name is required.")); + // The focus is in the row's text field, under which no caption shows. + let field = name_row.delegate().expect("the row's text field"); + assert!(gtk::test_accessible_has_state(&field, gtk::AccessibleState::Invalid)); + assert!(gtk::test_accessible_has_property( + &field, + gtk::AccessibleProperty::Description + )); name_row.set_text("by_email"); assert!(!name_row.has_css_class("error")); + assert!(!gtk::test_accessible_has_state(&field, gtk::AccessibleState::Invalid)); } #[gtk4::test] diff --git a/linux/crates/app/src/ui/structure_tab/save_handlers.rs b/linux/crates/app/src/ui/structure_tab/save_handlers.rs index aad4225b38..8a6763aec0 100644 --- a/linux/crates/app/src/ui/structure_tab/save_handlers.rs +++ b/linux/crates/app/src/ui/structure_tab/save_handlers.rs @@ -209,7 +209,7 @@ impl StructureModel { u32::try_from(pending_count).unwrap_or(u32::MAX), &[("n", &pending_count.to_string())], ); - self.widgets.pending_label.set_label(&label); + crate::ui::a11y::show_status(&self.widgets.pending_label, &label); self.widgets.pending_label.set_visible(true); } else { self.widgets.pending_label.set_visible(false); diff --git a/linux/crates/app/src/ui/structure_tab/tests.rs b/linux/crates/app/src/ui/structure_tab/tests.rs index d227079f9b..cfef2dd4db 100644 --- a/linux/crates/app/src/ui/structure_tab/tests.rs +++ b/linux/crates/app/src/ui/structure_tab/tests.rs @@ -881,6 +881,19 @@ fn preview(tab: &StructureTab) -> String { buffer.text(&buffer.start_iter(), &buffer.end_iter(), false).to_string() } +/// The preview is text a reader can move through, so it takes a name. +#[gtk4::test] +fn the_sql_preview_is_named() { + let mut script = postgres_script(); + script.fetch_table_definition.always(FakeReply::ok(keyed_users())); + let harness = Harness::open(StructureMode::Edit, script, []); + + assert_eq!( + crate::test_support::assert_labelled(&harness.tab().imp().sql_view.get()), + Ok(()) + ); +} + #[gtk4::test] fn renaming_existing_column_enables_save_and_previews_rename() { let mut script = postgres_script(); @@ -901,6 +914,13 @@ fn renaming_existing_column_enables_save_and_previews_rename() { assert_eq!(harness.column_names(), ["id", "points"]); assert!(imp.save_button.is_sensitive(), "a rename left Save off"); assert_eq!(imp.pending_label.label(), "1 unsaved change"); + assert_eq!( + crate::ui::a11y::announced_in(&imp.pending_label.get()), + [( + "1 unsaved change".to_owned(), + gtk4::AccessibleAnnouncementPriority::Medium + )] + ); assert_eq!( preview(harness.tab()), "ALTER TABLE \"users\" RENAME COLUMN \"score\" TO \"points\";" diff --git a/linux/crates/app/src/ui/tab_load/stale_banner.rs b/linux/crates/app/src/ui/tab_load/stale_banner.rs index 21a97459c0..d290c73a8b 100644 --- a/linux/crates/app/src/ui/tab_load/stale_banner.rs +++ b/linux/crates/app/src/ui/tab_load/stale_banner.rs @@ -19,12 +19,23 @@ impl StaleBanner { Self { banner } } + /// A new failure is said, with its reason: the focus stays in the + /// content, and the banner's reason shows only on hover. pub(crate) fn show(&self, stale: Option<&LoadFailure>) { let message = stale.map(LoadFailure::message); + let news = message.is_some() + && (!self.banner.is_revealed() || self.banner.tooltip_text().as_deref() != message.as_deref()); self.banner.set_tooltip_text(message.as_deref()); self.banner .upcast_ref::() .update_property(&[gtk::accessible::Property::Description(message.as_deref().unwrap_or(""))]); self.banner.set_revealed(stale.is_some()); + if let Some(message) = message.filter(|_| news) { + crate::ui::a11y::announce( + &self.banner, + &format!("{} {message}", self.banner.title()), + gtk::AccessibleAnnouncementPriority::High, + ); + } } } diff --git a/linux/crates/app/src/ui/workspace/close_flow.rs b/linux/crates/app/src/ui/workspace/close_flow.rs index 7ac36e62ad..f0b1b73e94 100644 --- a/linux/crates/app/src/ui/workspace/close_flow.rs +++ b/linux/crates/app/src/ui/workspace/close_flow.rs @@ -148,9 +148,20 @@ impl WorkspaceModel { /// window whether a logout would, so it holds the session back. pub(super) fn refresh_unsaved(&self) { let unsaved: HashSet = self.workspace_handle.dirty_tabs().iter().map(|tab| tab.id).collect(); + let marked: Vec = self + .workspace_tabs + .borrow() + .iter() + .filter(|(id, tab)| tab.page().icon().is_some() != unsaved.contains(id)) + .map(|(id, _)| *id) + .collect(); for (id, tab) in self.workspace_tabs.borrow().iter() { apply_unsaved_icon(tab.page(), unsaved.contains(id)); } + // The tooltip carries the mark to a screen reader. + for id in marked { + self.retitle_tab(id); + } let dirty = !unsaved.is_empty(); if self.reported_dirty.replace(Some(dirty)) == Some(dirty) { return; diff --git a/linux/crates/app/src/ui/workspace/health_banner.rs b/linux/crates/app/src/ui/workspace/health_banner.rs index a10b72b183..df02315312 100644 --- a/linux/crates/app/src/ui/workspace/health_banner.rs +++ b/linux/crates/app/src/ui/workspace/health_banner.rs @@ -39,10 +39,20 @@ impl WorkspaceModel { BannerButton::Retry => (crate::i18n::gettext("Retry"), "win.reconnect".to_owned()), BannerButton::ReviewKey => (crate::i18n::gettext("Review Key…"), format!("win.{REVIEW_HOST_KEY}")), }; + let news = !self.reconnect_banner.is_revealed() || self.reconnect_banner.title() != title; self.reconnect_banner.set_title(&title); self.reconnect_banner.set_button_label(Some(&label)); self.reconnect_banner.set_action_name(Some(&action)); self.reconnect_banner.set_revealed(true); + // The focus stays in the grid or the editor, under a banner no + // screen reader reads by itself. + if news { + crate::ui::a11y::announce( + &self.reconnect_banner, + &title, + gtk4::AccessibleAnnouncementPriority::High, + ); + } } /// What the server or the settings said when the session parked, @@ -456,6 +466,18 @@ mod tests { assert!(review_enabled(&test)); } + #[gtk4::test] + fn the_banner_is_said_when_it_shows() { + let test = parked("BannerSaid", vec![asks("bastion", FINGERPRINT)], None); + + let banner = wait_for_button(&test, "Review Key…"); + + assert_eq!( + crate::ui::a11y::announced_in(&banner).last(), + Some(&(banner.title().to_string(), gtk::AccessibleAnnouncementPriority::High)) + ); + } + #[gtk4::test] fn review_trust_retries_trusting_that_key() { let test = parked( diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index 34e5211e3d..e96407f476 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -319,10 +319,17 @@ impl WorkspaceTab { } /// Give `page` its title and tooltip. libadwaita treats an empty tooltip -/// as none, which clears the last one. +/// as none, which clears the last one. The tooltip says what the unsaved +/// icon shows, since a screen reader reads a tab's tooltip and not its +/// icon. pub(crate) fn apply_tab_title(page: &adw::TabPage, title: &tab_title::TabTitle) { page.set_title(&title.label); - page.set_tooltip(title.tooltip.as_deref().unwrap_or_default()); + let unsaved = page.icon().is_some().then(|| crate::i18n::gettext("Unsaved changes")); + let lines: Vec<&str> = [title.tooltip.as_deref(), unsaved.as_deref()] + .into_iter() + .flatten() + .collect(); + page.set_tooltip(&lines.join("\n")); } #[derive(Debug, Clone, Copy)] diff --git a/linux/crates/app/src/ui/workspace/tab_title_tests.rs b/linux/crates/app/src/ui/workspace/tab_title_tests.rs index a1b4d14f17..affbcd2aa8 100644 --- a/linux/crates/app/src/ui/workspace/tab_title_tests.rs +++ b/linux/crates/app/src/ui/workspace/tab_title_tests.rs @@ -80,9 +80,13 @@ fn unsaved_icon_follows_structure_tracker() { edit_structure(&workspace, id, a_column_added()); wait_until(WAIT, || modified(&page)).expect("the unsaved icon"); assert_eq!(page.title(), "New Table", "the title changed with the edit"); + // A screen reader reads the tab's tooltip, never its icon. + let tooltip = || page.tooltip().map(|tooltip| tooltip.to_string()).unwrap_or_default(); + assert!(tooltip().ends_with("Unsaved changes"), "{}", tooltip()); edit_structure(&workspace, id, None); wait_until(WAIT, || page.icon().is_none()).expect("the icon to go"); + assert!(!tooltip().contains("Unsaved changes"), "{}", tooltip()); } #[gtk4::test] diff --git a/linux/crates/app/tests/accessibility/audit.rs b/linux/crates/app/tests/accessibility/audit.rs index cdcf43f36a..c724d4e68c 100644 --- a/linux/crates/app/tests/accessibility/audit.rs +++ b/linux/crates/app/tests/accessibility/audit.rs @@ -138,7 +138,10 @@ fn judge(widget: >k4::Widget) -> Option { } fn judge_row(row: &adw::PreferencesRow) -> Option { - 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(row.upcast_ref::(), AccessibleProperty::Label) + { return Some(Missing::RowTitle); } if !row.is::() { @@ -160,7 +163,11 @@ fn named(widget: >k4::Widget) -> bool { /// of its own. A text view that never takes the focus is text on the /// page rather than a field. fn is_field(widget: >k4::Widget) -> bool { - widget.is::() + // A bare text field, such as a cell's editor; one inside an entry or + // a spin button is met as that control. + widget.is::() + || widget.is::() + || widget.is::() || widget.is::() || widget.is::() || widget.is::() diff --git a/linux/crates/app/tests/accessibility/main.rs b/linux/crates/app/tests/accessibility/main.rs index ab666d6c80..ced91f615e 100644 --- a/linux/crates/app/tests/accessibility/main.rs +++ b/linux/crates/app/tests/accessibility/main.rs @@ -37,11 +37,12 @@ use world::{World, drain_main_context, wait_until}; type Scenario = fn() -> Result, String>; -const SCENARIOS: [(&str, Scenario); 6] = [ +const SCENARIOS: [(&str, Scenario); 7] = [ ("picker_page_controls_are_labelled", picker_page), ("connecting_page_controls_are_labelled", connecting_page), ("connect_failed_page_controls_are_labelled", connect_failed_page), ("workspace_controls_are_labelled", workspace), + ("structure_forms_are_labelled", structure_forms), ("connect_dialog_fields_are_labelled", connect_dialog), ("preferences_dialog_fields_are_labelled", preferences_dialog), ]; @@ -135,39 +136,14 @@ fn connect_failed_page() -> Result, String> { } /// A table tab, a structure tab and an editor tab, each audited while it -/// is the one in front, with the sidebar's search and the editor's find -/// and replace open. +/// is the one in front, with the table's filter strip, the sidebar's +/// search and the editor's find and replace open, then the table tab +/// again with a cell's editor open. fn workspace() -> Result, String> { - let script = one_table(); - let world = World::start( - "Workspace", - fake_driver(), - FakeConnector::new([]).always(FakeEstablish::Connect(script)), - )?; - let window = world.open(WindowLaunch::Connect(Box::new(world.saved.clone())))?; - wait_until("the table in the sidebar", || { - shows(&window, "TableProWorkspace") && shows_text(window.upcast_ref(), TABLE) - })?; - let sidebar = descendants(window.upcast_ref()) - .into_iter() - .find(|widget| widget.is::()) - .ok_or("the sidebar has no list")?; - let table = (Some("public".to_owned()), TABLE.to_owned()).to_variant(); - - sidebar - .activate_action("sidebar.open-in-new-tab", Some(&table)) - .map_err(|error| format!("sidebar.open-in-new-tab: {error}"))?; - wait_until("the table's rows", || shows_text(window.upcast_ref(), CELL))?; - sidebar - .activate_action("sidebar.edit-structure", Some(&table)) - .map_err(|error| format!("sidebar.edit-structure: {error}"))?; - wait_until("the table's structure", || { - widgets_of(window.upcast_ref(), "TableProStructureTab") - .iter() - .flat_map(descendants) - .filter_map(|widget| widget.downcast::().ok()) - .any(|stack| stack.visible_child_name().as_deref() == Some("editor")) - })?; + let (_world, window) = connected("Workspace")?; + open_table(&window)?; + open_filter_strip(&window)?; + open_structure(&window)?; WidgetExt::activate_action(&window, "win.new-editor-tab", None) .map_err(|error| format!("win.new-editor-tab: {error}"))?; let mut editor = None; @@ -199,9 +175,131 @@ fn workspace() -> Result, String> { drain_main_context(); merge(&mut findings, audit(&window)); } + tabs.set_selected_page(&tabs.nth_page(0)); + drain_main_context(); + open_cell_editor(&window)?; + merge(&mut findings, audit(&window)); + Ok(findings) +} + +/// The Add Index and Add Foreign Key forms of the structure tab. +fn structure_forms() -> Result, String> { + let (_world, window) = connected("StructureForms")?; + open_structure(&window)?; + let mut findings = Vec::new(); + for title in ["Add Index…", "Add Foreign Key…"] { + let row = descendants(window.upcast_ref()) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .find(|row| row.title() == title) + .ok_or_else(|| format!("the structure tab has no {title} row"))?; + row.emit_by_name::<()>("activated", &[]); + let mut shown = None; + wait_until(title, || { + shown = window.visible_dialog(); + shown.is_some() + })?; + let dialog = shown.ok_or("no dialog")?; + drain_main_context(); + merge(&mut findings, audit(&dialog)); + dialog.close(); + wait_until("the form to close", || window.visible_dialog().is_none())?; + } Ok(findings) } +/// A window connected to `one_table`, on its workspace. +fn connected(name: &str) -> Result<(World, adw::ApplicationWindow), String> { + let world = World::start( + name, + fake_driver(), + FakeConnector::new([]).always(FakeEstablish::Connect(one_table())), + )?; + let window = world.open(WindowLaunch::Connect(Box::new(world.saved.clone())))?; + wait_until("the table in the sidebar", || { + shows(&window, "TableProWorkspace") && shows_text(window.upcast_ref(), TABLE) + })?; + Ok((world, window)) +} + +fn sidebar_action(window: &adw::ApplicationWindow, action: &str) -> Result<(), String> { + let sidebar = descendants(window.upcast_ref()) + .into_iter() + .find(|widget| widget.is::()) + .ok_or("the sidebar has no list")?; + let table = (Some("public".to_owned()), TABLE.to_owned()).to_variant(); + sidebar + .activate_action(action, Some(&table)) + .map_err(|error| format!("{action}: {error}")) +} + +fn open_table(window: &adw::ApplicationWindow) -> Result<(), String> { + sidebar_action(window, "sidebar.open-in-new-tab")?; + wait_until("the table's rows", || shows_text(window.upcast_ref(), CELL)) +} + +fn open_structure(window: &adw::ApplicationWindow) -> Result<(), String> { + sidebar_action(window, "sidebar.edit-structure")?; + wait_until("the table's structure", || { + widgets_of(window.upcast_ref(), "TableProStructureTab") + .iter() + .flat_map(descendants) + .filter_map(|widget| widget.downcast::().ok()) + .any(|stack| stack.visible_child_name().as_deref() == Some("editor")) + }) +} + +/// The table tab's filter strip, open on its rule editor with one rule. +fn open_filter_strip(window: &adw::ApplicationWindow) -> Result<(), String> { + let tab = widgets_of(window.upcast_ref(), "TableProBrowseTab") + .pop() + .ok_or("no table tab")?; + tab.activate_action("browse.filter", None) + .map_err(|error| format!("browse.filter: {error}"))?; + let strip = widgets_of(&tab, "TableProFilterStrip").pop().ok_or("no filter strip")?; + strip + .activate_action("filter.add-rule", None) + .map_err(|error| format!("filter.add-rule: {error}"))?; + for expander in descendants(&strip) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + { + expander.set_expanded(true); + } + wait_until("the filter rule", || { + !widgets_of(&strip, "TableProFilterRuleRow").is_empty() + }) +} + +/// The editor of the cell that shows `CELL`, opened from its menu the +/// way the Menu key opens it. +fn open_cell_editor(window: &adw::ApplicationWindow) -> Result<(), String> { + let cell = widgets_of(window.upcast_ref(), "TableProGridCell") + .into_iter() + .find(|cell| shows_text(cell, CELL)) + .ok_or("no cell shows the row")?; + let controllers = cell.observe_controllers(); + let menu = (0..controllers.n_items()) + .filter_map(|index| controllers.item(index).and_downcast::()) + .find(|gesture| gesture.button() == 3) + .ok_or("the cell has no menu")?; + menu.emit_by_name::<()>("pressed", &[&1i32, &1.0f64, &1.0f64]); + for popover in descendants(window.upcast_ref()) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + { + popover.popdown(); + } + drain_main_context(); + cell.activate_action("cell.edit", None) + .map_err(|error| format!("cell.edit: {error}"))?; + wait_until("the cell's editor", || { + descendants(&cell) + .iter() + .any(|widget| widget.is::() && widget.is_visible()) + }) +} + /// The dialog for a new connection, on a server engine and then on a /// file engine, with every section expanded. fn connect_dialog() -> Result, String> { diff --git a/linux/data/resources/shortcuts-dialog.ui b/linux/data/resources/shortcuts-dialog.ui index 6960765c73..74cc9ad5ed 100644 --- a/linux/data/resources/shortcuts-dialog.ui +++ b/linux/data/resources/shortcuts-dialog.ui @@ -293,6 +293,12 @@ Escape + + + Move Between the Editor and the Results + F6 + + Format SQL diff --git a/linux/data/resources/style.css b/linux/data/resources/style.css index 40683461de..05e2c711b9 100644 --- a/linux/data/resources/style.css +++ b/linux/data/resources/style.css @@ -18,9 +18,11 @@ textview.sql-editor, textview.sql-editor text { background: color-mix(in srgb, var(--success-bg-color) 22%, transparent); } +/* The full error colour: at 70% the text fell to about 3.5:1 on the + tint, under the 4.5:1 body text needs. */ .tp-row-pending-delete { background: color-mix(in srgb, var(--error-bg-color) 16%, transparent); - color: color-mix(in srgb, var(--error-color) 70%, transparent); + color: var(--error-color); text-decoration: line-through; } diff --git a/linux/data/resources/ui/browse-tab.ui b/linux/data/resources/ui/browse-tab.ui index 016728b6ef..41c797fb85 100644 --- a/linux/data/resources/ui/browse-tab.ui +++ b/linux/data/resources/ui/browse-tab.ui @@ -156,7 +156,7 @@ other use. --> Filter Rows - browse.filter + browse.toggle-filter diff --git a/linux/data/resources/ui/filter-rule-row.ui b/linux/data/resources/ui/filter-rule-row.ui index f43cd41101..cc37b4c050 100644 --- a/linux/data/resources/ui/filter-rule-row.ui +++ b/linux/data/resources/ui/filter-rule-row.ui @@ -6,11 +6,17 @@ center + + Column + center + + Operator + diff --git a/linux/data/resources/ui/filter-strip.ui b/linux/data/resources/ui/filter-strip.ui index e4259586f5..b6c3140f7c 100644 --- a/linux/data/resources/ui/filter-strip.ui +++ b/linux/data/resources/ui/filter-strip.ui @@ -37,6 +37,7 @@ Match + combinator_dropdown @@ -124,6 +125,7 @@ WHERE + raw_entry @@ -69,6 +73,7 @@ Delete Delete + title_label