diff --git a/linux/changelog.d/3932.added.md b/linux/changelog.d/3932.added.md new file mode 100644 index 0000000000..26a8f9f656 --- /dev/null +++ b/linux/changelog.d/3932.added.md @@ -0,0 +1 @@ +Every other row of a data grid is shaded, with a switch in Preferences to turn it off. diff --git a/linux/crates/app/src/ui/data_grid/columns.rs b/linux/crates/app/src/ui/data_grid/columns.rs index a634886d12..08c244d5d9 100644 --- a/linux/crates/app/src/ui/data_grid/columns.rs +++ b/linux/crates/app/src/ui/data_grid/columns.rs @@ -20,7 +20,7 @@ use crate::ui::handler_guard::HandlerGuard; /// Rows sampled for a column's opening width. const WIDTH_SAMPLE: usize = 200; -/// Cell padding Adwaita gives a data table's cell, both sides, and the +/// The padding `style.css` gives a cell's child, both sides, and the /// column separator. const CELL_CHROME_PX: i32 = 13; /// A resize step, in digits of the font the cells draw in, so a step is @@ -204,6 +204,7 @@ impl DataGrid { let grid_cell = GridCell::new(setup_paint.clone()); grid_cell.attach(cell); cell.set_child(Some(&grid_cell)); + super::stripes::follow(cell); }); factory.connect_bind(|_, item| { let Some(cell) = item.downcast_ref::() else { @@ -754,8 +755,20 @@ impl DataGrid { // GTK makes the header and rows only as wide as the columns unless one // expands, and never resizes the last column; this one takes both. +// Its empty child carries the stripe out to the view's edge. fn filler_column() -> gtk::ColumnViewColumn { - let column = gtk::ColumnViewColumn::builder().expand(true).build(); + let factory = gtk::SignalListItemFactory::new(); + factory.connect_setup(|_, item| { + let Some(cell) = item.downcast_ref::() else { + return; + }; + let blank = adw::Bin::builder() + .accessible_role(gtk::AccessibleRole::Presentation) + .build(); + cell.set_child(Some(&blank)); + super::stripes::follow(cell); + }); + let column = gtk::ColumnViewColumn::builder().expand(true).factory(&factory).build(); column.set_id(Some("filler")); column } diff --git a/linux/crates/app/src/ui/data_grid/grid_cell.rs b/linux/crates/app/src/ui/data_grid/grid_cell.rs index 8e16a2e32d..e338960d75 100644 --- a/linux/crates/app/src/ui/data_grid/grid_cell.rs +++ b/linux/crates/app/src/ui/data_grid/grid_cell.rs @@ -344,7 +344,7 @@ fn shown_text<'a>( } } -fn set_class(widget: &impl IsA, class: &str, on: bool) { +pub(super) fn set_class(widget: &impl IsA, class: &str, on: bool) { match on { true => widget.as_ref().add_css_class(class), false => widget.as_ref().remove_css_class(class), diff --git a/linux/crates/app/src/ui/data_grid/imp.rs b/linux/crates/app/src/ui/data_grid/imp.rs index d3c16c7de9..1b043907fd 100644 --- a/linux/crates/app/src/ui/data_grid/imp.rs +++ b/linux/crates/app/src/ui/data_grid/imp.rs @@ -87,6 +87,7 @@ pub struct DataGrid { /// The window's focus handler, while the grid is in a window. pub focus_watch: RefCell, glib::SignalHandlerId)>>, pub(super) font_watch: RefCell>, + pub(super) stripes_watch: RefCell>, } #[glib::object_subclass] diff --git a/linux/crates/app/src/ui/data_grid/mod.rs b/linux/crates/app/src/ui/data_grid/mod.rs index 4f445f4c57..b6e375c251 100644 --- a/linux/crates/app/src/ui/data_grid/mod.rs +++ b/linux/crates/app/src/ui/data_grid/mod.rs @@ -26,6 +26,7 @@ mod row_number_column; mod selection; mod selection_summary; mod server_sorter; +mod stripes; mod table_keys; #[cfg(test)] pub(crate) mod test_grid; @@ -88,6 +89,7 @@ impl DataGrid { imp.detail_pane.attach(&grid); grid.set_columns(columns, sorting, rules); grid.watch_fonts(); + grid.watch_stripes(); grid } diff --git a/linux/crates/app/src/ui/data_grid/row_number_column.rs b/linux/crates/app/src/ui/data_grid/row_number_column.rs index 3e31735c1d..19114d5711 100644 --- a/linux/crates/app/src/ui/data_grid/row_number_column.rs +++ b/linux/crates/app/src/ui/data_grid/row_number_column.rs @@ -37,6 +37,7 @@ pub fn build(grid: &DataGrid) -> gtk::ColumnViewColumn { content.append(&icon); content.append(&number); cell.set_child(Some(&content)); + super::stripes::follow(cell); }); let watches: Watches = Rc::default(); let weak = grid.downgrade(); diff --git a/linux/crates/app/src/ui/data_grid/stripes.rs b/linux/crates/app/src/ui/data_grid/stripes.rs new file mode 100644 index 0000000000..40f82a20dd --- /dev/null +++ b/linux/crates/app/src/ui/data_grid/stripes.rs @@ -0,0 +1,46 @@ +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use gtk4::{self as gtk, glib}; +use tablepro_storage::settings::keys; + +use super::DataGrid; +use super::grid_cell::set_class; +use crate::ui::handler_guard::HandlerGuard; + +const STRIPE: &str = "tp-stripe"; +const STRIPED: &str = "tp-striped"; + +// Rows are recycled, so `:nth-child` cannot stripe them and GTK has no API +// to class a row: each cell's child follows its row's position instead. +pub(super) fn follow(cell: >k::ColumnViewCell) { + cell.connect_notify_local(Some("position"), |cell, _| { + if let Some(child) = cell.child() { + let position = cell.position(); + let odd = position != gtk::INVALID_LIST_POSITION && !position.is_multiple_of(2); + set_class(&child, STRIPE, odd); + } + }); +} + +impl DataGrid { + pub(super) fn watch_stripes(&self) { + let Some(settings) = self.config().map(|config| config.settings.clone()) else { + return; + }; + let view = self.column_view(); + // Connected before the read: GSettings emits `changed` only for a key + // read while a handler was connected. + let handler = settings.gio().connect_changed( + Some(keys::SHOW_ALTERNATE_ROWS), + glib::clone!( + #[weak] + view, + move |settings, key| set_class(&view, STRIPED, settings.boolean(key)) + ), + ); + self.imp() + .stripes_watch + .replace(Some(HandlerGuard::new(settings.gio(), handler))); + set_class(&view, STRIPED, settings.show_alternate_rows()); + } +} diff --git a/linux/crates/app/src/ui/data_grid/tests/cells.rs b/linux/crates/app/src/ui/data_grid/tests/cells.rs index 84a64b0d9a..5cd4894e4e 100644 --- a/linux/crates/app/src/ui/data_grid/tests/cells.rs +++ b/linux/crates/app/src/ui/data_grid/tests/cells.rs @@ -133,6 +133,77 @@ fn a_row_with_a_boolean_is_as_tall_as_one_without() { assert_eq!(height(&flags), height(&texts)); } +// Every cell of the row, the gutter and the filler included. +fn stripes_across(cell: &GridCell) -> Vec { + let row = row_widget(cell); + std::iter::successors(row.first_child(), |column| column.next_sibling()) + .map(|column| { + column + .first_child() + .is_some_and(|child| child.has_css_class("tp-stripe")) + }) + .collect() +} + +fn striped_rows(ids: &[i64]) -> Grid { + Grid::build(GridSpec::table(two_text_columns()), keyed(ids)) +} + +#[gtk4::test] +fn every_other_row_is_striped_out_to_the_filler() { + let grid = striped_rows(&[1, 2, 3, 4]); + + assert!(grid.grid.column_view().has_css_class("tp-striped"), "on by default"); + for position in 0..4 { + let row = stripes_across(&grid.cell_at(position, 1)); + assert_eq!(row.len(), 4, "gutter, two columns and the filler"); + assert!( + row.iter().all(|striped| *striped == (position % 2 == 1)), + "row {position}: {row:?}" + ); + } +} + +#[gtk4::test] +fn a_row_added_above_moves_the_stripes() { + let grid = two_text_rows(); + + grid.insert_draft(); + + assert!(stripes_across(&grid.cell_of(1, 1)).iter().all(|striped| *striped)); + assert!(stripes_across(&grid.cell_of(2, 1)).iter().all(|striped| !striped)); +} + +#[gtk4::test] +fn recycled_rows_take_the_stripe_of_their_new_position() { + let ids: Vec = (1..=300).collect(); + let grid = striped_rows(&ids); + + grid.focus(250, 1); + + let drawn = grid.cells(); + assert!(drawn.iter().any(|cell| cell.position() == Some(250))); + for cell in drawn { + let position = cell.position().expect("a bound cell"); + assert_eq!(cell.has_css_class("tp-stripe"), position % 2 == 1, "row {position}"); + } +} + +#[gtk4::test] +fn open_grids_follow_the_stripes_preference() { + let grid = two_text_rows(); + let settings = grid.grid.config().expect("a configured grid").settings.clone(); + let striped = || grid.grid.column_view().has_css_class("tp-striped"); + + settings.set_show_alternate_rows(false).expect("store the flag"); + drain_main_context(); + assert!(!striped()); + + settings.set_show_alternate_rows(true).expect("store the flag"); + drain_main_context(); + assert!(striped()); +} + #[gtk4::test] fn column_view_carries_data_table_class() { let grid = two_text_rows(); diff --git a/linux/crates/app/src/ui/preferences_dialog/data_browser_group.rs b/linux/crates/app/src/ui/preferences_dialog/data_browser_group.rs index d072f3431d..932463082e 100644 --- a/linux/crates/app/src/ui/preferences_dialog/data_browser_group.rs +++ b/linux/crates/app/src/ui/preferences_dialog/data_browser_group.rs @@ -17,6 +17,8 @@ mod imp { #[template_child] pub page_size_row: TemplateChild, #[template_child] + pub alternate_rows_row: TemplateChild, + #[template_child] pub confirm_row: TemplateChild, } @@ -72,6 +74,9 @@ impl DataBrowserGroup { gio_settings .bind(keys::DEFAULT_PAGE_SIZE, &imp.page_size_row.get(), "value") .build(); + gio_settings + .bind(keys::SHOW_ALTERNATE_ROWS, &imp.alternate_rows_row.get(), "active") + .build(); gio_settings .bind(keys::CONFIRM_BULK_DELETE, &imp.confirm_row.get(), "active") .build(); @@ -97,18 +102,25 @@ mod tests { let settings = MemorySettings::new(); settings.get().set_default_page_size(500).expect("store the page size"); settings.get().set_confirm_bulk_delete(false).expect("store the flag"); + settings + .get() + .set_show_alternate_rows(false) + .expect("store the stripes"); let group = bound_group(&settings); let imp = group.imp(); assert_eq!(imp.page_size_row.value(), 500.0); assert!(!imp.confirm_row.is_active()); + assert!(!imp.alternate_rows_row.is_active()); imp.page_size_row.set_value(2_500.0); imp.confirm_row.set_active(true); + imp.alternate_rows_row.set_active(true); assert_eq!(settings.get().default_page_size(), 2_500); assert!(settings.get().confirm_bulk_delete()); + assert!(settings.get().show_alternate_rows()); } #[gtk4::test] diff --git a/linux/crates/storage/src/settings/keys.rs b/linux/crates/storage/src/settings/keys.rs index 9dea2b0ba7..c2289b1106 100644 --- a/linux/crates/storage/src/settings/keys.rs +++ b/linux/crates/storage/src/settings/keys.rs @@ -1,5 +1,6 @@ pub const DEFAULT_PAGE_SIZE: &str = "default-page-size"; pub const CONFIRM_BULK_DELETE: &str = "confirm-bulk-delete"; +pub const SHOW_ALTERNATE_ROWS: &str = "show-alternate-rows"; pub const SAVE_QUERY_HISTORY: &str = "save-query-history"; pub const HISTORY_RETENTION_DAYS: &str = "history-retention-days"; pub const QUERY_TIMEOUT_SECS: &str = "query-timeout-secs"; diff --git a/linux/crates/storage/src/settings/mod.rs b/linux/crates/storage/src/settings/mod.rs index ab020b8b7f..77eafb0f3f 100644 --- a/linux/crates/storage/src/settings/mod.rs +++ b/linux/crates/storage/src/settings/mod.rs @@ -80,6 +80,14 @@ impl AppSettings { self.set_boolean(keys::CONFIRM_BULK_DELETE, confirm) } + pub fn show_alternate_rows(&self) -> bool { + self.settings.boolean(keys::SHOW_ALTERNATE_ROWS) + } + + pub fn set_show_alternate_rows(&self, show: bool) -> Result<(), SettingsError> { + self.set_boolean(keys::SHOW_ALTERNATE_ROWS, show) + } + /// Whether runs go into the query history. Off records nothing and /// leaves the entries already there. pub fn save_query_history(&self) -> bool { diff --git a/linux/crates/storage/tests/settings.rs b/linux/crates/storage/tests/settings.rs index 1051e02c71..bc0a7c2eef 100644 --- a/linux/crates/storage/tests/settings.rs +++ b/linux/crates/storage/tests/settings.rs @@ -45,6 +45,7 @@ fn defaults_match_schema() -> TestResult { assert_eq!(fixture.settings.default_page_size(), 1_000); assert!(fixture.settings.confirm_bulk_delete()); + assert!(fixture.settings.show_alternate_rows()); assert_eq!(fixture.settings.history_retention_days(), 30); assert_eq!(fixture.settings.query_timeout(), Some(Duration::from_secs(60))); assert_eq!(fixture.settings.style_scheme(), "Adwaita"); @@ -152,6 +153,17 @@ fn confirm_bulk_delete_round_trip() -> TestResult { Ok(()) } +#[test] +fn show_alternate_rows_round_trip() -> TestResult { + let fixture = Fixture::new()?; + + fixture.settings.set_show_alternate_rows(false)?; + assert!(!fixture.settings.show_alternate_rows()); + fixture.settings.set_show_alternate_rows(true)?; + assert!(fixture.settings.show_alternate_rows()); + Ok(()) +} + #[test] fn save_query_history_round_trip() -> TestResult { let fixture = Fixture::new()?; diff --git a/linux/data/app.tablepro.TablePro.gschema.xml.in b/linux/data/app.tablepro.TablePro.gschema.xml.in index e1d082c737..b2459fbb0d 100644 --- a/linux/data/app.tablepro.TablePro.gschema.xml.in +++ b/linux/data/app.tablepro.TablePro.gschema.xml.in @@ -65,6 +65,12 @@ Ask before deleting 10 or more rows at once in a table tab. Deleted rows stay marked until Save, and Undo brings them back. + + true + Show alternate row backgrounds + Shade every other row of a data grid so a row reads across its columns. + + true Save query history diff --git a/linux/data/resources/style.css b/linux/data/resources/style.css index f9f58bcffe..5a1c9b816b 100644 --- a/linux/data/resources/style.css +++ b/linux/data/resources/style.css @@ -22,6 +22,28 @@ columnview > listview > row > cell > tp-grid-cell > text:focus-within { } } +/* The cell's child carries the padding, as in libadwaita's file chooser, + so a stripe on the child spans the cell. */ +columnview.data-table > listview > row > cell { + padding: 0; +} + +columnview.data-table > listview > row > cell > * { + padding: 2px 6px; +} + +/* A selected row keeps its accent: GTK draws it on the row, beneath the + cells. */ +columnview.tp-striped > listview > row:not(:selected) > cell > .tp-stripe { + background-color: color-mix(in srgb, var(--view-fg-color) 4%, transparent); +} + +@media (prefers-contrast: more) { + columnview.tp-striped > listview > row:not(:selected) > cell > .tp-stripe { + background-color: color-mix(in srgb, var(--view-fg-color) 8%, transparent); + } +} + /* Connection colour tags. The palette is libadwaita's accent set, kept as literal values rather than the accent variables: a tag names a specific colour, so it must not follow whatever accent the user has diff --git a/linux/data/resources/ui/preferences-data-browser-group.ui b/linux/data/resources/ui/preferences-data-browser-group.ui index e4decd5571..330633f43a 100644 --- a/linux/data/resources/ui/preferences-data-browser-group.ui +++ b/linux/data/resources/ui/preferences-data-browser-group.ui @@ -18,6 +18,11 @@ + + + Show Alternate Row Backgrounds + + Confirm Before Deleting Rows diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index a58c0d8161..4720abdb1c 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -276,6 +276,7 @@ crates/app/src/ui/data_grid/row_number_column.rs crates/app/src/ui/data_grid/selection.rs crates/app/src/ui/data_grid/selection_summary.rs crates/app/src/ui/data_grid/server_sorter.rs +crates/app/src/ui/data_grid/stripes.rs crates/app/src/ui/data_grid/table_keys.rs crates/app/src/ui/data_grid/test_grid.rs crates/app/src/ui/data_grid/tests/cells.rs