From cab8d804a5aa3307f2fdd261fd1ceaf476c08b64 Mon Sep 17 00:00:00 2001 From: Andreas Reinhold / reini Date: Mon, 14 Sep 2026 22:05:40 +0200 Subject: [PATCH] Reveal a cut-off carousel item under reduced motion Plan step 36 review of a149970d. Reduced motion writes a zero inset for every item, so isMasked() never called one partly shown and a press on an item the row cut off did nothing. It now reads the item's box against the scrollport there, as it already did for multi-aspect, and the browser test the rewrite added to document the gap asserts the reveal instead (plan step 32's owed test). A focused item's inset ring is 3px in again: Tailwind's -outline-offset-3 is 3px, not the 12px the rewrite wrote. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Qwx5USif3wFFmxtHg5U1g9 --- resources/css/components/carousel-item.css | 7 ++++--- resources/js/carousel.js | 9 ++++++--- tests/Browser/CarouselTest.php | 14 +++++++------- tests/Feature/Components/CarouselTest.php | 6 ++++++ 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/resources/css/components/carousel-item.css b/resources/css/components/carousel-item.css index bb6d48dc..01a7367a 100644 --- a/resources/css/components/carousel-item.css +++ b/resources/css/components/carousel-item.css @@ -12,8 +12,9 @@ * * The item renders the shared `md-focus-ring` class (foundation/interaction.css) rather than a * hand-rolled ring — the box the class draws *is* the item's own whole hit area — refined here to - * an inset ring: an item sits edge to edge with its neighbours in the row, so the class's own 2px - * *outward* offset would draw under the next item instead of around this one. + * an inset ring, 3px in, its own width (the old `focus-visible:-outline-offset-3`): items sit 8px + * apart with the row clipping them, so the class's own 2px *outward* offset would be cut off at the + * row's edges and crowd the next item. * * Full-screen: edge to edge, no corner, no mask — `--material-carousel-shift`/`-inset` go unread. * Multi-aspect: the view writes the item's own `aspect-ratio` inline (`aspect`, a prop, held @@ -40,7 +41,7 @@ } [data-md-carousel-item]:focus-visible { - outline-offset: -12px; + outline-offset: -3px; } [data-md-carousel='full-screen'] > [data-md-carousel-scroller] > [data-md-carousel-item] { diff --git a/resources/js/carousel.js b/resources/js/carousel.js index 27d8cb51..c41f79af 100644 --- a/resources/js/carousel.js +++ b/resources/js/carousel.js @@ -26,7 +26,8 @@ * Under reduced motion the buttons and keys scroll instantly and nothing is masked at all: M3 * says the parallax goes and items "should no longer expand as they come into view — all items * are the same size", so every item stays at the strategy's large size and the keylines only - * decide where the row snaps. + * decide where the row snaps. An item is then "not fully open" when the row cuts it off, so a + * press or focus on it still brings it into view. * * --------------------------------------------------------------------------------------- * Keyline maths ported from androidx (https://github.com/androidx/androidx), commit @@ -1218,8 +1219,10 @@ document.addEventListener('alpine:init', () => { return Math.abs(state.snaps[index] - this.scrollOffset()) > 1 } - // Nothing masks a multi-aspect item, so "not fully open" is "cut off by the row". - if (state.measured) { + // Nothing masks a multi-aspect item, nor any item under reduced motion (render() + // writes a zero inset for every one), so there "not fully open" is "cut off by the + // row": its box reaching past the scrollport's edge. + if (state.measured || state.reducedMotion.matches) { const row = this.$refs.scroller.getBoundingClientRect() const item = state.items[index].element.getBoundingClientRect() diff --git a/tests/Browser/CarouselTest.php b/tests/Browser/CarouselTest.php index d45f1dff..f3cdb6ef 100644 --- a/tests/Browser/CarouselTest.php +++ b/tests/Browser/CarouselTest.php @@ -174,12 +174,10 @@ it('scrolls instantly and leaves every item unmasked under reduced motion', func JS)); }); -it('leaves a reduced-motion click on an item cut off at the row\'s edge where it is', function () { +it('brings an item cut off at the row\'s edge into view when it is pressed under reduced motion', function () { // Owed by the Chromium baseline (step 32, docs/plans/material-3-browser-tests.md): reduced - // motion writes a zero inset for every item (C-05, so none of them count as masked at all), - // and the click-to-reveal affordance gates on that inset, so a press on an item only cut off - // by the row's own edge — not by a mask — does not scroll it into view. Documented here, not - // fixed: fixing isMasked() for this case is outside a hook rename. + // motion writes a zero inset for every item (C-05), so a mask cannot say an item is only partly + // shown; carousel.js reads the item's box against the row's instead, as for multi-aspect. $page = carouselShowcase(['reducedMotion' => 'reduce']) ->assertScript(onCarousel(0, <<<'JS' const row = scroller.getBoundingClientRect() @@ -190,8 +188,10 @@ it('leaves a reduced-motion click on an item cut off at the row\'s edge where it $page->script(onCarousel(0, 'items[4].querySelector(\'[data-md-carousel-content]\').click()')); $page->assertScript(onCarousel(0, <<<'JS' - await pause(400) - return scroller.scrollLeft === 0 + await pause(100) + const row = scroller.getBoundingClientRect() + const item = items[4].getBoundingClientRect() + return Math.abs(scroller.scrollLeft) > 0 && item.left >= row.left - 1 && item.right <= row.right + 1 JS)); }); diff --git a/tests/Feature/Components/CarouselTest.php b/tests/Feature/Components/CarouselTest.php index 83a36c24..5827bff6 100644 --- a/tests/Feature/Components/CarouselTest.php +++ b/tests/Feature/Components/CarouselTest.php @@ -1,5 +1,6 @@ and($item->declarations("[data-md-carousel='multi-aspect'] > [data-md-carousel-scroller] > [data-md-carousel-item]")) ->toBe(['width' => 'auto', 'border-radius' => 'var(--md-sys-shape-corner-xl)']); }); + +it('rings a focused item 3px inside its edge, and reveals an item the row cuts off under reduced motion', function () { + expect(ComponentStylesheet::read('carousel-item')->declarations('[data-md-carousel-item]:focus-visible'))->toBe(['outline-offset' => '-3px']) + ->and(File::get(__DIR__.'/../../../resources/js/carousel.js'))->toContain('if (state.measured || state.reducedMotion.matches) {'); +});