diff --git a/.changes/dashboard-hero-stable-height.md b/.changes/dashboard-hero-stable-height.md new file mode 100644 index 000000000..c1761cb5e --- /dev/null +++ b/.changes/dashboard-hero-stable-height.md @@ -0,0 +1,8 @@ +--- +type: fix +area: dashboard +--- + +The dashboard no longer jumps while the hero banner rotates on its own. The +banner now keeps the height of its tallest slide, so the rails below stay put +instead of moving a few pixels every 8 seconds. diff --git a/apps/electron-backend-e2e/src/dashboard-hero-legibility.e2e.ts b/apps/electron-backend-e2e/src/dashboard-hero-legibility.e2e.ts index 7accbb72e..5c4e906ac 100644 --- a/apps/electron-backend-e2e/src/dashboard-hero-legibility.e2e.ts +++ b/apps/electron-backend-e2e/src/dashboard-hero-legibility.e2e.ts @@ -119,7 +119,9 @@ async function showSlide(page: Page, index: number): Promise { page.evaluate(() => [ document.querySelector('.hero__backdrop--active'), - document.querySelector('.hero__content'), + document.querySelector( + '[data-test-id=dashboard-hero-slide]' + ), ].every( (element) => element && @@ -135,9 +137,16 @@ async function showSlide(page: Page, index: number): Promise { return page.getByTestId('dashboard-hero-slide'); } -/** Sum of non-input layout shifts while the hero runs through every slide - * on its own, at a shortened interval. */ -async function rotationLayoutShift(page: Page): Promise { +interface RotationShift { + /** Sum of the non-input layout-shift scores. */ + score: number; + /** What moved and by how much, e.g. `LIB-DASHBOARD-RAIL 0,7`. */ + sources: string[]; +} + +/** Non-input layout shifts while the hero runs through every slide on its + * own, at a shortened interval. */ +async function rotationLayoutShift(page: Page): Promise { const hero = page.getByTestId('dashboard-hero'); const dots = page.getByTestId('dashboard-hero-dot'); const count = await dots.count(); @@ -147,18 +156,38 @@ async function rotationLayoutShift(page: Page): Promise { '--hero-rotation-ms', '600ms' ); - const shifts: number[] = []; + const shifts: RotationShift = { score: 0, sources: [] }; new PerformanceObserver((list) => { for (const entry of list.getEntries() as (PerformanceEntry & { value: number; hadRecentInput: boolean; + sources: { + node: Node | null; + previousRect: DOMRectReadOnly; + currentRect: DOMRectReadOnly; + }[]; })[]) { - if (!entry.hadRecentInput) { - shifts.push(entry.value); + if (entry.hadRecentInput) { + continue; + } + shifts.score += entry.value; + for (const { + node, + previousRect, + currentRect, + } of entry.sources) { + const name = + node instanceof Element + ? [node.nodeName, ...node.classList].join('.') + : (node?.nodeName ?? 'removed'); + const dx = Math.round(currentRect.x - previousRect.x); + const dy = Math.round(currentRect.y - previousRect.y); + shifts.sources.push(`${name} ${dx},${dy}`); } } }).observe({ type: 'layout-shift' }); - (window as unknown as { __heroShifts: number[] }).__heroShifts = shifts; + (window as unknown as { __heroShifts: RotationShift }).__heroShifts = + shifts; }); // Back to the first slide after one full cycle, then a quiet moment. const first = await dots.evaluateAll((all) => @@ -172,19 +201,37 @@ async function rotationLayoutShift(page: Page): Promise { ); } await page.waitForTimeout(500); - return page.evaluate(() => - (window as unknown as { __heroShifts: number[] }).__heroShifts.reduce( - (sum, value) => sum + value, - 0 - ) + return page.evaluate( + () => + (window as unknown as { __heroShifts: RotationShift }).__heroShifts ); } -/** Adds what a TMDB-enriched slide shows: a rating chip and an overview. */ +/** One unattended rotation moves next to nothing on the page, and nothing + * at all inside the hero. */ +async function expectStableRotation( + page: Page, + label: string, + results: string[] +): Promise { + const shift = await rotationLayoutShift(page); + const sources = shift.sources.join('; '); + results.push( + `${label} rotation layout shift ${shift.score.toFixed(5)} [${sources}]` + ); + expect(shift.score, `${label}: ${sources}`).toBeLessThan(0.001); + expect( + shift.sources.filter((source) => source.includes('hero__')), + `${label}: nothing in the hero moves` + ).toEqual([]); +} + +/** Adds what a TMDB-enriched slide shows: a rating chip and an overview. + * Once per slide: a slide keeps its content while another one is shown. */ async function enrichSlide(slide: Locator): Promise { await slide.evaluate((content, overview) => { const chip = content.querySelector('.hero__pill'); - if (chip) { + if (chip && !content.querySelector('.meta-chip--rating')) { const rating = chip.cloneNode() as HTMLElement; rating.classList.add('meta-chip--rating'); rating.textContent = '★ 7.4'; @@ -287,15 +334,16 @@ test.describe('Dashboard hero legibility', () => { ).toHaveCount(1); // An unattended rotation, counted like the launch journey's - // settled layout-shift counter (non-input shifts only). Slides of - // different heights still resize the hero by a few pixels and - // move the rails below (0.005 here, 0.013 before this change); - // a scrim or heading that reflowed the slide would add lines. - const shift = await rotationLayoutShift(page); - results.push(`rotation layout shift ${shift.toFixed(3)}`); - expect(shift).toBeLessThan(0.02); + // settled layout-shift counter (non-input shifts only), moves + // nothing. The hero lays out every slide and is as tall as the + // tallest; with only the shown slide in flow, slides of different + // heights resized it and moved every rail below by 7px (a score + // of 0.005). Every rotation dot keeps its width, so the active + // one no longer pushes its neighbours either. + await expectStableRotation(page, 'wide', results); - await page.getByTestId('dashboard-hero-pause').click(); + const pause = page.getByTestId('dashboard-hero-pause'); + await pause.click(); const kinds = await slideKinds(page); for (const theme of ['light', 'dark'] as const) { await applyTheme(page, theme); @@ -304,7 +352,7 @@ test.describe('Dashboard hero legibility', () => { // The narrow layout is the dashboard container's // ≤720px query, not the window width. const narrow = await page - .locator('.hero__content') + .getByTestId('dashboard-hero-slide') .evaluate( (content) => getComputedStyle(content).maxWidth === 'none' @@ -342,6 +390,12 @@ test.describe('Dashboard hero legibility', () => { } } } + + // Again at the narrow width, where slides wrap the most, now that + // every slide also carries a rating and a two-line overview. + await pause.click(); + await expect(pause).toHaveAttribute('aria-pressed', 'false'); + await expectStableRotation(page, 'narrow enriched', results); } finally { const report = testInfo.outputPath('contrast.txt'); writeFileSync(report, results.join('\n')); diff --git a/apps/electron-backend-e2e/src/theme-contrast.ts b/apps/electron-backend-e2e/src/theme-contrast.ts index 895698d31..5bf7f4269 100644 --- a/apps/electron-backend-e2e/src/theme-contrast.ts +++ b/apps/electron-backend-e2e/src/theme-contrast.ts @@ -228,7 +228,15 @@ export async function measureBackdropTextContrast( } const range = document.createRange(); range.selectNodeContents(element); - const box = range.getBoundingClientRect(); + // The line boxes, cut to the element's own box: a line-clamped or + // ellipsized text has line boxes past it that never show, and those + // would measure whatever sits there (the next row's pills). + const lines = range.getBoundingClientRect(); + const own = element.getBoundingClientRect(); + const left = Math.max(lines.left, own.left); + const top = Math.max(lines.top, own.top); + const right = Math.min(lines.right, own.right); + const bottom = Math.min(lines.bottom, own.bottom); const style = (element as HTMLElement).style; const previous = style.getPropertyValue('color'); style.setProperty('color', 'transparent', 'important'); @@ -238,10 +246,10 @@ export async function measureBackdropTextContrast( // Whole pixels inside the line boxes, clear of glyph edges that // spill past them. clip: { - x: Math.ceil(box.left), - y: Math.ceil(box.top), - width: Math.max(1, Math.floor(box.width) - 1), - height: Math.max(1, Math.floor(box.height) - 1), + x: Math.ceil(left), + y: Math.ceil(top), + width: Math.max(1, Math.floor(right - left) - 1), + height: Math.max(1, Math.floor(bottom - top) - 1), }, }; }); diff --git a/docs/architecture/workspace-dashboard.md b/docs/architecture/workspace-dashboard.md index 96caaf115..48af3488b 100644 --- a/docs/architecture/workspace-dashboard.md +++ b/docs/architecture/workspace-dashboard.md @@ -108,6 +108,16 @@ Render rules: `clamp(320px, 42vh, 520px)` so the first rail starts above the fold, and uses `--app-content-bg` as its scrim so it dissolves into the page in both themes. +Height: that clamp is a floor, not a fixed height. Every slide's content is +laid out in the same grid cell at the bottom of the banner, so it is as tall +as its tallest slide whichever one is shown, and an unusually full slide or a +long translation grows it instead of being cut. Only the active slide is +shown; the others are `inert` and `visibility: hidden`. When only the shown +slide was in flow, each automatic rotation between slides of different +heights resized the banner and moved every rail below it, every 8 s on an idle +dashboard. Late data (TMDB extras, the live slide's first EPG answer, the next +programme) can still grow the tallest slide, once, when it arrives. + Slides (`pickDashboardHeroSources`, at most four, stable order, each title once): @@ -161,9 +171,11 @@ Semantics: the page has one stable, visually hidden `h1` ("Dashboard", `dashboard-page-heading`); each slide title is an `h2`, like the rail titles. Slide changes are announced by one polite live region (`dashboard-hero-announcement`, position and title) that lives outside the -re-created slide and is silent while the slides rotate on their own. A +slides and is silent while the slides rotate on their own. A slide's progress bar is named after its title (a live slide: the programme) -and a title's reads "N% watched". The dots are 24px targets (WCAG 2.5.8). +and a title's reads "N% watched". The dots are 26 × 24px targets (WCAG 2.5.8) +of one fixed width; the active pill is the same 18px bar with its +`clip-path` opened, so a slide change moves no dot. Rotation is the active dot's CSS fill animation (8 s); its `animationend` advances. The fill animates `transform` only (a bar sliding in under the @@ -176,10 +188,14 @@ region: ←/→ switch slides and Enter follows the primary action. The active s by id, so a late live slide never moves the user off the current one. Test hooks: `dashboard-hero`, `dashboard-hero-slide` (`data-hero-kind`), `dashboard-hero-dot`, `dashboard-hero-pause`, -`dashboard-hero-primary-action`, `dashboard-hero-secondary-action`. +`dashboard-hero-primary-action`, `dashboard-hero-secondary-action`. The +slide hooks mark the shown slide only; the inert slides carry none. `dashboard-hero-rotation.e2e.ts` drives the real fill animation (with a shortened `--hero-rotation-ms`) to prove its `animationend` still advances -and that pause holds the slide. +and that pause holds the slide. `dashboard-hero-legibility.e2e.ts` sums the +layout shifts of an unattended rotation, at the wide width and again at the +narrow one once every slide carries a rating and an overview: under 0.001 in +all, and none inside the hero. ## Rail Contract diff --git a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.html b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.html index cb0d80a23..3a5358d4f 100644 --- a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.html +++ b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.html @@ -58,18 +58,24 @@ - @for (slide of [active]; track slide.id) { + + @for (slide of slides(); track slide.id) { + @let current = $index === activeIndex();
{{ slide.title }} -

+

@if (slide.episodeBadge) { {{ slide.episodeBadge @@ -127,7 +138,9 @@ @if (slide.programmeTitle) {

{{ slide.programmeTitle }}

@@ -162,7 +175,7 @@ @let primary = slide.primaryAction; @@ -182,7 +195,9 @@ @if (slide.secondaryAction; as secondary) { diff --git a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.scss b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.scss index 07ce89e4e..9487469b0 100644 --- a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.scss +++ b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.scss @@ -47,12 +47,15 @@ ); position: relative; - // A floor, not a fixed height: the slide sits in normal flow at the - // bottom, so an unusually full slide (two-line title, programme, - // synopsis, wrapped pills) grows the banner instead of losing its top. - display: flex; - flex-direction: column; - justify-content: flex-end; + // A floor, not a fixed height: every slide sits in normal flow in the + // same grid cell at the bottom, so an unusually full slide (two-line + // title, programme, synopsis, wrapped pills) grows the banner instead of + // losing its top. The banner is as tall as its tallest slide whichever + // one is shown: with only the shown slide in flow, each rotation between + // slides of different heights resized it and moved every rail below. + display: grid; + grid-template-columns: minmax(0, 1fr); + align-items: end; box-sizing: border-box; min-height: clamp(320px, 42vh, 520px); // Flush with the content area's top and sides; the rails page's own @@ -223,6 +226,7 @@ // ── Slide content ────────────────────────────────────────────────────────── .hero__content { + grid-area: 1 / 1; position: relative; z-index: 1; box-sizing: border-box; @@ -237,6 +241,15 @@ animation: hero-content-in 420ms cubic-bezier(0.2, 0.7, 0.2, 1) both; } +// The slides not on screen still size the hero but are never shown or +// reached. Dropping their animation replays the entrance each time a slide +// becomes the active one. The attribute outranks the narrow layout's +// entrance rule, which would otherwise keep it applied. +.hero__content[inert] { + visibility: hidden; + animation: none; +} + @keyframes hero-content-in { from { opacity: 0; @@ -524,12 +537,15 @@ gap: 2px; } -// 6px dots inside a 24px hit area (WCAG 2.5.8 target size), 2px apart; the -// active one stretches into a pill whose fill runs for one rotation interval. +// 6px dots in 26 × 24px hit areas (WCAG 2.5.8 target size), 2px apart; the +// active one opens into an 18px pill whose fill runs for one rotation +// interval. Every dot keeps its footprint and the pill is the same bar +// unclipped: a wider active button, and a pill that grew in width, moved +// the dots on every rotation (layout shifts on an idle dashboard). .hero__dot { display: grid; place-items: center; - width: 24px; + width: 26px; height: 24px; padding: 0; border: 0; @@ -539,7 +555,7 @@ &::before { content: ''; grid-area: 1 / 1; - width: 6px; + width: 18px; height: 6px; border-radius: 3px; background: color-mix( @@ -547,8 +563,10 @@ var(--app-on-surface, #e6e1e5) 32%, transparent ); + // The middle 6px of the bar: a round dot. + clip-path: inset(0 6px round 3px); transition: - width 0.25s ease, + clip-path 0.25s ease, background-color 0.15s ease; } @@ -566,25 +584,20 @@ border-radius: 6px; } - &--active { - width: 32px; - - &::before { - width: 18px; - } + &--active::before { + clip-path: inset(0 round 3px); } } +// Over the pill, and shown on the active dot only. .hero__dot-fill { grid-area: 1 / 1; - justify-self: start; - // Aligns with the 18px pill centred in the 32px button. - margin-left: 7px; - width: 0; + width: 18px; height: 6px; border-radius: 3px; overflow: hidden; pointer-events: none; + visibility: hidden; // The fill is a full-width bar that slides in from the left under the // pill's rounded clip. Only `transform` animates, so the compositor runs @@ -600,7 +613,7 @@ } .hero__dot--active & { - width: 18px; + visibility: visible; } .hero--rotating .hero__dot--active &::before { diff --git a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.spec.ts b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.spec.ts index 68000d5f4..edab52bf9 100644 --- a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.spec.ts +++ b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.spec.ts @@ -254,6 +254,62 @@ describe('DashboardHeroComponent', () => { ).toBe('/workspace/a'); }); + it('lays out every slide but shows and exposes only the active one', () => { + // The hero is as tall as its tallest slide only while every slide + // stays in flow; re-creating one slide per change resized it. + render(); + const contents = () => + Array.from(host().querySelectorAll('.hero__content')); + const inert = () => contents().map((el) => el.hasAttribute('inert')); + const [first, second] = contents(); + expect(inert()).toEqual([false, true, true]); + const hooks = (testId: string) => + Array.from(host().querySelectorAll(`[data-test-id=${testId}]`)); + for (const testId of [ + 'dashboard-hero-slide', + 'dashboard-hero-badges', + 'dashboard-hero-primary-action', + ]) { + expect(hooks(testId)).toHaveLength(1); + expect(first.contains(hooks(testId)[0])).toBe(true); + } + + finishActiveDot(); + + expect(contents()).toHaveLength(3); + expect(contents()[0]).toBe(first); + expect(contents()[1]).toBe(second); + expect(inert()).toEqual([true, false, true]); + expect(second.getAttribute('data-test-id')).toBe( + 'dashboard-hero-slide' + ); + expect(first.hasAttribute('data-test-id')).toBe(false); + }); + + it('follows Enter to the primary action of the slide on screen', () => { + render(); + const section = host().querySelector( + '[data-test-id=dashboard-hero]' + ) as HTMLElement; + const followed: (string | null)[] = []; + host() + .querySelectorAll('.hero__button--primary') + .forEach((button) => + button.addEventListener('click', (event) => { + event.preventDefault(); + followed.push(button.getAttribute('href')); + }) + ); + + dots()[1].click(); + fixture.detectChanges(); + section.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }) + ); + + expect(followed).toEqual(['/workspace/b']); + }); + it('titles each slide with an h2 and leaves the h1 to the page', () => { render(); @@ -262,8 +318,9 @@ describe('DashboardHeroComponent', () => { }); it('announces slide changes through one live region that outlives the slides', () => { - // A live region inserted together with its text is not announced, - // so it must not belong to the slide that the @for re-creates. + // A live region inserted together with its text is not announced (a + // slide can arrive late) and the slides not on screen are inert, so + // it must not belong to a slide. render(); const announcement = () => host().querySelector( diff --git a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.ts b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.ts index f12b66726..96fd76522 100644 --- a/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.ts +++ b/libs/workspace/dashboard/feature/src/lib/rails/dashboard-hero.component.ts @@ -30,6 +30,8 @@ const REDUCED_MOTION_QUERY = '(prefers-reduced-motion: reduce)'; * * The active slide is tracked by id, so a slide that arrives late (the live * slide waits for its EPG answer) never yanks the user off the current one. + * Every slide is laid out and only the active one is shown, so the hero is + * as tall as its tallest slide and a rotation never resizes it. * * The hero is a focusable region: ←/→ switch slides, Enter follows the * primary action. Rotation also pauses while the document is hidden. @@ -194,8 +196,11 @@ export class DashboardHeroComponent { } if (event.key === 'Enter') { event.preventDefault(); + // Every slide is in the DOM; the inactive ones are inert. (event.currentTarget as HTMLElement) - .querySelector('.hero__button--primary') + .querySelector( + '.hero__content:not([inert]) .hero__button--primary' + ) ?.click(); } }