Sfoglia il codice sorgente

[Fix] Align RollAnnouncer timing with carousel settle (3350ms)

PM batch of 4 known issues — only #4 still needed code changes; the
other three were already addressed in prior commits:

1. U9 carousel modulo-wrap (landing reel) — already fixed in 7382c5e
   at carousel-animation.tsx:129-131.
2. useAddMovie onError + user-visible error — already present
   (onError logs; movie-list-client.tsx:133-138 surfaces an inline
   role="alert" off addMovie.isError).
3. Five dead UI files (expanded-panel, movie-search-panel,
   home-roll-teaser-card, roll-animation, roll-result-card) — already
   removed in c7859af; verified no references remain.
4. RollAnnouncer fired its "complete" aria-live message at 2500ms
   while ListRollCarousel actually settles at 3350ms
   (ENTRANCE_MS 500 + PRE_SPIN_PAUSE_MS 100 + SPIN_DURATION_MS 2750).
   Bump useRoll's ANIMATION_DURATION_MS to 3350 so screen-reader
   users get the result when it's visible. Update use-roll tests to
   match. Update stale carousel comment that claimed timelines were
   intentionally divergent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User 1 mese fa
parent
commit
18ba081a7d

+ 9 - 9
src/__tests__/hooks/use-roll.test.ts

@@ -64,7 +64,7 @@ describe("useRoll", () => {
     vi.restoreAllMocks();
   });
 
-  it("transitions idle -> rolling -> complete with the 2500ms timer", () => {
+  it("transitions idle -> rolling -> complete with the 3350ms timer", () => {
     const { result } = renderHook(() => useRoll());
     const pool = [makeMovie("1"), makeMovie("2"), makeMovie("3")];
 
@@ -79,7 +79,7 @@ describe("useRoll", () => {
     expect(result.current.result).not.toBeNull();
 
     act(() => {
-      vi.advanceTimersByTime(2499);
+      vi.advanceTimersByTime(3349);
     });
     expect(result.current.rollState).toBe("rolling");
 
@@ -97,7 +97,7 @@ describe("useRoll", () => {
       result.current.roll(pool);
     });
     act(() => {
-      vi.advanceTimersByTime(2500);
+      vi.advanceTimersByTime(3350);
     });
     expect(result.current.rollState).toBe("complete");
 
@@ -142,7 +142,7 @@ describe("useRoll", () => {
     pool.push(makeMovie("X"), makeMovie("Y"), makeMovie("Z"));
 
     act(() => {
-      vi.advanceTimersByTime(2500);
+      vi.advanceTimersByTime(3350);
     });
 
     expect(result.current.rollState).toBe("complete");
@@ -160,7 +160,7 @@ describe("useRoll", () => {
       result.current.roll(pool);
     });
     act(() => {
-      vi.advanceTimersByTime(2500);
+      vi.advanceTimersByTime(3350);
     });
     expect(result.current.rollState).toBe("complete");
 
@@ -172,7 +172,7 @@ describe("useRoll", () => {
     expect(result.current.result).not.toBeNull();
 
     act(() => {
-      vi.advanceTimersByTime(2500);
+      vi.advanceTimersByTime(3350);
     });
     expect(result.current.rollState).toBe("complete");
   });
@@ -184,7 +184,7 @@ describe("useRoll", () => {
       result.current.roll([makeMovie("1")]);
     });
     act(() => {
-      vi.advanceTimersByTime(2500);
+      vi.advanceTimersByTime(3350);
     });
 
     const onlySurvivor = makeMovie("only");
@@ -192,7 +192,7 @@ describe("useRoll", () => {
       result.current.roll([onlySurvivor]);
     });
     act(() => {
-      vi.advanceTimersByTime(2500);
+      vi.advanceTimersByTime(3350);
     });
 
     expect(result.current.result?.id).toBe("only");
@@ -243,7 +243,7 @@ describe("useRoll", () => {
     expect(vi.getTimerCount()).toBe(1);
 
     act(() => {
-      vi.advanceTimersByTime(2499);
+      vi.advanceTimersByTime(3349);
     });
     expect(result.current.rollState).toBe("rolling");
 

+ 4 - 3
src/components/dice/list-roll-carousel.tsx

@@ -10,9 +10,10 @@ const ITEM_WIDTH = 112; // w-28
 const ITEM_GAP = 12; // gap-3
 const POSTER_STRIDE = ITEM_WIDTH + ITEM_GAP;
 // Carousel pops in via the dice-emerge keyframe (~500ms scale-bounce), then
-// pauses briefly, then spins. useRoll's own 2500ms timer governs `state`
-// transitions but doesn't gate the visual settle — the carousel owns its
-// timeline and runs slightly longer for a punchier feel.
+// pauses briefly, then spins. Timeline: ENTRANCE_MS + PRE_SPIN_PAUSE_MS +
+// SPIN_DURATION_MS = 3350ms total to settle. useRoll's ANIMATION_DURATION_MS
+// is kept in sync with this sum so RollAnnouncer's "complete" aria-live
+// message fires when the result is actually visible.
 const ENTRANCE_MS = 500; // matches @utility animate-emerge duration
 const PRE_SPIN_PAUSE_MS = 100;
 const SPIN_DURATION_MS = 2750;

+ 7 - 3
src/hooks/use-roll.ts

@@ -6,7 +6,12 @@ import { selectRandomMovie } from "@/lib/dice/randomizer";
 
 export type RollState = "idle" | "rolling" | "complete";
 
-const ANIMATION_DURATION_MS = 2500;
+// Aligned with ListRollCarousel's visible settle: ENTRANCE_MS (500) +
+// PRE_SPIN_PAUSE_MS (100) + SPIN_DURATION_MS (2750) = 3350ms. RollAnnouncer
+// fires its "complete" aria-live message off this state transition, so it
+// must match what sighted users see — earlier values announced the result
+// while the carousel was still spinning.
+const ANIMATION_DURATION_MS = 3350;
 
 const REDUCED_MOTION_QUERY = "(prefers-reduced-motion: reduce)";
 
@@ -66,8 +71,7 @@ export function useRoll(): UseRollReturn {
 
       // Capture by value so concurrent real-time cache mutations to the upstream
       // array cannot change the in-flight winner.
-      const snapshot =
-        eligibleMovies !== undefined ? [...eligibleMovies] : capturedPoolRef.current;
+      const snapshot = eligibleMovies !== undefined ? [...eligibleMovies] : capturedPoolRef.current;
       capturedPoolRef.current = snapshot;
 
       const winner = selectRandomMovie(snapshot);