Compare commits

...
Author SHA1 Message Date
enricobuehler 96959c984f fix(console): the collections setting was wired to nothing, the grid lost its last row, and the sort bar was put down wherever there was room
Three more from the Deck.

"Even if I have the collections enabled in the settings, they don't actually get
shown when opening the library."

`LibraryScreen::collections_upgrade` was written, documented, and unit-tested for
its DECISION — and then nothing ever called it. It carried an
`#[allow(dead_code)]`, which is exactly what stopped the compiler from saying so,
and its own tests passed throughout because they called it directly. The setting
was on, the shelf agreed it should stand aside, and the library opened on the
shelf anyway.

The call belongs in the shell's per-frame sync and nowhere else: a screen cannot
replace ITSELF. The shelf can answer the question, because it holds the library
and the settings; only the shell owns the stack. It is guarded on a settled
transition, because mid-flight the stack's top is not yet what the user is looking
at, and swapping under a push they have already reversed with B would land them on
the collections of a host they just backed out of. The new test drives
`Shell::sync` and asserts on the STACK — the shelf was never the broken part, so a
test that asked the shelf would have gone on passing.

"The grid view is cut off at the bottom."

The mirror of the top inset fixed one commit ago, and that fix is what made it
visible. The content ended at the last row's card bottom, so at maximum scroll
that card sat exactly flush with the viewport's clip and lost its focus scale, its
shadow and its label. The air has to be part of `content_h` rather than of the
viewport, because `content_h` is the only thing the scroll clamp knows about.

"I don't like the top bar for switching the sorting & view type, please redesign
it — one of the worst parts is the not centered view switch, the ugly looking
focus indicator (dark bg, border)."

The arrangement group used to START at the band's midpoint. That is neither
centred nor trailing: it read as a control that had been put down wherever there
was room. Both groups are now anchored to the edge they belong to — the sort leads
at the heading's inset, the arrangement trails at the controller chip's, and the
shoulders that change it are the trailing pair of buttons, so the hand and the eye
agree. The band is the same two-anchor structure as the row above it.

The focus indicator was a glass panel, a brand hairline and a halo — the console's
recipe for a floating SURFACE, applied to a strip that sits flat in the field.
Hence the dark slab with a line round it, worse on the six pale palettes where the
glass turns to frost over an already-light field. It is an accent wash now and
nothing else: no border, no halo, no glass. The accent is palette-derived, so 14 %
reads at both poles without the strip ever becoming an object.

One defect introduced and caught in the same pass, by looking rather than by
testing: the gap between a caption and its pills was never explicit. It came from
`TabStrip`'s leading inset, which only appears when the rect it is handed has
slack — and a trailing group's rect is exactly as wide as its pills, so the gap
silently vanished and "VIEW" ended up touching the first pill, while the leading
group kept a gap by accident. Both groups now space their caption themselves and
both are handed exactly-sized rects, so neither depends on that side effect for
its position.
2026-08-16 23:55:51 +02:00
4 changed files with 224 additions and 50 deletions
+83 -42
View File
@@ -36,11 +36,8 @@ const BAR_CORNER: f64 = 14.0;
/// notice; the GRID starts its first row at the very top of what it is given, and a rank of
/// covers flush against a band of chrome reads as one clipped object rather than two.
const BAR_GAP: f64 = 12.0;
/// Where the bar's arrangement column starts, in design units, when the band is too narrow
/// to split down the middle: the sort's caption and its four pills, plus air. Measured from
/// the widest they can be — the labels are compile-time constants — so this is a floor on
/// the column, not a guess at it.
const BAR_VIEW_COL: f64 = 430.0;
/// Air between a caption and the pills it names, and between the two groups.
const CAPTION_GAP: f64 = 10.0;
/// How many decoded posters stay resident. Roughly nine grid rows' worth — enough that
/// paging back and forth over the same stretch never re-decodes, small enough that a
/// 400-title library cannot accumulate all of it. See `LibraryScreen::evict_art`.
@@ -303,6 +300,19 @@ fn store_view(view: LibraryView, ctx: &mut Ctx) {
/// The caller adds the returned width to `x` and hands the REST of the band to
/// [`TabStrip::render`], which insets itself from whatever rect it is given — so neither
/// side has to be told how wide the other came out.
/// How wide [`strip_caption`] will draw `label`.
///
/// Separate from the draw because a group anchored to the TRAILING edge has to know its own
/// width before it can decide where to start — and measuring it a second way, by hand, is how
/// the measured edge ends up somewhere the drawn edge is not.
pub(super) fn caption_width(fonts: &Fonts, label: &str, k: f64) -> f64 {
let size = 11.0 * k;
// `draw_tracked` adds tracking after every character including the last, so the ink ends
// one gap short of the pen.
f64::from(fonts.measure(label, W::SemiBold, size))
+ 1.4 * k * (label.chars().count().saturating_sub(1)) as f64
}
pub(super) fn strip_caption(
canvas: &Canvas,
fonts: &Fonts,
@@ -704,7 +714,6 @@ impl LibraryScreen {
/// the top screen once a frame and swaps the returned screen in where the shelf stands —
/// guarded on a settled transition, or a swap can land on a push the user has already
/// reversed with B and put them on the collections instead of home.
#[allow(dead_code)]
pub(crate) fn collections_upgrade(
&mut self,
library: &LibraryShared,
@@ -1397,42 +1406,31 @@ impl LibraryScreen {
/// is named here anyway: one strip that answers both questions the same way is a control
/// the user finds once.
fn draw_bar(&mut self, canvas: &Canvas, bar: Rect, k: f64, fonts: &Fonts, dt: f64) {
// Focused, the WHOLE band lifts — the two rows are one control here (◀ ▶ step the
// sort, the shoulders pick the arrangement), so a ring around one pill row would
// name a focus the input model does not have. The glass and the brand hairline are
// the same pair the grid's focused cell uses, so this reads as focus rather than as
// a new kind of chrome.
// Focused, the WHOLE band takes an accent WASH — the two groups are one control here
// (◀ ▶ step the sort, the shoulders pick the arrangement), so a ring around one pill
// row would name a focus the input model does not have.
//
// A wash and nothing else: no border, no halo, no glass. This was a glass panel with
// a brand hairline and a halo behind it, which is the console's recipe for a floating
// SURFACE — and a strip of chrome sitting flat in the field is not one. It read as a
// dark slab with a line round it, worse on the pale palettes where the glass turns to
// frost over an already-light field. The accent is palette-derived, so at 14 % it is
// legible at both poles without ever becoming an object.
if self.bar.focus {
crate::theme::focus_halo(canvas, bar, BAR_CORNER as f32, k as f32, 1.0);
crate::theme::panel(
canvas,
bar,
BAR_CORNER as f32,
None,
crate::theme::PanelStroke::Brand(0.9),
k as f32,
canvas.draw_rrect(
RRect::new_rect_xy(bar, (BAR_CORNER * k) as f32, (BAR_CORNER * k) as f32),
&fill(accent(0.14)),
);
}
// Two columns. The sort leads, at the same inset and in the same place the
// Collections screen puts the identical pills, so a user who has seen one has seen
// the other; the arrangement takes the trailing column, where the shoulders that
// change it are also the trailing pair.
// Two groups, each anchored to the edge it belongs to: the sort LEADS at the same
// inset as the heading above it, the arrangement TRAILS at the same inset as the
// controller chip above it — and the shoulders that change it are the trailing pair
// of buttons, so the hand and the eye agree.
//
// Halfway, but never nearer than the sort's own column is wide. The console's scale
// follows the window's HEIGHT alone (`shell::render`), so on a tall narrow window a
// plain half would land the VIEW caption in the middle of the sort's pills. Pushed
// out instead, the arrangement's pills run off the trailing edge on a window that
// narrow — the shoulders still change it and the legend still says so, where two
// overlapping rows would leave neither readable.
let half = bar
.center_x()
.max(bar.left + (BAR_VIEW_COL * k) as f32)
.min(bar.right);
let sort_x = f64::from(bar.left) + EDGE_INSET * k;
let sort_left = sort_x + strip_caption(canvas, fonts, "SORT", bar, sort_x, k);
let view_x = f64::from(half);
let view_left = view_x + strip_caption(canvas, fonts, "VIEW", bar, view_x, k);
// The arrangement used to START at the band's midpoint, which is neither centred nor
// trailing: it read as a control that had been put down wherever there was room. An
// anchored pair is the same structure the top band already has, so the two rows line
// up as one piece of chrome rather than three.
let sorts: Vec<&str> = crate::collate::SortKey::ALL
.iter()
.map(|s| s.label())
@@ -1441,9 +1439,41 @@ impl LibraryScreen {
.iter()
.position(|s| *s == self.sort)
.unwrap_or(0);
let views: Vec<&str> = LibraryView::ALL.iter().map(|v| v.label()).collect();
// Each group is caption + gap + pills, measured whole. The gap is spelled out for
// BOTH rather than left to `TabStrip`'s own leading inset, which only appears when the
// rect it is handed has slack to spend: the trailing group's rect is exactly as wide
// as its pills, so it had no slack, no inset, and its caption ended up touching the
// first pill while the leading group — handed a wide rect — kept a gap by accident.
let gap = CAPTION_GAP * k;
let sort_cap = caption_width(fonts, "SORT", k);
let view_cap = caption_width(fonts, "VIEW", k);
let sort_pills = crate::widgets::TabStrip::width(&sorts, fonts, k);
let view_pills = crate::widgets::TabStrip::width(&views, fonts, k);
let sort_x = f64::from(bar.left) + EDGE_INSET * k;
// Clamped so a narrow window degrades by crowding the sort's own column rather than
// sliding off the leading edge: the console's scale follows the window's HEIGHT alone,
// so a tall narrow window can put these two groups closer than either likes.
let view_x = (f64::from(bar.right) - EDGE_INSET * k - view_cap - gap - view_pills)
.max(sort_x + sort_cap + gap + sort_pills + gap);
strip_caption(canvas, fonts, "SORT", bar, sort_x, k);
strip_caption(canvas, fonts, "VIEW", bar, view_x, k);
let sort_left = sort_x + sort_cap + gap;
let view_left = view_x + view_cap + gap;
// Both strips are handed a rect exactly as wide as their pills, so neither depends on
// that inset for its position — the caller places them.
self.bar.sort_tabs.render(
canvas,
Rect::from_ltrb(sort_left as f32, bar.top, half, bar.bottom),
Rect::from_ltrb(
sort_left as f32,
bar.top,
(sort_left + sort_pills) as f32,
bar.bottom,
),
&sorts,
sort_at,
fonts,
@@ -1451,14 +1481,18 @@ impl LibraryScreen {
dt,
);
let views: Vec<&str> = LibraryView::ALL.iter().map(|v| v.label()).collect();
let view_at = LibraryView::ALL
.iter()
.position(|v| *v == self.view_mode)
.unwrap_or(0);
self.bar.view_tabs.render(
canvas,
Rect::from_ltrb(view_left as f32, bar.top, bar.right, bar.bottom),
Rect::from_ltrb(
view_left as f32,
bar.top,
(view_left + view_pills) as f32,
bar.bottom,
),
&views,
view_at,
fonts,
@@ -1512,7 +1546,14 @@ impl LibraryScreen {
row as f64 * pitch_y + heading_h + section_gap
};
let content_h = row_top(shape.rows().saturating_sub(1)) + ch;
// …and the SAME inset again at the end, which is the other half of the same defect.
// The content used to stop at the last row's card bottom, so at maximum scroll that
// card sat exactly flush with the viewport's clip and lost its focus scale, its
// shadow and its label off the bottom edge — reported from a Deck as the grid being
// cut off at the bottom, once the top had air and the asymmetry became visible. The
// scroll range is what carries this: `content_h` is the only thing the clamp knows
// about, so the air has to be part of the content rather than part of the viewport.
let content_h = row_top(shape.rows().saturating_sub(1)) + ch + heading_h;
// The detail band keeps its place at the bottom; the grid scrolls above it.
let view_h = f64::from(rect.height()) - DETAIL_BAND * k;
let (focus_row, _) = shape.cell_of(self.cursor.max(0) as usize);
+33
View File
@@ -453,6 +453,39 @@ impl Shell {
}
}
}
self.collections_handover();
}
/// "Start in collections": a shelf that has just learned it holds more than one collection
/// stands aside for the collections screen.
///
/// It lives here rather than in the screen because a screen cannot replace ITSELF — the
/// decision needs the library model and the settings, which the shelf has, but the swap
/// needs the stack, which only the shell has. The shelf answers the question and hands
/// back a screen; this puts it where the shelf was standing.
///
/// Guarded on a settled transition. Mid-flight the stack's top is not yet what the user
/// is looking at, and swapping under a push the user has already reversed with B would
/// land them on the collections of a host they just backed out of.
fn collections_handover(&mut self) {
if !matches!(self.motion, Motion::None) {
return;
}
// Field borrows rather than clones: this runs every frame for the life of the shelf,
// and `Settings` is a struct of owned Strings. `stack` is borrowed mutably while
// `library` and `settings` are borrowed shared — disjoint fields, so the shelf can
// read both while it is itself being held.
let upgraded = match self.stack.last_mut() {
Some(Screen::Library(shelf)) => {
shelf.collections_upgrade(&self.library, &self.settings)
}
_ => None,
};
if let Some(screen) = upgraded {
let n = self.stack.len();
self.stack[n - 1] = Screen::Collections(screen);
}
}
fn start_connect(&mut self, intent: ConnectIntent) {
+78
View File
@@ -659,6 +659,84 @@ fn mixed_library(library: &LibraryShared) {
]);
}
/// "Start in collections" actually starts in collections — asserted on the SHELL, because
/// the shelf was never the part that was broken.
///
/// The handover shipped dead: `LibraryScreen::collections_upgrade` was written, documented and
/// unit-tested for its DECISION, and then nothing ever called it. It carried an
/// `#[allow(dead_code)]`, which is precisely what stopped the compiler from saying so, and the
/// shelf's own tests passed throughout because they called it directly. The setting was on,
/// the shelf agreed it should stand aside, and the library opened on the shelf anyway.
///
/// So this drives `Shell::sync` and asserts on the STACK. A screen cannot replace itself —
/// only the shell owns the stack — so the shell is where the wiring has to be witnessed.
#[test]
fn the_setting_hands_a_multi_platform_library_over_to_collections() {
let games: Vec<crate::library::LibraryGame> = platform_games();
for (want_collections, enabled) in [(true, true), (false, false)] {
let (mut s, _console, library) = shell(vec![
Screen::Home(HomeScreen::new()),
Screen::Library(LibraryScreen::new(&hosts()[0])),
]);
s.settings.library_collections = enabled;
// The shelf must see its OWN fetch go out before it will read the model: a library
// that is Ready before the fetch is the PREVIOUS host's, still sitting in the shared
// model. A default library is Loading, so this frame is that proof.
s.sync();
assert!(
matches!(s.stack.last(), Some(Screen::Library(_))),
"nothing to hand over to while the fetch is still out"
);
library.set_games(games.clone());
s.sync();
if want_collections {
assert!(
matches!(s.stack.last(), Some(Screen::Collections(_))),
"the setting is on and the library has four platforms — it must open on them"
);
assert_eq!(
s.stack.len(),
2,
"it REPLACES the shelf, never stacks on it"
);
} else {
assert!(
matches!(s.stack.last(), Some(Screen::Library(_))),
"with the setting off the library opens on its shelf"
);
}
}
}
/// …and a library with only ONE collection opens on its shelf whatever the setting says,
/// because a collections screen listing a single tile is a press that buys nothing.
#[test]
fn one_collection_is_not_worth_a_screen() {
let (mut s, _console, library) = shell(vec![
Screen::Home(HomeScreen::new()),
Screen::Library(LibraryScreen::new(&hosts()[0])),
]);
s.settings.library_collections = true;
s.sync();
library.set_games(
platform_games()
.into_iter()
.map(|mut g| {
g.platform = Some("PlayStation 3".into());
g
})
.collect(),
);
s.sync();
assert!(
matches!(s.stack.last(), Some(Screen::Library(_))),
"one platform is not a set of collections"
);
}
/// The user's flow, verbatim: group by platform, walk the platforms, pick PS3, see its
/// games — and get back out again. This is the whole point of Part C, so it is asserted
/// end to end rather than in pieces.
+30 -8
View File
@@ -647,7 +647,34 @@ pub(crate) struct TabStrip {
pills: Vec<Rect>,
}
/// A pill's label size and its padding either side, in design units.
const PILL_TEXT: f64 = 13.0;
const PILL_PAD_X: f64 = 13.0;
/// Air between two pills.
const PILL_GAP: f64 = 7.0;
/// Each pill's width and the run's total, in device px.
///
/// Shared with [`TabStrip::width`] rather than computed twice, because a caller that
/// RIGHT-ALIGNS a strip has to know the run's width before the strip draws itself — and a
/// second copy of this arithmetic would put the measured edge somewhere the drawn edge is not.
fn pill_widths(labels: &[&str], fonts: &Fonts, k: f64) -> (Vec<f64>, f64) {
let size = PILL_TEXT * k;
let widths: Vec<f64> = labels
.iter()
.map(|l| f64::from(fonts.measure(l, W::SemiBold, size)) + 2.0 * PILL_PAD_X * k)
.collect();
let total = widths.iter().sum::<f64>() + PILL_GAP * k * (labels.len().saturating_sub(1)) as f64;
(widths, total)
}
impl TabStrip {
/// How wide this run of pills draws. For a caller placing the strip against a TRAILING
/// edge, where the position depends on the width.
pub(crate) fn width(labels: &[&str], fonts: &Fonts, k: f64) -> f64 {
pill_widths(labels, fonts, k).1
}
pub(crate) fn new() -> TabStrip {
TabStrip {
indicator: None,
@@ -686,15 +713,10 @@ impl TabStrip {
if labels.is_empty() {
return;
}
let size = 13.0 * k;
let pad_x = 13.0 * k;
let gap = 7.0 * k;
let pill_h = 30.0 * k;
let widths: Vec<f64> = labels
.iter()
.map(|l| f64::from(fonts.measure(l, W::SemiBold, size)) + 2.0 * pad_x)
.collect();
let total: f64 = widths.iter().sum::<f64>() + gap * (labels.len() - 1) as f64;
let size = PILL_TEXT * k;
let (widths, total) = pill_widths(labels, fonts, k);
let gap = PILL_GAP * k;
// Leading, under the heading it belongs to — a strip centred beneath a left-aligned
// title reads as two unrelated pieces of chrome. Both callers hand this widget the
// full content width, so the inset is measured here rather than baked into the band.