From d04633d308e200adc1e9ea9c9180499d679fd674 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 15:58:15 -0400 Subject: [PATCH 1/8] LR-LIB-RATING: one shared star rating component The bottom bar, the Info panel and the filter bar each drew their own row of stars, and the grid badge, filmstrip badges and compare caption had private painter loops. They now share rating::Rating (a clickable row: hover previews, clicking the current rating clears it, read-only and disabled states, left/right arrows on a focused star) and rating::paint_rating / paint_star for painter-only contexts. Existing automation ids (star:bottom:n, star:info:n, filter:starN) are unchanged. The preset, profile and theme favourite stars use paint_star too, so Icon::Star lives only in rating.rs. Co-Authored-By: Claude --- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/panels/bottombar.rs | 4 +- crates/ui-egui/src/panels/compare.rs | 2 +- crates/ui-egui/src/panels/detail.rs | 5 +- crates/ui-egui/src/panels/filterbar.rs | 18 +-- crates/ui-egui/src/panels/grid.rs | 5 +- crates/ui-egui/src/panels/presets.rs | 7 +- crates/ui-egui/src/panels/profiles.rs | 2 +- crates/ui-egui/src/panels/right.rs | 2 +- crates/ui-egui/src/panels/settings.rs | 7 +- crates/ui-egui/src/rating.rs | 213 +++++++++++++++++++++++++ crates/ui-egui/src/tests_rating.rs | 110 +++++++++++++ crates/ui-egui/src/widgets.rs | 45 ------ docs/parity-checklist.md | 2 +- docs/whats-new.md | 5 + 15 files changed, 348 insertions(+), 82 deletions(-) create mode 100644 crates/ui-egui/src/rating.rs create mode 100644 crates/ui-egui/src/tests_rating.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1b2517907..04bd65d74 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -31,6 +31,7 @@ mod model_setup; pub mod panels; pub mod pick; pub mod print_view; +pub mod rating; pub mod recheck; pub mod region; pub mod reload; @@ -149,6 +150,8 @@ mod tests_profiles_layout; #[cfg(test)] mod tests_quit_unsaved; #[cfg(test)] +mod tests_rating; +#[cfg(test)] mod tests_recheck; #[cfg(test)] mod tests_rename_person; diff --git a/crates/ui-egui/src/panels/bottombar.rs b/crates/ui-egui/src/panels/bottombar.rs index 69b76bcb8..415e23d06 100644 --- a/crates/ui-egui/src/panels/bottombar.rs +++ b/crates/ui-egui/src/panels/bottombar.rs @@ -8,7 +8,7 @@ use crate::LightcraftApp; use crate::icons::{Icon, paint}; use crate::state::{ViewMode, Zoom}; use crate::theme::Tokens; -use crate::widgets::{icon_button, register, stars}; +use crate::widgets::{icon_button, register}; const VIEW_MODES: &[(&str, Icon, ViewMode, &str)] = &[ ("photoGrid", Icon::GridPhoto, ViewMode::PhotoGrid, "Photo Grid (G)"), @@ -105,7 +105,7 @@ fn centre(app: &mut LightcraftApp, ui: &mut egui::Ui, full: Rect, from: f32, to: ui.new_child(egui::UiBuilder::new().max_rect(pill.shrink2(vec2(10.0, 4.0))).layout(egui::Layout::left_to_right(egui::Align::Center))); pill_ui.spacing_mut().item_spacing.x = 3.0; let rating = active.as_ref().map(|p| p.rating).unwrap_or(0); - if let Some(r) = stars(&mut pill_ui, "bottom", rating, 19.0) { + if let Some(r) = crate::rating::Rating::new("bottom", rating).show(&mut pill_ui) { let mut params = json!({"rating": r}); crate::panels::compare::target_active(app, &mut params); let _ = app.run("photo.rate", params); diff --git a/crates/ui-egui/src/panels/compare.rs b/crates/ui-egui/src/panels/compare.rs index bb7937640..8d0f6eaec 100644 --- a/crates/ui-egui/src/panels/compare.rs +++ b/crates/ui-egui/src/panels/compare.rs @@ -343,7 +343,7 @@ fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slo let filled = i < photo.rating; let r = Rect::from_min_size(pos2(x + i as f32 * 13.0, cap.center().y - 6.0), vec2(12.0, 12.0)); if r.right() < cap.right() { - paint(pt, r, if filled { Icon::StarFilled } else { Icon::Star }, if filled { t.star } else { t.icon.gamma_multiply(0.6) }); + crate::rating::paint_star(pt, r, filled, if filled { t.star } else { t.icon.gamma_multiply(0.6) }); } } x += 5.0 * 13.0 + 4.0; diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index b95d7b056..c61a81383 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -2939,10 +2939,7 @@ fn film_badges(p: &egui::Painter, t: &Tokens, fr: Rect, ph: &lightcraft_catalog: p.rect_filled(bar, 0.0, Color32::from_black_alpha(130)); let y = bar.center().y; let mut x = bar.left() + 3.0; - for i in 0..ph.rating { - paint(p, Rect::from_min_size(pos2(x + i as f32 * 9.0, y - 4.0), vec2(8.0, 8.0)), Icon::StarFilled, t.star); - } - x += ph.rating as f32 * 9.0 + 2.0; + x += crate::rating::paint_rating(p, pos2(x, y), ph.rating, crate::rating::RatingSize::Small, t.star) + 2.0; match ph.flag { Flag::Pick => paint(p, Rect::from_min_size(pos2(x, y - 5.0), vec2(10.0, 10.0)), Icon::FlagPickFilled, t.pick), Flag::Reject => paint(p, Rect::from_min_size(pos2(x, y - 5.0), vec2(10.0, 10.0)), Icon::FlagReject, t.reject), diff --git a/crates/ui-egui/src/panels/filterbar.rs b/crates/ui-egui/src/panels/filterbar.rs index 8365a1ff8..8b524df11 100644 --- a/crates/ui-egui/src/panels/filterbar.rs +++ b/crates/ui-egui/src/panels/filterbar.rs @@ -130,22 +130,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { .map(|(l, k, op)| (l.to_string(), json!({"ratingOp": k}), f.rating_op == op)) .collect(), ); - for i in 1..=5u8 { - let (r, resp) = ui.allocate_exact_size(vec2(17.0, 26.0), Sense::click()); - crate::widgets::announce(&resp, format!("{i} ★")); - register(ui.ctx(), format!("filter:star{i}"), r); - let filled = i <= f.rating; - let c = if filled { - t.star - } else if resp.hovered() { - t.text_label - } else { - t.icon - }; - paint(ui.painter(), Rect::from_center_size(r.center(), vec2(14.0, 14.0)), if filled { Icon::StarFilled } else { Icon::Star }, c); - if resp.clicked() { - filter(app, json!({"rating": if f.rating == i { 0 } else { i }})); - } + if let Some(r) = crate::rating::Rating::new("filter", f.rating).automation_ids("filter:star").show(ui) { + filter(app, json!({"rating": r})); } ui.add_space(14.0); diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index a1184892c..2d5b05e76 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -857,10 +857,7 @@ fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square p.rect_filled(bar, 0.0, Color32::from_black_alpha(120)); } let mut x = bar.left() + 6.0; - for i in 0..photo.rating { - paint(p, Rect::from_min_size(pos2(x + i as f32 * 13.0, bar.center().y - 6.0), vec2(12.0, 12.0)), Icon::StarFilled, t.star); - } - x += photo.rating as f32 * 13.0 + 4.0; + x += crate::rating::paint_rating(p, pos2(x, bar.center().y), photo.rating, crate::rating::RatingSize::Medium, t.star) + 4.0; match photo.flag { Flag::Pick => paint(p, Rect::from_min_size(pos2(x, bar.center().y - 7.0), vec2(14.0, 14.0)), Icon::FlagPickFilled, t.pick), Flag::Reject => paint(p, Rect::from_min_size(pos2(x, bar.center().y - 7.0), vec2(14.0, 14.0)), Icon::FlagReject, t.reject), diff --git a/crates/ui-egui/src/panels/presets.rs b/crates/ui-egui/src/panels/presets.rs index a8cf21424..439a15a1a 100644 --- a/crates/ui-egui/src/panels/presets.rs +++ b/crates/ui-egui/src/panels/presets.rs @@ -227,7 +227,12 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } ui.painter().text(pos2(text_x, r.center().y), Align2::LEFT_CENTER, &name, t.font(13.0), t.text_label); if fav { - paint(ui.painter(), Rect::from_center_size(pos2(r.right() - 20.0, r.center().y), vec2(16.0, 16.0)), Icon::StarFilled, t.star); + crate::rating::paint_star( + ui.painter(), + Rect::from_center_size(pos2(r.right() - 20.0, r.center().y), vec2(16.0, 16.0)), + true, + t.star, + ); } // resting on a preset previews it in the loupe (no history entry) if resp.hovered() diff --git a/crates/ui-egui/src/panels/profiles.rs b/crates/ui-egui/src/panels/profiles.rs index 8a4b08345..000ebdbcb 100644 --- a/crates/ui-egui/src/panels/profiles.rs +++ b/crates/ui-egui/src/panels/profiles.rs @@ -182,7 +182,7 @@ fn cell_ui( let over_star = resp.hover_pos().is_some_and(|q| star.contains(q)); if fav || resp.hovered() { painter.circle_filled(star.center(), 10.0, Color32::from_black_alpha(110)); - paint(painter, star.shrink(4.0), if fav { Icon::StarFilled } else { Icon::Star }, if fav || over_star { t.star } else { Color32::WHITE }); + crate::rating::paint_star(painter, star.shrink(4.0), fav, if fav || over_star { t.star } else { Color32::WHITE }); } if resp.hovered() && !applied && !over_star { app.hover_preview = diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 76ecc746a..26bb0ec3c 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -642,7 +642,7 @@ fn info(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { ui.label(egui::RichText::new(crate::i18n::tr_format!("Virtual copy “{name}” of {of}", name = name, of = of)).color(t.text_label)); } ui.add_space(8.0); - if let Some(r) = ui.horizontal(|ui| crate::widgets::stars(ui, "info", p.rating, 20.0)).inner { + if let Some(r) = ui.horizontal(|ui| crate::rating::Rating::new("info", p.rating).show(ui)).inner { let mut params = json!({"rating": r}); crate::panels::compare::target_active(app, &mut params); let _ = app.run("photo.rate", params); diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 2f844f4ae..f6cb3bc12 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -953,12 +953,7 @@ fn theme_card( }); register(ui.ctx(), format!("button:themeFavorite-{}", kind.id()), r.rect); if favorite || hovered { - crate::icons::paint( - ui.painter(), - r.rect.shrink(4.0), - if favorite { crate::icons::Icon::StarFilled } else { crate::icons::Icon::Star }, - if favorite { t.accent } else { t.text_dim }, - ); + crate::rating::paint_star(ui.painter(), r.rect.shrink(4.0), favorite, if favorite { t.accent } else { t.text_dim }); } if r.clicked() { if favorite { diff --git a/crates/ui-egui/src/rating.rs b/crates/ui-egui/src/rating.rs new file mode 100644 index 000000000..50fa00fda --- /dev/null +++ b/crates/ui-egui/src/rating.rs @@ -0,0 +1,213 @@ +//! One star rating for the whole interface: the clickable row of five stars ([`Rating`]) and the +//! painter-only helpers for stars drawn onto photos and captions ([`paint_rating`], [`paint_star`]). +//! It replaces `widgets::stars` and the private star-painting loops of the grid badge, the film +//! badges, the compare caption and the filter bar. +//! +//! ```ignore +//! if let Some(r) = Rating::new("bottom", photo.rating).size(RatingSize::Large).show(ui) { … } +//! Rating::new("row", 3).size(RatingSize::Small).read_only().show(ui); +//! ``` +//! +//! Hovering a star previews the rating a click would set; clicking the current rating clears it. +//! With a star focused, ← and → lower and raise the rating (0 to 5). + +use egui::{Color32, Key, Modifiers, Painter, Pos2, Rect, Sense, Ui, pos2, vec2}; + +use crate::icons::{Icon, paint}; +use crate::theme::Tokens; +use crate::widgets::{announce_selected, register}; + +/// Number of stars in a rating. +const STARS: u8 = 5; + +/// How big a rating is drawn. Each size matches one of the renderers it replaced. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum RatingSize { + /// Film badges: 8 px stars every 9 px. + Small, + /// Grid badges and compare captions: 12 px stars every 13 px. + Medium, + /// The bottom bar, the Info panel and the filter bar: 14 px stars in 19 px cells. + Large, +} + +impl RatingSize { + /// Side of one painted star. + pub fn icon(self) -> f32 { + match self { + Self::Small => 8.0, + Self::Medium => 12.0, + Self::Large => 14.0, + } + } + + /// Distance from one painted star to the next in [`paint_rating`]. + pub fn step(self) -> f32 { + match self { + Self::Small => 9.0, + Self::Medium => 13.0, + Self::Large => 17.0, + } + } + + /// Side of one clickable cell in a [`Rating`] row. + fn cell(self) -> f32 { + match self { + Self::Small => 12.0, + Self::Medium => 16.0, + Self::Large => 19.0, + } + } +} + +/// Paint one star (outline or filled) centred in `rect`. +pub fn paint_star(painter: &Painter, rect: Rect, filled: bool, color: Color32) { + paint(painter, rect, if filled { Icon::StarFilled } else { Icon::Star }, color); +} + +/// Paint `rating` filled stars in a row, the first one's left edge at `pos.x` and its vertical centre +/// at `pos.y`. Returns the width the row takes (0 for no stars), so the caller can lay out what follows. +pub fn paint_rating(painter: &Painter, pos: Pos2, rating: u8, size: RatingSize, color: Color32) -> f32 { + let n = rating.min(STARS); + let side = size.icon(); + for i in 0..n { + let left = pos.x + f32::from(i) * size.step(); + paint_star(painter, Rect::from_min_size(pos2(left, pos.y - side / 2.0), vec2(side, side)), true, color); + } + f32::from(n) * size.step() +} + +/// What the stars show once hovering is taken into account: the hovered star's rating previews over +/// the current one. +fn star_lit(i: u8, rating: u8, hover: Option) -> bool { + i < hover.unwrap_or(rating) +} + +/// The rating a click on star `n` (1 to 5) sets: clicking the current rating clears it. +fn clicked_rating(current: u8, n: u8) -> u8 { + if current == n { 0 } else { n } +} + +/// The rating the arrow keys move to from `current`, if any. +fn nudged_rating(current: u8, left: bool, right: bool) -> Option { + match (left, right) { + (true, false) if current > 0 => Some(current - 1), + (false, true) if current < STARS => Some(current + 1), + _ => None, + } +} + +/// A row of five stars. Build it, then [`Rating::show`]. +pub struct Rating { + id: String, + rating: u8, + size: RatingSize, + read_only: bool, + enabled: bool, + ids_prefix: Option, +} + +impl Rating { + /// A rating of `rating` (0 to 5) whose stars are reachable as `star:{id}:{n}`. + pub fn new(id: impl Into, rating: u8) -> Self { + Self { id: id.into(), rating: rating.min(STARS), size: RatingSize::Large, read_only: false, enabled: true, ids_prefix: None } + } + + /// The size the stars are drawn at (default [`RatingSize::Large`]). + pub fn size(mut self, size: RatingSize) -> Self { + self.size = size; + self + } + + /// Show the rating without reacting to hover or clicks. + pub fn read_only(mut self) -> Self { + self.read_only = true; + self + } + + /// Whether the stars react (default true). A disabled row is faded and ignores clicks. + pub fn enabled(mut self, enabled: bool) -> Self { + self.enabled = enabled; + self + } + + /// Register the stars under `{prefix}{n}` instead of `star:{id}:{n}`, for rows that already have + /// published automation ids (the filter bar's `filter:star3`). + pub fn automation_ids(mut self, prefix: impl Into) -> Self { + self.ids_prefix = Some(prefix.into()); + self + } + + fn widget_id(&self, n: u8) -> String { + match &self.ids_prefix { + Some(prefix) => format!("{prefix}{n}"), + None => format!("star:{}:{n}", self.id), + } + } + + /// Draw the row. Returns the new rating when a click or an arrow key changed it. + pub fn show(self, ui: &mut Ui) -> Option { + let t = Tokens::get(ui.ctx()); + let interactive = self.enabled && !self.read_only; + let cell = self.size.cell(); + let mut out = None; + let mut hover_n = None; + let mut focused = false; + let mut cells = [Rect::NOTHING; STARS as usize]; + for (i, slot) in (0..STARS).zip(cells.iter_mut()) { + let (r, resp) = ui.allocate_exact_size(vec2(cell, cell), if interactive { Sense::click() } else { Sense::hover() }); + announce_selected(&resp, format!("{} ★", i + 1), i < self.rating); + register(ui.ctx(), self.widget_id(i + 1), r); + if interactive { + if resp.hovered() { + hover_n = Some(i + 1); + } + if resp.clicked() { + out = Some(clicked_rating(self.rating, i + 1)); + } + focused |= resp.has_focus(); + } + *slot = r; + } + if focused && out.is_none() { + let (left, right) = ui.input_mut(|i| (i.consume_key(Modifiers::NONE, Key::ArrowLeft), i.consume_key(Modifiers::NONE, Key::ArrowRight))); + out = nudged_rating(self.rating, left, right); + } + // The hovered star also gets the icon-button hover square, so hovering the current rating (where + // the preview looks the same) still shows feedback. + let fade = if self.enabled { 1.0 } else { 0.4 }; + let side = self.size.icon(); + for (i, r) in (0..STARS).zip(cells) { + if hover_n == Some(i + 1) { + ui.painter().rect_filled(r, 4.0, t.hover.gamma_multiply(0.7)); + } + let filled = star_lit(i, self.rating, hover_n); + let color = if filled { t.star } else { t.icon }; + paint_star(ui.painter(), Rect::from_center_size(r.center(), vec2(side, side)), filled, color.gamma_multiply(fade)); + } + out + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_hovered_star_previews_its_rating() { + let lit = |rating, hover| (0..STARS).filter(|&i| star_lit(i, rating, hover)).count(); + assert_eq!(lit(2, None), 2, "no hover shows the rating"); + assert_eq!(lit(2, Some(4)), 4); + assert_eq!(lit(5, Some(1)), 1); + assert_eq!(lit(0, Some(3)), 3); + } + + #[test] + fn arrows_stay_between_zero_and_five() { + assert_eq!(nudged_rating(0, true, false), None); + assert_eq!(nudged_rating(5, false, true), None); + assert_eq!(nudged_rating(3, true, false), Some(2)); + assert_eq!(nudged_rating(3, false, true), Some(4)); + assert_eq!(nudged_rating(3, true, true), None); + } +} diff --git a/crates/ui-egui/src/tests_rating.rs b/crates/ui-egui/src/tests_rating.rs new file mode 100644 index 000000000..a7a8e65f9 --- /dev/null +++ b/crates/ui-egui/src/tests_rating.rs @@ -0,0 +1,110 @@ +//! The star rating: clicking sets the rating, clicking the current one clears it, a read-only or +//! disabled row ignores clicks, the arrow keys step it, and screen readers hear every star. + +use egui::{Pos2, Rect, vec2}; + +use crate::rating::{Rating, RatingSize}; + +/// Draw `build` for a few frames. After the first, click the centre of star `click` (1 to 5) if given, +/// then press `keys`. Returns every value `build` reported and the AccessKit +/// (role, name) pairs. +fn drive(click: Option, keys: &[egui::Key], build: impl Fn(&mut egui::Ui) -> Option) -> (Vec, Vec<(String, String)>) { + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + ctx.enable_accesskit(); + let mut reported = Vec::new(); + let mut nodes = Vec::new(); + let mut rects: Vec<(String, Rect)> = Vec::new(); + for frame in 0..6 { + let mut events = Vec::new(); + if frame == 1 + && let Some(n) = click + { + let want = format!("star:t:{n}"); + let at = rects.iter().find(|(id, _)| *id == want).map(|(_, r)| r.center()).unwrap_or(Pos2::ZERO); + events.push(egui::Event::PointerMoved(at)); + events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + } + // one key per frame from the third on, so Tab has moved the focus before an arrow arrives + if frame >= 2 + && let Some(k) = keys.get(frame - 2) + { + events.push(egui::Event::Key { key: *k, physical_key: Some(*k), pressed: true, repeat: false, modifiers: Default::default() }); + } + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(400.0, 200.0))), events, ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| { + if let Some(v) = build(ui) { + reported.push(v); + } + }); + out.textures_delta.clear(); + let seen = crate::widgets::take_registry(&ctx); + if rects.is_empty() { + rects = seen; + } + if let Some(update) = out.platform_output.accesskit_update.take() { + nodes = update.nodes.iter().map(|(_, n)| (format!("{:?}", n.role()), n.label().unwrap_or_default().to_string())).collect(); + } + } + (reported, nodes) +} + +fn row(rating: u8) -> Rating { + Rating::new("t", rating).size(RatingSize::Large) +} + +#[test] +fn clicking_a_star_sets_that_rating() { + let (got, _) = drive(Some(4), &[], |ui| row(2).show(ui)); + assert_eq!(got, vec![4]); +} + +#[test] +fn clicking_the_current_rating_clears_it() { + let (got, _) = drive(Some(3), &[], |ui| row(3).show(ui)); + assert_eq!(got, vec![0]); +} + +#[test] +fn a_read_only_row_ignores_clicks() { + let (got, _) = drive(Some(4), &[], |ui| row(2).read_only().show(ui)); + assert!(got.is_empty(), "{got:?}"); +} + +#[test] +fn a_disabled_row_ignores_clicks() { + let (got, _) = drive(Some(4), &[], |ui| row(2).enabled(false).show(ui)); + assert!(got.is_empty(), "{got:?}"); +} + +#[test] +fn hovering_alone_changes_nothing() { + // the preview is only painted; the rating is reported on click + let (got, _) = drive(None, &[], |ui| row(2).show(ui)); + assert!(got.is_empty()); +} + +#[test] +fn a_screen_reader_hears_every_star_with_its_state() { + let (_, nodes) = drive(None, &[], |ui| row(2).show(ui)); + for n in 1..=5 { + let name = format!("{n} ★"); + assert!(nodes.iter().any(|(role, label)| role == "Button" && *label == name), "{name}: {nodes:?}"); + } +} + +#[test] +fn the_arrow_keys_step_the_rating_of_a_focused_star() { + // Tab focuses the first star; the row shows 3, so → asks for 4 and ← for 2 + let (got, _) = drive(None, &[egui::Key::Tab, egui::Key::ArrowRight], |ui| row(3).show(ui)); + assert_eq!(got, vec![4], "{got:?}"); + let (got, _) = drive(None, &[egui::Key::Tab, egui::Key::ArrowLeft], |ui| row(3).show(ui)); + assert_eq!(got, vec![2], "{got:?}"); +} + +#[test] +fn arrow_keys_do_nothing_when_no_star_is_focused() { + let (got, _) = drive(None, &[egui::Key::ArrowRight], |ui| row(3).show(ui)); + assert!(got.is_empty(), "{got:?}"); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 2db3f885e..01d5b65f8 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -776,41 +776,6 @@ pub fn icon_button(ui: &mut Ui, id: &str, icon: Icon, size: egui::Vec2, active: if !tooltip.is_empty() { resp.on_hover_text(tooltip) } else { resp } } -/// Star rating row (5 stars); returns a new rating when clicked (clicking the current rating clears). -pub fn stars(ui: &mut Ui, id: &str, rating: u8, size: f32) -> Option { - let t = Tokens::get(ui.ctx()); - let mut out = None; - let mut hover_n = None; - let mut rects = [Rect::NOTHING; 5]; - for (i, slot) in (0..5u8).zip(rects.iter_mut()) { - let (r, resp) = ui.allocate_exact_size(vec2(size, size), Sense::click()); - crate::widgets::announce_selected(&resp, format!("{} ★", i + 1), i < rating); - register(ui.ctx(), format!("star:{id}:{}", i + 1), r); - if resp.hovered() { - hover_n = Some(i + 1); - } - if resp.clicked() { - out = Some(if rating == i + 1 { 0 } else { i + 1 }); - } - *slot = r; - } - // Hover previews the rating a click would set: stars 1..=hovered light up. The hovered star also gets the - // icon_button hover square, so hovering the current rating (where the preview looks the same) still shows feedback. - for (i, r) in (0..5u8).zip(rects) { - if hover_n == Some(i + 1) { - ui.painter().rect_filled(r, 4.0, t.hover.gamma_multiply(0.7)); - } - let filled = star_lit(i, rating, hover_n); - paint(ui.painter(), r.shrink(size * 0.12), if filled { Icon::StarFilled } else { Icon::Star }, if filled { t.star } else { t.icon }); - } - out -} - -/// Whether the star at zero-based index `i` is drawn filled: the hovered star's rating previews over the current one. -fn star_lit(i: u8, rating: u8, hover: Option) -> bool { - i < hover.unwrap_or(rating) -} - /// A borderless dropdown button: `text` followed by a painted chevron (no font glyph needed). pub fn dropdown(ui: &mut Ui, id: &str, text: &str, font: egui::FontId, color: Color32) -> Response { dropdown_within(ui, id, text, font, color, f32::INFINITY) @@ -849,16 +814,6 @@ mod tests { assert!(preview_only_explanation("unsupported CR3").contains("unsupported CR3")); } - /// Issue #647: hovering a star previews the rating a click would set. - #[test] - fn hovered_star_previews_its_rating() { - let lit = |rating, hover| (0..5u8).filter(|&i| star_lit(i, rating, hover)).count(); - assert_eq!(lit(2, None), 2, "no hover shows the rating"); - assert_eq!(lit(2, Some(4)), 4); - assert_eq!(lit(5, Some(1)), 1); - assert_eq!(lit(0, Some(3)), 3); - } - #[test] fn nudges_are_a_sensible_step() { let spec = |id, min, max, step| ControlSpec { diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 99f17561e..55a35c85e 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -100,7 +100,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-SHARED-ALBUM | Shared albums | P2 | ⬜ | | needs a sharing service | | LR-LIB-OFFLINE | Keep album offline | P2 | 🚫 | | not applicable: local-first library | | LR-LIB-TARGET | Target album | P2 | ✅ | `cmd:album.setTarget`, `cmd:album.toggleTarget` | see LRC-LIB-COLLECTIONS | -| LR-LIB-RATING | Star ratings | P0 | ✅ | `cmd:photo.rate` (`advance`), `cmd:photo.decreaseRating`, `cmd:photo.increaseRating`, `crates/ui-egui/src/shortcuts.rs` | 0 clears; 1–5 set; Shift+0–5 applies and advances once, including with Auto Advance. Default grid brackets adjust each photo by one, clamped to 0–5; one batch undo and AutoWrite. Batch grid and undo/redo regressions; macOS native ownership corrected (PR #261) | +| LR-LIB-RATING | Star ratings | P0 | ✅ | `cmd:photo.rate` (`advance`), `cmd:photo.decreaseRating`, `cmd:photo.increaseRating`, `crates/ui-egui/src/shortcuts.rs`, `crates/ui-egui/src/rating.rs` | one shared star row everywhere (bottom bar, Info panel, filter bar: hover previews, ←/→ on a focused star) and one painter for badges; 0 clears; 1–5 set; Shift+0–5 applies and advances once, including with Auto Advance. Default grid brackets adjust each photo by one, clamped to 0–5; one batch undo and AutoWrite. Batch grid and undo/redo regressions; macOS native ownership corrected (PR #261) | | LR-LIB-FLAG | Pick / reject flags | P0 | ✅ | `cmd:photo.pick`, `cmd:photo.reject`, `cmd:photo.unflag`, `cmd:photo.flag` | P pick, X reject, U clear; Shift+P picks and advances in both grids (Presets elsewhere); Shift+X/U and legacy Shift+Z advance; no flag cycle; rejection does not delete the file; a pick shows as a filled flag on thumbnails, in Compare, in the loupe and on the Pick button, a reject outlined (#1013) | | LR-LIB-LABEL | Colour labels | P1 | ✅ | `cmd:photo.label`, `cmd:label.setNames`, `cmd:label.names`, `cmd:dialog.labelNames`, keys 6–9 in `crates/ui-egui/src/shortcuts.rs` | Photo menu and grid context menu (coloured, named), Info-panel swatches, label dot plus translucent colour on square-grid/filmstrip surrounds and photo-grid footers (photo pixels unchanged); colour-label toasts have pale matching backgrounds; setting/clearing through keys (including Shift), menus or Info swatches shows the same bottom toast as ratings, with the colour/custom name; editable label names (undoable, journaled); no purple key (as in Lightroom), no custom extra labels | | LR-LIB-KEYWORD | Keywords | P0 | ✅ | `cmd:panel.keywords`, `cmd:photo.setMeta` (`addKeywords`/`removeKeywords`), `cmd:keyword.list`, `cmd:keyword.rename`, `cmd:keyword.delete`, `cmd:keyword.merge`, `cmd:keyword.suggest`, `crates/catalog/src/keywords.rs`, `crates/ui-egui/src/panels/left.rs` | left-panel keyword tree with counts (click filters, children included; context menu rename / merge / delete / add / remove); library-wide ops are one undo step and replay from the op log; suggestions (co-occurring / most used / completions) in the Keywords panel; keywords reorganize by drag and drop in the Keyword List (LRC-LIB-KEYWORDS) | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..e284816e2 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,11 @@ ## 2026-10-21 +### One star rating everywhere +- The stars in the bottom bar, the Info panel and the filter bar are now one shared control, and the badges on grid + cells, filmstrip thumbnails and the Compare caption use the same drawing. Hovering a star in the filter bar now + previews the rating like the others do, and with a star focused (Tab), ← and → lower and raise the rating. + ### Sharper resampling in HDR and panorama merges - Merged photos (HDR and panorama) are resampled with a sharper filter (Lanczos-3 instead of bilinear). Moving a photo by half a pixel used to lose a third of its finest detail; now it keeps most of it, without halos around From 0e04000f60d11735c766d54d5d3f46e5242fbee3 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 16:44:14 -0400 Subject: [PATCH 2/8] LR-LIB-RATING: keep arrow keys on a focused star, draw its ring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Left/right moved egui focus to the neighbouring widget after one or two steps. A focused star now locks horizontal arrows (as the slider does) and draws the accent focus ring. Rating drops the options no caller uses (size, read_only, enabled). The mask-effect favourite stars use paint_star and a shared favorite_button instead of a raw egui::Button and a "★" glyph. Co-Authored-By: Claude --- crates/ui-egui/src/panels/mask_effects.rs | 12 ++-- crates/ui-egui/src/rating.rs | 70 +++++++++-------------- crates/ui-egui/src/tests_rating.rs | 54 ++++++++++++----- 3 files changed, 74 insertions(+), 62 deletions(-) diff --git a/crates/ui-egui/src/panels/mask_effects.rs b/crates/ui-egui/src/panels/mask_effects.rs index 6062d241c..ef17bfba0 100644 --- a/crates/ui-egui/src/panels/mask_effects.rs +++ b/crates/ui-egui/src/panels/mask_effects.rs @@ -237,7 +237,12 @@ fn menu(app: &mut LightcraftApp, ui: &mut egui::Ui, mask: &Mask, current: Option let elided = g.elided; ui.painter().galley(rect.left_center() + vec2(18.0, -g.size().y / 2.0), g, t.text); if fav { - ui.painter().text(rect.right_center() - vec2(4.0, 0.0), egui::Align2::RIGHT_CENTER, "★", t.font(11.0), t.text_dim); + crate::rating::paint_star( + ui.painter(), + egui::Rect::from_center_size(rect.right_center() - vec2(10.0, 0.0), vec2(11.0, 11.0)), + true, + t.text_dim, + ); } if moved && st.highlight == Some(i) { resp.scroll_to_me(None); @@ -522,10 +527,7 @@ pub fn manage_body( for e in list { let sel = selected.contains(&e.name); ui.horizontal(|ui| { - let star = ui.add(egui::Button::new(if e.favorite { "★" } else { "☆" }).frame(false)).on_hover_text(crate::i18n::tr("Favorite")); - if ui.is_rect_visible(star.rect) { - register(ui.ctx(), format!("effectStar:{}", e.name), star.rect); - } + let star = crate::rating::favorite_button(ui, format!("effectStar:{}", e.name), e.favorite, crate::i18n::tr("Favorite")); if star.clicked() { toggle_star = Some((e.name.clone(), !e.favorite)); } diff --git a/crates/ui-egui/src/rating.rs b/crates/ui-egui/src/rating.rs index 50fa00fda..f8b3c3a19 100644 --- a/crates/ui-egui/src/rating.rs +++ b/crates/ui-egui/src/rating.rs @@ -13,6 +13,7 @@ use egui::{Color32, Key, Modifiers, Painter, Pos2, Rect, Sense, Ui, pos2, vec2}; +use crate::button::Button; use crate::icons::{Icon, paint}; use crate::theme::Tokens; use crate::widgets::{announce_selected, register}; @@ -27,7 +28,7 @@ pub enum RatingSize { Small, /// Grid badges and compare captions: 12 px stars every 13 px. Medium, - /// The bottom bar, the Info panel and the filter bar: 14 px stars in 19 px cells. + /// The clickable row (bottom bar, Info panel, filter bar): 14 px stars in 19 px cells. Large, } @@ -52,11 +53,7 @@ impl RatingSize { /// Side of one clickable cell in a [`Rating`] row. fn cell(self) -> f32 { - match self { - Self::Small => 12.0, - Self::Medium => 16.0, - Self::Large => 19.0, - } + self.step() + 2.0 } } @@ -101,34 +98,13 @@ fn nudged_rating(current: u8, left: bool, right: bool) -> Option { pub struct Rating { id: String, rating: u8, - size: RatingSize, - read_only: bool, - enabled: bool, ids_prefix: Option, } impl Rating { /// A rating of `rating` (0 to 5) whose stars are reachable as `star:{id}:{n}`. pub fn new(id: impl Into, rating: u8) -> Self { - Self { id: id.into(), rating: rating.min(STARS), size: RatingSize::Large, read_only: false, enabled: true, ids_prefix: None } - } - - /// The size the stars are drawn at (default [`RatingSize::Large`]). - pub fn size(mut self, size: RatingSize) -> Self { - self.size = size; - self - } - - /// Show the rating without reacting to hover or clicks. - pub fn read_only(mut self) -> Self { - self.read_only = true; - self - } - - /// Whether the stars react (default true). A disabled row is faded and ignores clicks. - pub fn enabled(mut self, enabled: bool) -> Self { - self.enabled = enabled; - self + Self { id: id.into(), rating: rating.min(STARS), ids_prefix: None } } /// Register the stars under `{prefix}{n}` instead of `star:{id}:{n}`, for rows that already have @@ -148,24 +124,28 @@ impl Rating { /// Draw the row. Returns the new rating when a click or an arrow key changed it. pub fn show(self, ui: &mut Ui) -> Option { let t = Tokens::get(ui.ctx()); - let interactive = self.enabled && !self.read_only; - let cell = self.size.cell(); + let cell = RatingSize::Large.cell(); let mut out = None; let mut hover_n = None; let mut focused = false; let mut cells = [Rect::NOTHING; STARS as usize]; for (i, slot) in (0..STARS).zip(cells.iter_mut()) { - let (r, resp) = ui.allocate_exact_size(vec2(cell, cell), if interactive { Sense::click() } else { Sense::hover() }); + let (r, resp) = ui.allocate_exact_size(vec2(cell, cell), Sense::click()); announce_selected(&resp, format!("{} ★", i + 1), i < self.rating); register(ui.ctx(), self.widget_id(i + 1), r); - if interactive { - if resp.hovered() { - hover_n = Some(i + 1); - } - if resp.clicked() { - out = Some(clicked_rating(self.rating, i + 1)); - } - focused |= resp.has_focus(); + if resp.hovered() { + hover_n = Some(i + 1); + } + if resp.clicked() { + out = Some(clicked_rating(self.rating, i + 1)); + } + if resp.has_focus() { + focused = true; + // ← and → step the rating instead of moving the focus to the neighbouring widget + ui.memory_mut(|m| { + m.set_focus_lock_filter(resp.id, egui::EventFilter { horizontal_arrows: true, ..Default::default() }); + }); + ui.painter().rect_stroke(r.expand(1.0), 6.0, egui::Stroke::new(2.0, t.accent), egui::StrokeKind::Outside); } *slot = r; } @@ -175,20 +155,24 @@ impl Rating { } // The hovered star also gets the icon-button hover square, so hovering the current rating (where // the preview looks the same) still shows feedback. - let fade = if self.enabled { 1.0 } else { 0.4 }; - let side = self.size.icon(); + let side = RatingSize::Large.icon(); for (i, r) in (0..STARS).zip(cells) { if hover_n == Some(i + 1) { ui.painter().rect_filled(r, 4.0, t.hover.gamma_multiply(0.7)); } let filled = star_lit(i, self.rating, hover_n); - let color = if filled { t.star } else { t.icon }; - paint_star(ui.painter(), Rect::from_center_size(r.center(), vec2(side, side)), filled, color.gamma_multiply(fade)); + paint_star(ui.painter(), Rect::from_center_size(r.center(), vec2(side, side)), filled, if filled { t.star } else { t.icon }); } out } } +/// A frameless star toggle for favourites: filled when `favorite`, named `name` for screen readers, +/// reachable as `widget` (a full automation id). +pub fn favorite_button(ui: &mut Ui, widget: String, favorite: bool, name: &str) -> egui::Response { + Button::icon_only(if favorite { Icon::StarFilled } else { Icon::Star }, name).ghost().small().widget(widget).show(ui) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/ui-egui/src/tests_rating.rs b/crates/ui-egui/src/tests_rating.rs index a7a8e65f9..15534853b 100644 --- a/crates/ui-egui/src/tests_rating.rs +++ b/crates/ui-egui/src/tests_rating.rs @@ -3,7 +3,7 @@ use egui::{Pos2, Rect, vec2}; -use crate::rating::{Rating, RatingSize}; +use crate::rating::Rating; /// Draw `build` for a few frames. After the first, click the centre of star `click` (1 to 5) if given, /// then press `keys`. Returns every value `build` reported and the AccessKit @@ -51,7 +51,7 @@ fn drive(click: Option, keys: &[egui::Key], build: impl Fn(&mut egui::Ui) -> } fn row(rating: u8) -> Rating { - Rating::new("t", rating).size(RatingSize::Large) + Rating::new("t", rating) } #[test] @@ -66,18 +66,6 @@ fn clicking_the_current_rating_clears_it() { assert_eq!(got, vec![0]); } -#[test] -fn a_read_only_row_ignores_clicks() { - let (got, _) = drive(Some(4), &[], |ui| row(2).read_only().show(ui)); - assert!(got.is_empty(), "{got:?}"); -} - -#[test] -fn a_disabled_row_ignores_clicks() { - let (got, _) = drive(Some(4), &[], |ui| row(2).enabled(false).show(ui)); - assert!(got.is_empty(), "{got:?}"); -} - #[test] fn hovering_alone_changes_nothing() { // the preview is only painted; the rating is reported on click @@ -108,3 +96,41 @@ fn arrow_keys_do_nothing_when_no_star_is_focused() { let (got, _) = drive(None, &[egui::Key::ArrowRight], |ui| row(3).show(ui)); assert!(got.is_empty(), "{got:?}"); } + +/// Feed the shown rating back each frame, with other focusable widgets on both sides. +fn step_with_neighbours(start: u8, keys: &[egui::Key]) -> Vec { + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let rating = std::cell::Cell::new(start); + let mut seen = Vec::new(); + // a key every other frame, as a person presses them: the focus lock is set during the frame after a focus change + for frame in 0..keys.len() * 2 + 3 { + let mut events = Vec::new(); + if let Some(k) = frame.checked_sub(1).filter(|f| f % 2 == 0).and_then(|f| keys.get(f / 2)) { + events.push(egui::Event::Key { key: *k, physical_key: Some(*k), pressed: true, repeat: false, modifiers: Default::default() }); + } + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(600.0, 200.0))), events, ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| { + ui.horizontal(|ui| { + let _ = ui.button("left neighbour"); + if let Some(r) = row(rating.get()).show(ui) { + rating.set(r); + } + let _ = ui.button("right neighbour"); + }); + }); + out.textures_delta.clear(); + seen.push(rating.get()); + } + seen +} + +#[test] +fn arrows_keep_stepping_between_other_focusable_widgets() { + use egui::Key::*; + // Tab, Tab: the left neighbour, then star 1 + let seen = step_with_neighbours(5, &[Tab, Tab, ArrowLeft, ArrowLeft, ArrowLeft, ArrowLeft, ArrowLeft]); + assert_eq!(seen.last(), Some(&0), "{seen:?}"); + let seen = step_with_neighbours(0, &[Tab, Tab, ArrowRight, ArrowRight, ArrowRight, ArrowRight, ArrowRight, ArrowLeft]); + assert_eq!(seen.last(), Some(&4), "{seen:?}"); +} From e161744d707e3a1c0ac66c8cff4eb858bdfec1d3 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 16 Oct 2026 16:56:27 -0400 Subject: [PATCH 3/8] LR-LIB-RATING: first arrow after Tab steps the rating egui picks the neighbour to focus from the arrow key in begin_pass, before a widget can set a focus lock for the frame it just gained focus (set_focus_lock_filter needs a frame of focus first). A focused star that consumes an arrow now cancels that pending focus move. The favourite toggle is one interact-height line square again, so Manage Effect Presets rows keep their height. Co-Authored-By: Claude --- crates/ui-egui/src/rating.rs | 25 +++++++++++++++++++++---- crates/ui-egui/src/tests_rating.rs | 6 +++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/rating.rs b/crates/ui-egui/src/rating.rs index f8b3c3a19..ee81fc360 100644 --- a/crates/ui-egui/src/rating.rs +++ b/crates/ui-egui/src/rating.rs @@ -13,7 +13,6 @@ use egui::{Color32, Key, Modifiers, Painter, Pos2, Rect, Sense, Ui, pos2, vec2}; -use crate::button::Button; use crate::icons::{Icon, paint}; use crate::theme::Tokens; use crate::widgets::{announce_selected, register}; @@ -151,6 +150,11 @@ impl Rating { } if focused && out.is_none() { let (left, right) = ui.input_mut(|i| (i.consume_key(Modifiers::NONE, Key::ArrowLeft), i.consume_key(Modifiers::NONE, Key::ArrowRight))); + if left || right { + // egui picked the neighbour to focus from this same key before the lock above could apply (the + // first arrow after Tab): cancel that move + ui.memory_mut(|m| m.move_focus(egui::FocusDirection::None)); + } out = nudged_rating(self.rating, left, right); } // The hovered star also gets the icon-button hover square, so hovering the current rating (where @@ -167,10 +171,23 @@ impl Rating { } } -/// A frameless star toggle for favourites: filled when `favorite`, named `name` for screen readers, -/// reachable as `widget` (a full automation id). +/// A frameless star toggle for favourites: filled when `favorite`, named `name` for screen readers and as +/// its tooltip, reachable as `widget` (a full automation id). A square one line tall. pub fn favorite_button(ui: &mut Ui, widget: String, favorite: bool, name: &str) -> egui::Response { - Button::icon_only(if favorite { Icon::StarFilled } else { Icon::Star }, name).ghost().small().widget(widget).show(ui) + let t = Tokens::get(ui.ctx()); + let side = ui.spacing().interact_size.y; + let (r, resp) = ui.allocate_exact_size(vec2(side, side), Sense::click()); + announce_selected(&resp, name, favorite); + register(ui.ctx(), widget, r); + if resp.hovered() { + ui.painter().rect_filled(r, 4.0, t.hover.gamma_multiply(0.7)); + } + if resp.has_focus() { + ui.painter().rect_stroke(r.expand(1.0), 6.0, egui::Stroke::new(2.0, t.accent), egui::StrokeKind::Outside); + } + let icon = RatingSize::Medium.icon(); + paint_star(ui.painter(), Rect::from_center_size(r.center(), vec2(icon, icon)), favorite, if favorite { t.star } else { t.icon }); + resp.on_hover_text(name) } #[cfg(test)] diff --git a/crates/ui-egui/src/tests_rating.rs b/crates/ui-egui/src/tests_rating.rs index 15534853b..e3ebf27ab 100644 --- a/crates/ui-egui/src/tests_rating.rs +++ b/crates/ui-egui/src/tests_rating.rs @@ -103,10 +103,10 @@ fn step_with_neighbours(start: u8, keys: &[egui::Key]) -> Vec { crate::theme::install_fonts(&ctx); let rating = std::cell::Cell::new(start); let mut seen = Vec::new(); - // a key every other frame, as a person presses them: the focus lock is set during the frame after a focus change - for frame in 0..keys.len() * 2 + 3 { + // a key every frame, Tab then an arrow straight away + for frame in 0..keys.len() + 3 { let mut events = Vec::new(); - if let Some(k) = frame.checked_sub(1).filter(|f| f % 2 == 0).and_then(|f| keys.get(f / 2)) { + if let Some(k) = frame.checked_sub(1).and_then(|f| keys.get(f)) { events.push(egui::Event::Key { key: *k, physical_key: Some(*k), pressed: true, repeat: false, modifiers: Default::default() }); } let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(600.0, 200.0))), events, ..Default::default() }; From 8720bbcfeff7b9860335d893d7fcd871f95dd210 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Tue, 20 Oct 2026 07:27:14 -0400 Subject: [PATCH 4/8] LR-LIB-RATING: one Tab stop per rating row, visible-only favourite registration, true docs The star row is a single Tab stop (the current rating's star, or the first) with the arrow keys stepping inside it and the focus following. favorite_button registers its automation id only while on screen again. The module and test docs no longer mention a size/read-only API that doesn't exist. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/rating.rs | 46 ++++++++++++++++++++++++++---- crates/ui-egui/src/tests_rating.rs | 33 +++++++++++++++++++-- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/rating.rs b/crates/ui-egui/src/rating.rs index ee81fc360..febcb3790 100644 --- a/crates/ui-egui/src/rating.rs +++ b/crates/ui-egui/src/rating.rs @@ -4,12 +4,13 @@ //! badges, the compare caption and the filter bar. //! //! ```ignore -//! if let Some(r) = Rating::new("bottom", photo.rating).size(RatingSize::Large).show(ui) { … } -//! Rating::new("row", 3).size(RatingSize::Small).read_only().show(ui); +//! if let Some(r) = Rating::new("bottom", photo.rating).show(ui) { … } +//! paint_rating(ui.painter(), pos, 3, RatingSize::Small, color); //! ``` //! //! Hovering a star previews the rating a click would set; clicking the current rating clears it. -//! With a star focused, ← and → lower and raise the rating (0 to 5). +//! The row is one Tab stop (the star of the current rating, or the first when unrated); inside it, ← and → +//! lower and raise the rating (0 to 5) and the focus follows. use egui::{Color32, Key, Modifiers, Painter, Pos2, Rect, Sense, Ui, pos2, vec2}; @@ -120,6 +121,10 @@ impl Rating { } } + fn star_id(&self, ui: &Ui, i: u8) -> egui::Id { + ui.id().with(("rating-star", &self.id, i)) + } + /// Draw the row. Returns the new rating when a click or an arrow key changed it. pub fn show(self, ui: &mut Ui) -> Option { let t = Tokens::get(ui.ctx()); @@ -128,8 +133,14 @@ impl Rating { let mut hover_n = None; let mut focused = false; let mut cells = [Rect::NOTHING; STARS as usize]; + // One Tab stop for the row, like a radio group: the star of the current rating (the first when + // unrated), plus whichever star already has the focus so it keeps it + let stop = self.rating.max(1) - 1; for (i, slot) in (0..STARS).zip(cells.iter_mut()) { - let (r, resp) = ui.allocate_exact_size(vec2(cell, cell), Sense::click()); + let id = self.star_id(ui, i); + let (r, _) = ui.allocate_exact_size(vec2(cell, cell), Sense::hover()); + let sense = if i == stop || ui.memory(|m| m.has_focus(id)) { Sense::click() } else { Sense::CLICK }; + let resp = ui.interact(r, id, sense); announce_selected(&resp, format!("{} ★", i + 1), i < self.rating); register(ui.ctx(), self.widget_id(i + 1), r); if resp.hovered() { @@ -156,6 +167,11 @@ impl Rating { ui.memory_mut(|m| m.move_focus(egui::FocusDirection::None)); } out = nudged_rating(self.rating, left, right); + if let Some(n) = out { + // keep the focus on the star that is now the row's Tab stop + let to = self.star_id(ui, n.max(1) - 1); + ui.memory_mut(|m| m.request_focus(to)); + } } // The hovered star also gets the icon-button hover square, so hovering the current rating (where // the preview looks the same) still shows feedback. @@ -178,7 +194,10 @@ pub fn favorite_button(ui: &mut Ui, widget: String, favorite: bool, name: &str) let side = ui.spacing().interact_size.y; let (r, resp) = ui.allocate_exact_size(vec2(side, side), Sense::click()); announce_selected(&resp, name, favorite); - register(ui.ctx(), widget, r); + // (rows scrolled out of view can't be clicked) + if ui.is_rect_visible(r) { + register(ui.ctx(), widget, r); + } if resp.hovered() { ui.painter().rect_filled(r, 4.0, t.hover.gamma_multiply(0.7)); } @@ -211,4 +230,21 @@ mod tests { assert_eq!(nudged_rating(3, false, true), Some(4)); assert_eq!(nudged_rating(3, true, true), None); } + + #[test] + fn rows_scrolled_out_of_view_are_not_registered() { + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let mut out = ctx.run_ui(egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(300.0, 100.0))), ..Default::default() }, |ui| { + egui::ScrollArea::vertical().max_height(60.0).show(ui, |ui| { + for i in 0..30 { + favorite_button(ui, format!("effectStar:{i}"), false, "Favorite"); + } + }); + }); + out.textures_delta.clear(); + let ids: Vec = crate::widgets::take_registry(&ctx).into_iter().map(|(id, _)| id).collect(); + assert!(ids.contains(&"effectStar:0".to_string()), "{ids:?}"); + assert!(!ids.contains(&"effectStar:29".to_string()), "{ids:?}"); + } } diff --git a/crates/ui-egui/src/tests_rating.rs b/crates/ui-egui/src/tests_rating.rs index e3ebf27ab..db6453e77 100644 --- a/crates/ui-egui/src/tests_rating.rs +++ b/crates/ui-egui/src/tests_rating.rs @@ -1,5 +1,5 @@ -//! The star rating: clicking sets the rating, clicking the current one clears it, a read-only or -//! disabled row ignores clicks, the arrow keys step it, and screen readers hear every star. +//! The star rating: clicking sets the rating, clicking the current one clears it, the row is one +//! Tab stop, the arrow keys step it, and screen readers hear every star. use egui::{Pos2, Rect, vec2}; @@ -134,3 +134,32 @@ fn arrows_keep_stepping_between_other_focusable_widgets() { let seen = step_with_neighbours(0, &[Tab, Tab, ArrowRight, ArrowRight, ArrowRight, ArrowRight, ArrowRight, ArrowLeft]); assert_eq!(seen.last(), Some(&4), "{seen:?}"); } + +#[test] +fn a_rating_row_is_one_tab_stop() { + use egui::Key::Tab; + // left neighbour, one stop for the five stars, right neighbour: the third Tab is already past the row + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let mut focused = Vec::new(); + for frame in 0..5 { + let events = if frame >= 1 { + vec![egui::Event::Key { key: Tab, physical_key: Some(Tab), pressed: true, repeat: false, modifiers: Default::default() }] + } else { + Vec::new() + }; + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(600.0, 200.0))), events, ..Default::default() }; + let mut right = None; + let mut out = ctx.run_ui(raw, |ui| { + ui.horizontal(|ui| { + let _ = ui.button("left neighbour"); + let _ = row(3).show(ui); + right = Some(ui.button("right neighbour").id); + }); + }); + out.textures_delta.clear(); + focused.push(right.is_some_and(|id| ctx.memory(|m| m.has_focus(id)))); + } + // frames 1 to 3 press Tab: left neighbour, the row, right neighbour + assert_eq!(focused, vec![false, false, false, true, false], "{focused:?}"); +} From 0885ba5fd234614d0233547e119762105c38e13d Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Tue, 20 Oct 2026 07:27:51 -0400 Subject: [PATCH 5/8] LR-LIB-RATING: format Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/rating.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/rating.rs b/crates/ui-egui/src/rating.rs index febcb3790..2325ed09a 100644 --- a/crates/ui-egui/src/rating.rs +++ b/crates/ui-egui/src/rating.rs @@ -235,13 +235,14 @@ mod tests { fn rows_scrolled_out_of_view_are_not_registered() { let ctx = egui::Context::default(); crate::theme::install_fonts(&ctx); - let mut out = ctx.run_ui(egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(300.0, 100.0))), ..Default::default() }, |ui| { - egui::ScrollArea::vertical().max_height(60.0).show(ui, |ui| { - for i in 0..30 { - favorite_button(ui, format!("effectStar:{i}"), false, "Favorite"); - } + let mut out = + ctx.run_ui(egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(300.0, 100.0))), ..Default::default() }, |ui| { + egui::ScrollArea::vertical().max_height(60.0).show(ui, |ui| { + for i in 0..30 { + favorite_button(ui, format!("effectStar:{i}"), false, "Favorite"); + } + }); }); - }); out.textures_delta.clear(); let ids: Vec = crate::widgets::take_registry(&ctx).into_iter().map(|(id, _)| id).collect(); assert!(ids.contains(&"effectStar:0".to_string()), "{ids:?}"); From 5947667c6ca72fedfc521a7f3f5970d881128dc0 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Tue, 20 Oct 2026 08:09:26 -0400 Subject: [PATCH 6/8] LR-LIB-RATING: focus follows the Tab stop when the rating changes elsewhere Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/rating.rs | 9 +++++++-- crates/ui-egui/src/tests_rating.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/rating.rs b/crates/ui-egui/src/rating.rs index 2325ed09a..16cd611d2 100644 --- a/crates/ui-egui/src/rating.rs +++ b/crates/ui-egui/src/rating.rs @@ -131,7 +131,7 @@ impl Rating { let cell = RatingSize::Large.cell(); let mut out = None; let mut hover_n = None; - let mut focused = false; + let mut focused_at = None; let mut cells = [Rect::NOTHING; STARS as usize]; // One Tab stop for the row, like a radio group: the star of the current rating (the first when // unrated), plus whichever star already has the focus so it keeps it @@ -150,7 +150,7 @@ impl Rating { out = Some(clicked_rating(self.rating, i + 1)); } if resp.has_focus() { - focused = true; + focused_at = Some(i); // ← and → step the rating instead of moving the focus to the neighbouring widget ui.memory_mut(|m| { m.set_focus_lock_filter(resp.id, egui::EventFilter { horizontal_arrows: true, ..Default::default() }); @@ -159,6 +159,7 @@ impl Rating { } *slot = r; } + let focused = focused_at.is_some(); if focused && out.is_none() { let (left, right) = ui.input_mut(|i| (i.consume_key(Modifiers::NONE, Key::ArrowLeft), i.consume_key(Modifiers::NONE, Key::ArrowRight))); if left || right { @@ -171,6 +172,10 @@ impl Rating { // keep the focus on the star that is now the row's Tab stop let to = self.star_id(ui, n.max(1) - 1); ui.memory_mut(|m| m.request_focus(to)); + } else if focused_at != Some(stop) { + // the rating changed from outside (undo, a shortcut, another photo): the focus follows the Tab stop + let to = self.star_id(ui, stop); + ui.memory_mut(|m| m.request_focus(to)); } } // The hovered star also gets the icon-button hover square, so hovering the current rating (where diff --git a/crates/ui-egui/src/tests_rating.rs b/crates/ui-egui/src/tests_rating.rs index db6453e77..fd1fff09b 100644 --- a/crates/ui-egui/src/tests_rating.rs +++ b/crates/ui-egui/src/tests_rating.rs @@ -163,3 +163,30 @@ fn a_rating_row_is_one_tab_stop() { // frames 1 to 3 press Tab: left neighbour, the row, right neighbour assert_eq!(focused, vec![false, false, false, true, false], "{focused:?}"); } + +#[test] +fn a_rating_changed_from_outside_moves_the_focus_to_the_tab_stop() { + use egui::Key::Tab; + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let rating = std::cell::Cell::new(2u8); + let mut stars_focused = Vec::new(); + for frame in 0..5 { + // frame 1: Tab focuses the row's stop (star 2); frame 2: the rating changes elsewhere to 5 + if frame == 2 { + rating.set(5); + } + let events = if frame == 1 { + vec![egui::Event::Key { key: Tab, physical_key: Some(Tab), pressed: true, repeat: false, modifiers: Default::default() }] + } else { + Vec::new() + }; + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(600.0, 200.0))), events, ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| { + let _ = row(rating.get()).show(ui); + stars_focused = (0..5).map(|i| ctx.memory(|m| m.has_focus(ui.id().with(("rating-star", "t", i as u8))))).collect(); + }); + out.textures_delta.clear(); + } + assert_eq!(stars_focused, vec![false, false, false, false, true], "{stars_focused:?}"); +} From 5d1f9da46c7addbd0a37b06ac472ab77e2bedced Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Tue, 20 Oct 2026 09:00:33 -0400 Subject: [PATCH 7/8] LR-LIB-RATING: say which surfaces share the star row The parity row and the What's New heading said one star rating 'everywhere', but the Set Rating / Select By Rating menu rows and the grid context menu are separate menu items with star text and are unchanged. Name the real scope. Co-Authored-By: Claude Opus 5.5 --- docs/parity-checklist.md | 2 +- docs/whats-new.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 55a35c85e..ead0265ca 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -100,7 +100,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-SHARED-ALBUM | Shared albums | P2 | ⬜ | | needs a sharing service | | LR-LIB-OFFLINE | Keep album offline | P2 | 🚫 | | not applicable: local-first library | | LR-LIB-TARGET | Target album | P2 | ✅ | `cmd:album.setTarget`, `cmd:album.toggleTarget` | see LRC-LIB-COLLECTIONS | -| LR-LIB-RATING | Star ratings | P0 | ✅ | `cmd:photo.rate` (`advance`), `cmd:photo.decreaseRating`, `cmd:photo.increaseRating`, `crates/ui-egui/src/shortcuts.rs`, `crates/ui-egui/src/rating.rs` | one shared star row everywhere (bottom bar, Info panel, filter bar: hover previews, ←/→ on a focused star) and one painter for badges; 0 clears; 1–5 set; Shift+0–5 applies and advances once, including with Auto Advance. Default grid brackets adjust each photo by one, clamped to 0–5; one batch undo and AutoWrite. Batch grid and undo/redo regressions; macOS native ownership corrected (PR #261) | +| LR-LIB-RATING | Star ratings | P0 | ✅ | `cmd:photo.rate` (`advance`), `cmd:photo.decreaseRating`, `cmd:photo.increaseRating`, `crates/ui-egui/src/shortcuts.rs`, `crates/ui-egui/src/rating.rs` | one shared star row in the bottom bar, Info panel and filter bar (hover previews, ←/→ on a focused star) and one painter for badges (grid cells, filmstrip, Compare caption); menu rows with star text (Photo ▸ Set Rating, Select By Rating, the grid context menu) are separate menu items, unchanged; 0 clears; 1–5 set; Shift+0–5 applies and advances once, including with Auto Advance. Default grid brackets adjust each photo by one, clamped to 0–5; one batch undo and AutoWrite. Batch grid and undo/redo regressions; macOS native ownership corrected (PR #261) | | LR-LIB-FLAG | Pick / reject flags | P0 | ✅ | `cmd:photo.pick`, `cmd:photo.reject`, `cmd:photo.unflag`, `cmd:photo.flag` | P pick, X reject, U clear; Shift+P picks and advances in both grids (Presets elsewhere); Shift+X/U and legacy Shift+Z advance; no flag cycle; rejection does not delete the file; a pick shows as a filled flag on thumbnails, in Compare, in the loupe and on the Pick button, a reject outlined (#1013) | | LR-LIB-LABEL | Colour labels | P1 | ✅ | `cmd:photo.label`, `cmd:label.setNames`, `cmd:label.names`, `cmd:dialog.labelNames`, keys 6–9 in `crates/ui-egui/src/shortcuts.rs` | Photo menu and grid context menu (coloured, named), Info-panel swatches, label dot plus translucent colour on square-grid/filmstrip surrounds and photo-grid footers (photo pixels unchanged); colour-label toasts have pale matching backgrounds; setting/clearing through keys (including Shift), menus or Info swatches shows the same bottom toast as ratings, with the colour/custom name; editable label names (undoable, journaled); no purple key (as in Lightroom), no custom extra labels | | LR-LIB-KEYWORD | Keywords | P0 | ✅ | `cmd:panel.keywords`, `cmd:photo.setMeta` (`addKeywords`/`removeKeywords`), `cmd:keyword.list`, `cmd:keyword.rename`, `cmd:keyword.delete`, `cmd:keyword.merge`, `cmd:keyword.suggest`, `crates/catalog/src/keywords.rs`, `crates/ui-egui/src/panels/left.rs` | left-panel keyword tree with counts (click filters, children included; context menu rename / merge / delete / add / remove); library-wide ops are one undo step and replay from the op log; suggestions (co-occurring / most used / completions) in the Keywords panel; keywords reorganize by drag and drop in the Keyword List (LRC-LIB-KEYWORDS) | diff --git a/docs/whats-new.md b/docs/whats-new.md index e284816e2..6f61958f5 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,7 +2,7 @@ ## 2026-10-21 -### One star rating everywhere +### One star rating control - The stars in the bottom bar, the Info panel and the filter bar are now one shared control, and the badges on grid cells, filmstrip thumbnails and the Compare caption use the same drawing. Hovering a star in the filter bar now previews the rating like the others do, and with a star focused (Tab), ← and → lower and raise the rating. From f76680dc3b70a62c6bfe9388749c786cab33d408 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 07:15:53 -0400 Subject: [PATCH 8/8] LR-LIB-RATING: a test paints the hover state through Rating::show --- crates/ui-egui/src/tests_rating.rs | 46 ++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/crates/ui-egui/src/tests_rating.rs b/crates/ui-egui/src/tests_rating.rs index fd1fff09b..f89d1267b 100644 --- a/crates/ui-egui/src/tests_rating.rs +++ b/crates/ui-egui/src/tests_rating.rs @@ -73,6 +73,52 @@ fn hovering_alone_changes_nothing() { assert!(got.is_empty()); } +/// How many stars `Rating::show` paints filled (in the star colour) after the pointer has moved to +/// the centre of star `hover` (1 to 5), if given. A filled star is ten triangles of one colour. +fn filled_stars(rating: u8, hover: Option) -> usize { + fn count(shape: &egui::Shape, colour: egui::Color32) -> usize { + match shape { + egui::Shape::Path(path) => usize::from(path.fill == colour), + egui::Shape::Vec(shapes) => shapes.iter().map(|shape| count(shape, colour)).sum(), + _ => 0, + } + } + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let colour = crate::theme::Tokens::get(&ctx).star; + let mut at = None; + let mut filled = 0; + for frame in 0..4 { + let mut events = Vec::new(); + if frame >= 1 + && let Some(at) = at + { + events.push(egui::Event::PointerMoved(at)); + } + let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(400.0, 200.0))), events, ..Default::default() }; + let mut out = ctx.run_ui(raw, |ui| { + let _ = row(rating).show(ui); + }); + out.textures_delta.clear(); + let seen = crate::widgets::take_registry(&ctx); + if let (Some(n), true) = (hover, frame == 0) { + let want = format!("star:t:{n}"); + at = seen.iter().find(|(id, _)| *id == want).map(|(_, r)| r.center()); + assert!(at.is_some(), "star {n} is registered"); + } + filled = out.shapes.iter().map(|clipped| count(&clipped.shape, colour)).sum::() / 10; + } + filled +} + +#[test] +fn hovering_a_star_paints_the_rating_a_click_would_set() { + // the row shows 2 filled stars; the pointer over star 4 shows 4, and over star 1 shows 1 + assert_eq!(filled_stars(2, None), 2); + assert_eq!(filled_stars(2, Some(4)), 4); + assert_eq!(filled_stars(2, Some(1)), 1, "hovering a lower star previews the lower rating"); +} + #[test] fn a_screen_reader_hears_every_star_with_its_state() { let (_, nodes) = drive(None, &[], |ui| row(2).show(ui));