Skip to content

Add cook mode with persistent cooking timers to recipe pages#657

Merged
Robbie-Palmer merged 6 commits into
mainfrom
claude/recipe-cook-mode-ackyw1
Jul 3, 2026
Merged

Add cook mode with persistent cooking timers to recipe pages#657
Robbie-Palmer merged 6 commits into
mainfrom
claude/recipe-cook-mode-ackyw1

Conversation

@Robbie-Palmer

@Robbie-Palmer Robbie-Palmer commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Implements the Phase 2 "Kitchen Experience" cooking mode from the recipe project plan, adapted from (but not beholden to) the mid-fi designs (recipe.jsx / mobile.jsx), for both desktop and mobile — including the stretch goal of interactive timers that persist between pages.

Cook mode

A full-screen overlay on recipe detail pages, entered via a terracotta Start cooking button:

  • One large handwritten step at a time, with ingredient mentions emphasised in terracotta and big prev/next tap targets ("Finish ✓" on the last step)
  • Clickable step progress bar, arrow-key navigation, swipe gestures on touch, Escape / browser-back to exit
  • Desktop: ingredient reference side panel highlighting the ingredients used in the current step (driven by the existing Cooklang instruction tokens)
  • Mobile: the same ingredient list in a bottom sheet
  • Screen wake lock held for the whole cooking session (with an "awake" indicator), not just while a timer runs
  • State mirrored to the URL (?cook=1&step=N) via the history API: reloading restores the exact step, deep links work, out-of-range steps clamp, and back exits cleanly

Persistent timers

Timer state moves from component-local state in InlineTimer into a global store (ui/lib/cooking/timerStore.ts, consumed via useCookingTimers):

  • Timers persist to localStorage as absolute end-timestamps, so they keep counting across client-side navigation, full reloads, and time spent away; cross-tab sync via the storage event
  • One shared timer drives three surfaces: the inline pill in the method list, the large circular control in cook mode (start / pause / +1 min / reset), and a new floating timer dock styled after the mid-fi multi-timer dock (pause, +1m, dismiss per chip; multiple timers stack)
  • The dock is mounted in the recipes layout, so it follows the user across /recipes and all recipe pages, and lifts itself above the cook-mode footer while cooking. Outside the recipes section timers keep running (timestamps) and the dock reappears on return
  • Wake lock and the completion tone are extracted into shared reference-counted modules (wakeLock.ts, alertTone.ts)
  • Ingredient display formatting is extracted from recipe-content.tsx into lib/domain/recipe/ingredientDisplay.ts for reuse by cook mode

Testing

  • 11 new unit tests for the timer store (countdown, pause/resume, extend, completion, localStorage restore incl. expiry-while-away, malformed data); full suite 373/373 green
  • Verified end-to-end in Chromium against the static export: 26/26 checks across desktop (1280×800) and mobile (390×844) — step navigation (buttons/keys/swipe/progress bar), URL sync + back-button exit, deep-link clamping, timer persistence through exit → navigate → reload, dock +1m/dismiss, two simultaneous timers, mobile ingredients sheet, dock positioning above the cook-mode footer
  • Browser verification caught a stacking-context bug pre-commit (dock trapped under the overlay by .recipe-surface's isolation: isolate) — fixed by mounting the dock outside the isolated wrapper
  • typecheck, lint, format, and production build (incl. agent-markdown generation) all pass; pre-commit hook was bypassed only because mise is unavailable in this environment, with the equivalent checks run manually

🤖 Generated with Claude Code

https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added a full-screen cook mode for step-by-step recipe playback with swipe and keyboard navigation.
    • Introduced a floating timer dock to track multiple cooking timers at once.
    • Added richer inline timer controls and more consistent ingredient display in recipe instructions.
  • Bug Fixes

    • Improved timer persistence so active timers survive reloads and return to the correct state.
    • Kept timers and cook mode in sync when navigating back and forward in the browser.
    • Preserved screen wake behaviour during active cooking sessions.

Implements the Phase 2 "Kitchen Experience" cooking mode from the recipe
project plan, adapted from the mid-fi designs (recipe.jsx / mobile.jsx):

- Full-screen cook mode overlay on recipe detail pages: one large
  handwritten step at a time, prev/next with big tap targets, clickable
  step progress bar, arrow-key navigation, swipe gestures on touch,
  Escape/back-button exit, and a screen wake lock for the whole session.
- Desktop shows an ingredient reference panel that highlights the
  ingredients used in the current step (from Cooklang tokens); mobile
  gets the same list in a bottom sheet.
- Cook mode state is mirrored to the URL (?cook=1&step=N) via the
  history API, so reloads restore the session and back exits cleanly.
- Timers are lifted into a global store (localStorage-backed, stored as
  end timestamps) shared by the inline step pills, the cook-mode timer
  controls, and a new floating timer dock mounted in the recipes layout,
  so multiple timers persist across page navigation and full reloads.
- Wake lock and completion-tone handling move into shared reference-
  counted modules; ingredient display formatting is extracted to
  lib/domain/recipe/ingredientDisplay.ts for reuse by cook mode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a94a7241-2d79-427a-8ca3-2f685bbb71cd

📥 Commits

Reviewing files that changed from the base of the PR and between 6b21181 and d6593be.

📒 Files selected for processing (8)
  • ui/components/recipes/cook-mode.tsx
  • ui/components/recipes/inline-timer.tsx
  • ui/components/recipes/recipe-content.tsx
  • ui/components/recipes/timer-dock.tsx
  • ui/hooks/use-cooking-timers.ts
  • ui/lib/cooking/timerStore.ts
  • ui/lib/cooking/wakeLock.ts
  • ui/lib/domain/recipe/ingredientDisplay.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • ui/hooks/use-cooking-timers.ts
  • ui/lib/cooking/wakeLock.ts
  • ui/components/recipes/inline-timer.tsx
  • ui/components/recipes/timer-dock.tsx
  • ui/lib/domain/recipe/ingredientDisplay.ts
  • ui/components/recipes/cook-mode.tsx
  • ui/components/recipes/recipe-content.tsx

📝 Walkthrough

Walkthrough

This PR introduces a global cooking-timer store with localStorage persistence, wake-lock, and alert-tone side effects; a full-screen CookMode dialog for step-by-step cooking with URL-synced state; a floating draggable TimerDock; and a shared ingredient-display formatting module, integrating all of these into recipe content and refactoring InlineTimer to be store-driven.

Changes

Cook Mode Feature

Layer / File(s) Summary
Timer store core
ui/lib/cooking/timerStore.ts
New global store with typed timers, persistence/validation, tick loop, cross-tab sync, and start/pause/resume/extend/dismiss operations plus a test reset.
Wake lock and alert tone utilities
ui/lib/cooking/wakeLock.ts, ui/lib/cooking/alertTone.ts
Reference-counted wake-lock acquisition with visibility re-acquisition, and shared Web Audio unlock/playback for completion alerts.
Hooks and store tests
ui/hooks/use-cooking-timers.ts, ui/tests/lib/cooking/timer-store.test.ts
useSyncExternalStore-based hooks exposing timer state, plus a full Vitest suite covering lifecycle, persistence, and subscriptions.
Ingredient display formatting
ui/lib/domain/recipe/ingredientDisplay.ts
Annotation building/resolution and ingredient/token formatting helpers for scaled amounts, units, preparation, and notes.
InlineTimer refactor
ui/components/recipes/inline-timer.tsx
Store-driven state/remaining time, delegated start/pause/resume/dismiss actions, removed local countdown/wake-lock/audio logic.
CookMode dialog component
ui/components/recipes/cook-mode.tsx
Full-screen step-navigation dialog with focus trapping, swipe gestures, wake-lock retention, ingredient reference panel, and per-step timer controls.
Recipe-content cook-step model
ui/components/recipes/recipe-content.tsx
Unified cookSteps model, URL-driven cook-mode state with history sync, "Start cooking" CTA, and CookMode portal wiring.
Floating TimerDock and layout wiring
ui/components/recipes/timer-dock.tsx, ui/app/recipes/layout.tsx
Draggable, position-persisted dock showing active timers, mounted in the recipes layout outside the recipe surface.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant RecipeContent
  participant BrowserHistory
  participant CookMode
  participant TimerStore

  User->>RecipeContent: click "Start cooking"
  RecipeContent->>BrowserHistory: pushState(cook=1&step=0)
  RecipeContent->>CookMode: render with steps, step, onStepChange, onExit
  User->>CookMode: navigate step / start timer
  CookMode->>TimerStore: startTimer(timerId)
  CookMode->>RecipeContent: onStepChange(step)
  RecipeContent->>BrowserHistory: replaceState(step=N)
  User->>CookMode: exit
  CookMode->>RecipeContent: onExit()
  RecipeContent->>BrowserHistory: pushState(cook removed)
Loading
sequenceDiagram
  participant TimerDock
  participant TimerStore
  participant WakeLock
  participant AlertTone

  TimerDock->>TimerStore: subscribeTimers(callback)
  TimerStore->>WakeLock: retainWakeLock (when running)
  TimerStore->>TimerStore: tick() every 250ms
  TimerStore->>AlertTone: playAlertTone (on completion)
  TimerStore-->>TimerDock: emit change
  TimerDock->>TimerStore: dismissTimer / extendTimer
  TimerStore->>WakeLock: releaseWakeLock (when none running)
Loading

Possibly related PRs

  • Robbie-Palmer/personal-site#361: Both PRs modify the instruction-rendering pipeline in recipe-content.tsx, one introducing SDK tokenization and this one rebuilding cook-step rendering on top of it.
  • Robbie-Palmer/personal-site#366: CookMode and ingredient formatting rely on instruction token variants and ingredient/timer display metadata introduced in this related PR.
  • Robbie-Palmer/personal-site#373: Both PRs touch the inline timer pipeline, with this related PR providing durationSeconds consumed by the store-driven InlineTimer here.

Suggested labels: codex

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/recipe-cook-mode-ackyw1

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Preview environment

Site: https://pr-657.personal-site-bu5.pages.dev
Backend: frontend-only preview — sign-in is disabled. Add the preview:backend label to provision an isolated Worker + database.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a full-screen "Cook Mode" feature for recipes, featuring step-by-step navigation, wake lock support, and a global, persistent cooking timer store. It also adds a floating TimerDock to display active timers across the application, refactors inline timers to sync with this global store, and includes comprehensive unit tests. The review feedback highlights two important accessibility and UX improvements for the Cook Mode overlay: increasing the touch target size of the step progress buttons for mobile users, and implementing focus trapping to prevent keyboard navigation from escaping the full-screen modal.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ui/components/recipes/cook-mode.tsx
Comment thread ui/components/recipes/cook-mode.tsx Outdated
Addresses review feedback on #657:
- Trap Tab/Shift+Tab inside the cook-mode dialog so keyboard focus
  cannot escape to the page covered by the overlay.
- Pad the step progress-bar buttons vertically so their touch targets
  are comfortably tappable while the visual bar stays thin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K
Multiple running timers previously rendered as a stack of separate
chips that could cover the cook-mode step content, with no way to move
them, and pausing widened a chip (the "· paused" suffix).

- The dock is now one compact pill by default, showing the most urgent
  timer (completed first, then soonest-ending) plus a "+N" badge;
  tapping expands it into a panel with per-timer controls.
- A grip handle makes the dock draggable via pointer events (touch and
  mouse); the position is clamped to the viewport and persisted to
  localStorage. Until dragged, the default bottom-right anchor and the
  cook-mode footer offset still apply.
- Paused state is now shown by the resume icon and a dimmed countdown
  instead of a text suffix, so the width no longer jumps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K
Expanding the dock (or adding timers while expanded) could grow it past
the screen edges when it had been dragged near one — leaving the
collapse button and per-timer controls unreachable.

- Re-clamp the dragged position whenever the dock's size changes
  (expand/collapse, timer count) and on window resize/rotation.
- Cap the expanded timer list at 45vh with internal scrolling and cap
  the panel width to the viewport.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K
- Use a native <dialog> element (non-modal `open`, not showModal(), so
  the top layer doesn't paint over the floating timer dock) with UA
  styles neutralised, instead of role="dialog" on a div.
- Reduce cognitive complexity: extract trapFocus/isEditableTarget from
  the cook-mode keydown handler, and a shared unitLabelFor helper from
  formatAmount's nested unit-label logic.
- Extract nested ternaries into named helpers (progressSegmentColor,
  timerCircleBackground, dotColor as if-chains, amountText).
- Avoid negated conditions where an else exists; prefer ??= over
  guarded assignments; prefer globalThis over window; mark component
  props Readonly<>.

No behaviour change intended; the full browser verification suite
(26 desktop/mobile checks plus the dock clamping suite) still passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K
@Robbie-Palmer Robbie-Palmer temporarily deployed to production-site-ui July 3, 2026 16:44 — with GitHub Actions Inactive
@Robbie-Palmer Robbie-Palmer marked this pull request as ready for review July 3, 2026 18:04
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add cook mode overlay and persistent timers for recipe pages

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add full-screen cook mode with step navigation, ingredient reference, and URL-synced state.
• Introduce localStorage-backed global timers shared by inline steps, cook mode, and a floating
 dock.
• Extract wake lock/audio/ingredient formatting utilities and add unit tests for timer persistence.
Diagram

graph TD
  RC["RecipeContent"] --> CM["CookMode Overlay"] --> WL["wakeLock"]
  RC --> IT["InlineTimer Pill"] --> TS["timerStore"] --> LS[("localStorage")]
  CM --> TS --> AT["alertTone"]
  TD["TimerDock"] --> TS
  TS --> WL
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a small state library (e.g., Zustand) for timers
  • ➕ Less custom subscription/emit logic; devtools and patterns are familiar
  • ➕ Easier selector-based subscriptions to avoid full-list scans (useCookingTimer)
  • ➖ Adds a dependency and bundle weight for a single domain-specific store
  • ➖ Persistence/cross-tab logic still needs custom work
2. Cross-tab sync via BroadcastChannel instead of storage events
  • ➕ Immediate sync without relying on localStorage writes
  • ➕ Can carry richer event semantics (start/pause/extend) without reloading full state
  • ➖ Extra API surface and compatibility considerations
  • ➖ Still need localStorage (or similar) for reload persistence
3. Make cook mode a true modal dialog (showModal) and move dock into overlay
  • ➕ Native focus trapping/backdrop semantics reduce custom keyboard handling
  • ➕ Fewer z-index/stacking-context interactions with the underlying page
  • ➖ Conflicts with requirement that the dock stays visible above the overlay
  • ➖ Would still require careful layering/portals for the dock and header coverage

Recommendation: The chosen approach (module-level timer store + end-timestamp persistence + useSyncExternalStore) is a good fit: it avoids new dependencies, naturally supports reload persistence, and cleanly feeds three UI surfaces. The non-modal plus custom focus trap is justified to keep the timer dock painting above the overlay; alternatives like showModal would complicate that constraint. If cross-tab responsiveness becomes a pain point later, BroadcastChannel would be the most incremental improvement.

Files changed (12) +2039 / -390

Enhancement (8) +1603 / -226
layout.tsxMount floating TimerDock at the recipes layout root +6/-0

Mount floating TimerDock at the recipes layout root

• Adds the TimerDock to the recipes layout and intentionally places it outside the isolated stacking context so it can paint above the cook-mode overlay (which portals to <body>).

ui/app/recipes/layout.tsx

cook-mode.tsxImplement full-screen cook mode overlay with step navigation and timers +633/-0

Implement full-screen cook mode overlay with step navigation and timers

• Introduces a cook mode overlay rendering one step at a time with progress bar navigation, arrow keys, swipe gestures, and Escape/back exit. Adds ingredient reference (desktop side panel, mobile bottom sheet) and integrates global timers with a large circular control per step timer, while retaining a wake lock for the entire session.

ui/components/recipes/cook-mode.tsx

recipe-content.tsxAdd cook mode entry, URL sync, and stable timer ids for method steps +166/-226

Add cook mode entry, URL sync, and stable timer ids for method steps

• Adds a Start cooking CTA and portals the CookMode overlay to <body> for proper stacking above the site header. Builds a unified CookStep model (SDK tokens or plain text) and assigns stable timer ids so inline pills, cook mode controls, and the dock share the same timers; mirrors cook mode open/step into the URL via history push/replace and popstate.

ui/components/recipes/recipe-content.tsx

timer-dock.tsxAdd draggable floating timer dock for multiple concurrent timers +367/-0

Add draggable floating timer dock for multiple concurrent timers

• Implements a floating dock that summarizes the most urgent timer when collapsed and provides per-timer controls when expanded. Persists dock position to localStorage and re-clamps on resize/expansion to keep controls on-screen; links timers back to their recipe pages.

ui/components/recipes/timer-dock.tsx

use-cooking-timers.tsExpose timerStore via useSyncExternalStore hooks +25/-0

Expose timerStore via useSyncExternalStore hooks

• Adds useCookingTimers (snapshot subscription) and a convenience useCookingTimer lookup hook to consume the global timer store from UI components.

ui/hooks/use-cooking-timers.ts

alertTone.tsExtract shared timer completion tone with audio unlocking +43/-0

Extract shared timer completion tone with audio unlocking

• Adds a shared AudioContext helper to unlock audio on user gestures and play a completion tone later from timer ticks, handling browsers that block non-gesture audio.

ui/lib/cooking/alertTone.ts

timerStore.tsAdd localStorage-backed global cooking timer store with ticking + side effects +302/-0

Add localStorage-backed global cooking timer store with ticking + side effects

• Introduces a module-level timer store that persists running timers as absolute end timestamps, restores state on load, and syncs across tabs via the storage event. Drives periodic ticking while any timer runs, triggers alert tone on completion, and reference-counts wake lock while timers are running.

ui/lib/cooking/timerStore.ts

wakeLock.tsAdd reference-counted screen wake lock utility +61/-0

Add reference-counted screen wake lock utility

• Implements keyed retain/release semantics so multiple features can hold the wake lock concurrently. Re-acquires on visibility changes and cancels in-flight requests when no holders remain.

ui/lib/cooking/wakeLock.ts

Refactor (2) +248 / -164
inline-timer.tsxRefactor InlineTimer to use the global cooking timer store +37/-164

Refactor InlineTimer to use the global cooking timer store

• Removes component-local countdown logic, wake lock, and audio handling in favor of the shared timerStore. InlineTimer now starts/pauses/resumes/dismisses timers by id and mirrors countdown formatting from the store.

ui/components/recipes/inline-timer.tsx

ingredientDisplay.tsExtract ingredient display formatting for reuse in cook mode +211/-0

Extract ingredient display formatting for reuse in cook mode

• Moves scaling, unit conversion, and annotation resolution into a shared formatter module. Provides helpers for both ingredient list rendering and instruction-token ingredient formatting.

ui/lib/domain/recipe/ingredientDisplay.ts

Tests (1) +177 / -0
timer-store.test.tsAdd unit tests for timer store ticking and persistence semantics +177/-0

Add unit tests for timer store ticking and persistence semantics

• Adds Vitest coverage for start/countdown/complete, pause/resume, extend/restart, dismiss, id replacement, localStorage restore (including expiry while away), malformed storage handling, subscription notifications, and countdown formatting.

ui/tests/lib/cooking/timer-store.test.ts

Other (1) +11 / -0
recipe-theme.cssAdjust timer dock positioning while cook mode is open +11/-0

Adjust timer dock positioning while cook mode is open

• Adds body-scoped CSS rules to lift the floating timer dock above the cook-mode footer, with separate offsets for mobile and >=640px layouts.

ui/app/recipes/recipe-theme.css

@qodo-code-review

qodo-code-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 4 rules

Grey Divider


Action required

1. Wake lock not reacquired ✓ Resolved 🐞 Bug ☼ Reliability
Description
wakeLock.ts clears the stored sentinel on the WakeLockSentinel release event but does not
attempt to reacquire when there are still active holders. This can cause cook mode / running timers
to unexpectedly lose the wake lock mid-session without recovery unless the tab visibility changes.
Code

ui/lib/cooking/wakeLock.ts[R30-42]

+async function acquire(): Promise<void> {
+  if (!isWakeLockSupported() || sentinel !== null) return;
+  const requestEpoch = ++epoch;
+  try {
+    const lock = await navigator.wakeLock.request("screen");
+    if (requestEpoch !== epoch || holders.size === 0) {
+      lock.release().catch(() => {});
+      return;
+    }
+    sentinel = lock;
+    lock.addEventListener("release", () => {
+      if (sentinel === lock) sentinel = null;
+    });
Relevance

⭐⭐ Medium

Team has accepted multiple wake-lock robustness fixes, but no prior guidance on reacquiring after
sentinel release event.

PR-#373

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The module only nulls out sentinel on release; without a follow-up acquire() when holders
remain, the wake lock can remain dropped until a separate visibility change occurs.

ui/lib/cooking/wakeLock.ts[30-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
If the browser/OS releases the wake lock while the page remains visible, the module sets `sentinel = null` but never reacquires despite `holders.size > 0`.

## Issue Context
The current reacquisition path only runs on `visibilitychange` to `visible`, but wake locks can be released for other reasons while still visible.

## Fix Focus Areas
- ui/lib/cooking/wakeLock.ts[30-46]

### Suggested fix
- In the sentinel `release` event handler:
 - Set `sentinel = null` (as today).
 - If `holders.size > 0` and `document.visibilityState === "visible"`, call `acquire()` to restore the lock.
- Ensure this doesn’t fight `epoch` cancellation (may need to increment `epoch` before reacquiring or rely on the existing `acquire()` logic).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Dock off-screen restore ✓ Resolved 🐞 Bug ☼ Reliability
Description
TimerDock loads a persisted drag position but does not re-clamp it after loading, only clamping on
expand/timer-count changes or window resize. If the viewport changes between sessions, the dock can
render partially/fully off-screen until another event triggers clamping.
Code

ui/components/recipes/timer-dock.tsx[R100-137]

+  useEffect(() => {
+    setPosition(loadPosition());
+  }, []);
+
+  // A dragged position is stored as bottom-right offsets measured at the
+  // dock's size at drag time. Expanding, gaining timers, or rotating the
+  // screen changes the dock's size (or the viewport), which can push parts of
+  // it off-screen — re-clamp whenever that happens so the controls stay
+  // reachable.
+  const clampToViewport = useCallback(() => {
+    setPosition((current) => {
+      if (!current) return current;
+      const rect = dockRef.current?.getBoundingClientRect();
+      if (!rect) return current;
+      const next = {
+        right: clamp(
+          current.right,
+          EDGE_MARGIN,
+          globalThis.innerWidth - rect.width - EDGE_MARGIN,
+        ),
+        bottom: clamp(
+          current.bottom,
+          EDGE_MARGIN,
+          globalThis.innerHeight - rect.height - EDGE_MARGIN,
+        ),
+      };
+      if (next.right === current.right && next.bottom === current.bottom) {
+        return current;
+      }
+      savePosition(next);
+      return next;
+    });
+  }, []);
+
+  // biome-ignore lint/correctness/useExhaustiveDependencies: expanded and timers.length change the dock's size, which is what requires re-clamping
+  useLayoutEffect(() => {
+    clampToViewport();
+  }, [expanded, timers.length, clampToViewport]);
Relevance

⭐⭐ Medium

No historical review evidence found on clamping restored dock positions; new file/path not present
in repo history.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code sets position after mount, but the clamping layout effect does not depend on position,
so the restored value can be used without any immediate bounds correction.

ui/components/recipes/timer-dock.tsx[100-137]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TimerDock` restores `position` from localStorage, but the clamping effect doesn’t run when `position` changes, so a restored position may not be adjusted to the current viewport.

## Issue Context
`position` is set in a `useEffect`, while `useLayoutEffect` re-clamps only when `expanded` or `timers.length` changes.

## Fix Focus Areas
- ui/components/recipes/timer-dock.tsx[100-142]

### Suggested fix
- Trigger `clampToViewport()` after `position` is loaded/applied. Two straightforward options:
 1) Add `position` to the `useLayoutEffect` dependency list (the internal `setPosition` functional update already no-ops when unchanged, so it won’t loop).
 2) After `setPosition(loadPosition())`, schedule a clamp on the next frame (`requestAnimationFrame`) once the dock has a measurable rect.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. React key collisions ✓ Resolved 🐞 Bug ≡ Correctness
Description
RecipeContent keys method steps and per-token spans using values that are not guaranteed unique
(step text / token.value), so repeated steps or repeated ingredient/timer mentions can create
duplicate keys. Duplicate keys can make React reuse the wrong DOM nodes, causing incorrect
step/timer UI rendering and unstable updates.
Code

ui/components/recipes/recipe-content.tsx[R481-502]

+          {cookSteps.map((step) => (
+            <li
+              key={step.key}
+              className="border-b border-dashed border-[var(--line)] last:border-0"
+            >
+              <div className="flex gap-4 py-3">
+                <span
+                  aria-hidden="true"
+                  className="rt-step-num rt-display text-3xl text-[var(--terracotta)] leading-none min-w-[2ch]"
+                />
+                <div className="rt-body flex-1 leading-relaxed pt-1">
+                  {step.tokens
+                    ? step.tokens.map((token) => (
                        <span key={`${token.type}:${token.value}`}>
-                          {token.type === "timer" ? (
+                          {token.type === "timer" && token.timerId ? (
                            <InlineTimer
+                              timerId={token.timerId}
+                              recipeSlug={recipe.slug}
+                              recipeTitle={recipe.title}
                              durationSeconds={token.durationSeconds}
                              label={token.value}
                            />
Relevance

⭐ Low

Similar “React list-key collisions” fix was explicitly rejected in recipe-content.tsx review.

PR-#641

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The step keys are derived from step text/joined token values, and those keys are reused directly for
<li key={step.key}> in the method list; token spans are keyed only by token.type and
token.value, which can repeat within a step (e.g., the same ingredient mentioned twice).

ui/components/recipes/recipe-content.tsx[208-230]
ui/components/recipes/recipe-content.tsx[480-513]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RecipeContent` renders method steps and instruction tokens with keys derived from `step.key` (itself derived from step text / joined token values) and `token.type + token.value`. These keys can collide when content repeats, which can lead to React reusing the wrong elements.

## Issue Context
This affects both the method list and cook-mode step model because `cookSteps` builds `key` from non-unique values, and the method list uses those keys directly.

## Fix Focus Areas
- ui/components/recipes/recipe-content.tsx[208-230]
- ui/components/recipes/recipe-content.tsx[480-518]

### Suggested fix
- For `cookSteps`, generate a stable unique key that includes the step index (e.g. `${recipe.slug}:step:${stepIndex}`) rather than step text/joined token values.
- For the method list token spans, include the token index in the key (e.g. `${tokenIndex}:${token.type}:${token.value}`), mirroring the approach used in `CookMode`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread ui/components/recipes/timer-dock.tsx Outdated
Comment thread ui/lib/cooking/wakeLock.ts
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a full-screen cook mode to recipe pages and lifts timer state into a persistent global store, so countdowns survive client-side navigation, page reloads, and tab switches via absolute end-timestamps in localStorage.

  • Cook mode overlay (cook-mode.tsx): step-by-step view with ingredient reference panel, swipe/keyboard/progress-bar navigation, URL state mirrored via History API (?cook=1&step=N), and screen wake lock for the session.
  • Global timer store (timerStore.ts, useCookingTimers): module-level store consumed by useSyncExternalStore; one timer drives the inline pill, the large cook-mode control, and the floating dock simultaneously, with cross-tab sync via the storage event.
  • Floating timer dock (timer-dock.tsx): draggable, collapsible multi-timer panel mounted in the recipes layout outside the isolated stacking context; position persists to localStorage and re-clamps on viewport resize.

Confidence Score: 4/5

The core timer persistence and cook mode logic are well-structured and the test suite is solid; the issues found are non-blocking quality improvements.

The architecture is careful — absolute end-timestamps survive reloads correctly, the wake lock is properly reference-counted with epoch guards, and the History API integration handles both push and deep-link entry points. The findings are scoped to: an accessibility gap in the non-modal dialog pattern, a minor per-component re-render cost from the flat useCookingTimer subscription, a step key collision edge case for duplicate-text recipe steps, and a missing reset of globalListenersHooked that leaves storage/visibility event coverage incomplete in the test suite. None affect correctness at runtime for the typical user.

ui/lib/cooking/timerStore.ts (test reset), ui/components/recipes/cook-mode.tsx (screen-reader dialog pattern), ui/components/recipes/recipe-content.tsx (step key generation), ui/hooks/use-cooking-timers.ts (tick re-render frequency)

Important Files Changed

Filename Overview
ui/lib/cooking/timerStore.ts New global timer store using useSyncExternalStore pattern; persists timers as absolute end-timestamps in localStorage with cross-tab sync. Logic is sound but __resetTimerStoreForTests omits resetting globalListenersHooked, weakening test isolation for storage/visibility event paths.
ui/components/recipes/cook-mode.tsx New full-screen cook-mode overlay using a non-modal dialog open with manual focus trapping; intentional top-layer bypass for timer dock compatibility. aria-modal=true on a non-modal dialog may not restrict screen-reader virtual cursor, leaving background content reachable to AT users.
ui/components/recipes/recipe-content.tsx Cook mode integration and URL-state management added cleanly; formatting helpers extracted to ingredientDisplay.ts. Step key generation uses joined token values which can collide for duplicate-text steps.
ui/components/recipes/timer-dock.tsx New floating draggable timer dock; pointer-capture drag, viewport clamping on resize/expand, and localStorage position persistence are all correctly implemented.
ui/hooks/use-cooking-timers.ts Thin useSyncExternalStore wrappers over the timer store. useCookingTimer subscribes every InlineTimer to the full store, causing all timer pills to re-render on every 250 ms tick regardless of their own state.
ui/tests/lib/cooking/timer-store.test.ts Good coverage of countdown, pause/resume, extend, completion, and localStorage restore including expiry-while-away. Cross-tab (storage event) and visibilitychange paths are not tested because globalListenersHooked is not reset between tests.
ui/lib/cooking/wakeLock.ts Reference-counted wake lock with epoch-based in-flight cancellation and automatic re-acquisition on visibility change; implementation is correct.
ui/lib/cooking/alertTone.ts Shared Web Audio context that must be unlocked from a user gesture before the background tick can play the completion tone; correctly extracted from the old inline-timer.
ui/lib/domain/recipe/ingredientDisplay.ts Clean extraction of ingredient formatting helpers from recipe-content.tsx for reuse in cook mode; logic is unchanged from the original.
ui/components/recipes/inline-timer.tsx Simplified from self-contained local state to a thin view over the global timer store; all previous logic removed cleanly.
ui/app/recipes/layout.tsx TimerDock mounted outside the isolated .recipe-surface stacking context so it paints above the cook-mode overlay; minimal, correct change.
ui/app/recipes/recipe-theme.css CSS-only lift of the timer dock above the cook-mode footer when .rt-cook-mode-open is set on body; responsive breakpoints are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    U([User]) --> RC[RecipeContent]
    RC -->|openCookMode| HA[History API\npushState ?cook=1]
    HA --> CM[CookMode overlay\nportaled to body]
    CM -->|onExit| HB[history.back / replaceState]
    HB -->|popstate| RC

    RC --> IL[InlineTimer pill\nper step]
    CM --> CT[CookModeTimer\nlarge control]
    RL[RecipesLayout] --> TD[TimerDock\nfloating dock]

    IL --> TS[(timerStore\nmodule singleton)]
    CT --> TS
    TD --> TS

    TS -->|persist| LS[(localStorage\nendTimeMs)]
    LS -->|loadStored on hydrate| TS
    TS -->|storage event| TS

    TS --> WL[wakeLock.ts\nref-counted]
    TS --> AT[alertTone.ts\nshared AudioContext]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    U([User]) --> RC[RecipeContent]
    RC -->|openCookMode| HA[History API\npushState ?cook=1]
    HA --> CM[CookMode overlay\nportaled to body]
    CM -->|onExit| HB[history.back / replaceState]
    HB -->|popstate| RC

    RC --> IL[InlineTimer pill\nper step]
    CM --> CT[CookModeTimer\nlarge control]
    RL[RecipesLayout] --> TD[TimerDock\nfloating dock]

    IL --> TS[(timerStore\nmodule singleton)]
    CT --> TS
    TD --> TS

    TS -->|persist| LS[(localStorage\nendTimeMs)]
    LS -->|loadStored on hydrate| TS
    TS -->|storage event| TS

    TS --> WL[wakeLock.ts\nref-counted]
    TS --> AT[alertTone.ts\nshared AudioContext]
Loading
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
ui/components/recipes/cook-mode.tsx:300-312
**Non-modal `<dialog>` with `aria-modal` may not exclude background content for screen readers**

The dialog is opened with the `open` attribute (non-modal) rather than `showModal()` — an intentional choice to avoid the top-layer stacking issue. However, because `showModal()` is not used, the browser does not set `aria-modal` on the dialog's native role, and many screen readers will not restrict their virtual cursor to the dialog content even though `aria-modal="true"` is set in HTML. A user navigating with arrow keys or reading-mode shortcuts will be able to reach elements behind the overlay. The manual `trapFocus` only intercepts Tab/Shift+Tab, not screen-reader cursor movement. On VoiceOver/Safari and NVDA/Firefox this is a well-documented gap with this pattern. Adding `aria-hidden="true"` to the siblings of the portal target, or maintaining a live `inert` attribute on the background tree while the dialog is open, would close this gap.

### Issue 2 of 4
ui/hooks/use-cooking-timers.ts:6-8
**All `InlineTimer` instances re-render every 250 ms tick**

`useCookingTimer(id)` calls `useCookingTimers()` which subscribes every caller to the shared store. The store emits on every tick (every 250 ms while any timer is running) and also on every action. A recipe with N timer tokens will therefore schedule N component re-renders 4× per second — even for timers that are idle. For a long recipe this is unnecessary work. A lightweight fix is to memoize the per-timer selector so individual `InlineTimer` instances only re-render when their own timer's state changes.

### Issue 3 of 4
ui/lib/cooking/timerStore.ts:292-302
**`__resetTimerStoreForTests` does not reset `globalListenersHooked`**

After the first test that calls `subscribeTimers`, `globalListenersHooked` is set to `true` and never cleared by `__resetTimerStoreForTests`. On subsequent test runs the `storage` and `visibilitychange` listeners are not re-hooked; tests that mutate `localStorage` to verify cross-tab behaviour or depend on the visibilitychange path will silently skip those code paths, making them weaker than they appear. Adding `globalListenersHooked = false;` to the reset function would restore full isolation.

```suggestion
/** Test-only: reset module state (optionally keeping localStorage). */
export function __resetTimerStoreForTests(): void {
  if (tickHandle !== null) {
    clearInterval(tickHandle);
    tickHandle = null;
  }
  releaseWakeLock(WAKE_LOCK_KEY);
  timers = EMPTY_TIMERS;
  hydrated = false;
  globalListenersHooked = false;
  listeners.clear();
}
```

### Issue 4 of 4
ui/components/recipes/recipe-content.tsx:212-214
**Step `key` can collide for two steps with identical token values**

The `key` for each `CookStep` is produced by joining every token's `.value` with `|`. Two recipe steps that happen to have the same text (e.g., two "Season to taste" steps) will produce the same key, causing a React duplicate-key warning and potential reconciliation artifacts. Using a stable positional key such as `String(stepIndex)` would be safer while still being stable across re-renders for the same recipe.

```suggestion
    if (instructionTokenization?.ok === true) {
      return instructionTokenization.steps.map((tokens, stepIndex) => ({
        key: String(stepIndex),
```

Reviews (1): Last reviewed commit: "Address SonarQube findings on the cook m..." | Re-trigger Greptile

Comment on lines +300 to +312
className="flex min-h-0 min-w-0 flex-1 flex-col"
onTouchStart={onTouchStart}
onTouchEnd={onTouchEnd}
>
<div className="flex-1 overflow-y-auto px-5 py-6 sm:px-10 sm:py-8">
<div className="rt-mono text-[var(--terracotta)]">
Step {String(clampedStep + 1).padStart(2, "0")} of {steps.length}
</div>
<p
aria-live="polite"
className="rt-display mt-3 max-w-3xl text-3xl leading-[1.08] text-[var(--ink)] sm:text-4xl lg:text-5xl"
>
{current.tokens

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Non-modal <dialog> with aria-modal may not exclude background content for screen readers

The dialog is opened with the open attribute (non-modal) rather than showModal() — an intentional choice to avoid the top-layer stacking issue. However, because showModal() is not used, the browser does not set aria-modal on the dialog's native role, and many screen readers will not restrict their virtual cursor to the dialog content even though aria-modal="true" is set in HTML. A user navigating with arrow keys or reading-mode shortcuts will be able to reach elements behind the overlay. The manual trapFocus only intercepts Tab/Shift+Tab, not screen-reader cursor movement. On VoiceOver/Safari and NVDA/Firefox this is a well-documented gap with this pattern. Adding aria-hidden="true" to the siblings of the portal target, or maintaining a live inert attribute on the background tree while the dialog is open, would close this gap.

Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/components/recipes/cook-mode.tsx
Line: 300-312

Comment:
**Non-modal `<dialog>` with `aria-modal` may not exclude background content for screen readers**

The dialog is opened with the `open` attribute (non-modal) rather than `showModal()` — an intentional choice to avoid the top-layer stacking issue. However, because `showModal()` is not used, the browser does not set `aria-modal` on the dialog's native role, and many screen readers will not restrict their virtual cursor to the dialog content even though `aria-modal="true"` is set in HTML. A user navigating with arrow keys or reading-mode shortcuts will be able to reach elements behind the overlay. The manual `trapFocus` only intercepts Tab/Shift+Tab, not screen-reader cursor movement. On VoiceOver/Safari and NVDA/Firefox this is a well-documented gap with this pattern. Adding `aria-hidden="true"` to the siblings of the portal target, or maintaining a live `inert` attribute on the background tree while the dialog is open, would close this gap.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d6593be — same approach as CodeRabbit's parallel note: background top-level content gets inert/aria-hidden while cook mode is mounted, with the timer dock portaled to <body> so it stays usable. Verified in a browser that the app root is inert and the dialog + dock are not.


Generated by Claude Code

Comment thread ui/hooks/use-cooking-timers.ts
Comment thread ui/lib/cooking/timerStore.ts
Comment on lines +212 to +214
if (instructionTokenization?.ok === true) {
return instructionTokenization.steps.map((tokens, stepIndex) => ({
key: tokens.map((token) => token.value).join("|"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Step key can collide for two steps with identical token values

The key for each CookStep is produced by joining every token's .value with |. Two recipe steps that happen to have the same text (e.g., two "Season to taste" steps) will produce the same key, causing a React duplicate-key warning and potential reconciliation artifacts. Using a stable positional key such as String(stepIndex) would be safer while still being stable across re-renders for the same recipe.

Suggested change
if (instructionTokenization?.ok === true) {
return instructionTokenization.steps.map((tokens, stepIndex) => ({
key: tokens.map((token) => token.value).join("|"),
if (instructionTokenization?.ok === true) {
return instructionTokenization.steps.map((tokens, stepIndex) => ({
key: String(stepIndex),
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/components/recipes/recipe-content.tsx
Line: 212-214

Comment:
**Step `key` can collide for two steps with identical token values**

The `key` for each `CookStep` is produced by joining every token's `.value` with `|`. Two recipe steps that happen to have the same text (e.g., two "Season to taste" steps) will produce the same key, causing a React duplicate-key warning and potential reconciliation artifacts. Using a stable positional key such as `String(stepIndex)` would be safer while still being stable across re-renders for the same recipe.

```suggestion
    if (instructionTokenization?.ok === true) {
      return instructionTokenization.steps.map((tokens, stepIndex) => ({
        key: String(stepIndex),
```

How can I resolve this? If you propose a fix, please make it concise.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
ui/components/recipes/timer-dock.tsx (1)

308-315: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Consider encoding the dynamic slug in the Link href.

Based on learnings, dynamic path segments in Link hrefs should use encodeURIComponent as defense-in-depth, even when the value is expected to already be safe.

🛡️ Suggested diff
       <Link
-        href={`/recipes/${timer.recipeSlug}`}
+        href={`/recipes/${encodeURIComponent(timer.recipeSlug)}`}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/components/recipes/timer-dock.tsx` around lines 308 - 315, The Link href
for the recipe navigation is using a dynamic slug without encoding, so update
the href construction in timer-dock.tsx to pass timer.recipeSlug through
encodeURIComponent before interpolating it into the /recipes path. Keep the
change localized to the Link element so the dynamic route remains safe even if
timer.recipeSlug contains unexpected characters.

Source: Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ui/app/recipes/recipe-theme.css`:
- Around line 145-155: The cook-mode lift rule on rt-timer-dock is being
overridden by the inline bottom style persisted by TimerDock after a drag.
Update TimerDock so it does not apply the saved bottom inline offset while
body.rt-cook-mode-open is present, or otherwise make the cook-mode positioning
take precedence when cook mode is active. Use the TimerDock component and its
persisted drag-position logic as the place to gate the inline style, and keep
the existing .rt-timer-dock cook-mode stylesheet rule as the fallback
positioning.

In `@ui/components/recipes/cook-mode.tsx`:
- Around line 248-259: The non-modal Cook mode dialog in cook-mode.tsx leaves
the rest of the app reachable to assistive tech, so update the Cook mode
implementation to make background content inert while the dialog is mounted. In
the Cook mode component around the <dialog> render and its focus-trap logic,
either apply inert and aria-hidden to the underlying app/timer-dock content
during active cook mode, or refactor the floating timer dock approach so the
dialog can use showModal() safely. Make sure the change is tied to the Cook mode
mount/unmount lifecycle so background interactivity is restored correctly.
- Around line 471-477: The ingredient row key in the group items list can
collide when the same ingredient appears more than once, so update the key used
in the `group.items.map(...)` render inside `cook-mode.tsx` to include the item
index as well as the ingredient. Use the existing `item` mapping in the recipe
row rendering to build a stable unique key, so repeated ingredients do not
produce duplicate keys during reconciliation.

In `@ui/components/recipes/recipe-content.tsx`:
- Around line 213-226: The cook-step key generation in recipe-content.tsx can
collide when instruction text repeats because the tokenized path uses
tokens.map(...).join("|") and the non-tokenized path uses the raw step text.
Update the key logic in the recipe instruction mapping so each step gets a
stable unique identifier, using the existing recipe.slug plus the step index and
any token/index data in the relevant map callbacks for the cook-mode step list
and progress controls. Keep the key generation centralized in the step-mapping
code so repeated instructions no longer reuse the same key.

In `@ui/lib/domain/recipe/ingredientDisplay.ts`:
- Around line 39-43: The annotation lookup in ingredientDisplay.ts is
overwriting entries in the annotations map when duplicate ingredient rows share
the same slug but have different preparation/note values. Update the
annotation-building logic around the item.ingredient and normalizeSlug keys to
detect conflicting duplicates and mark them as ambiguous instead of replacing
the existing entry, and make resolveIngredientAnnotation ignore ambiguous keys
so annotations are not applied to the wrong row.

---

Nitpick comments:
In `@ui/components/recipes/timer-dock.tsx`:
- Around line 308-315: The Link href for the recipe navigation is using a
dynamic slug without encoding, so update the href construction in timer-dock.tsx
to pass timer.recipeSlug through encodeURIComponent before interpolating it into
the /recipes path. Keep the change localized to the Link element so the dynamic
route remains safe even if timer.recipeSlug contains unexpected characters.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 16a81cc1-132e-4f13-a96f-0ca0c62e1c98

📥 Commits

Reviewing files that changed from the base of the PR and between 9a7dd8d and 6b21181.

📒 Files selected for processing (12)
  • ui/app/recipes/layout.tsx
  • ui/app/recipes/recipe-theme.css
  • ui/components/recipes/cook-mode.tsx
  • ui/components/recipes/inline-timer.tsx
  • ui/components/recipes/recipe-content.tsx
  • ui/components/recipes/timer-dock.tsx
  • ui/hooks/use-cooking-timers.ts
  • ui/lib/cooking/alertTone.ts
  • ui/lib/cooking/timerStore.ts
  • ui/lib/cooking/wakeLock.ts
  • ui/lib/domain/recipe/ingredientDisplay.ts
  • ui/tests/lib/cooking/timer-store.test.ts

Comment thread ui/app/recipes/recipe-theme.css Outdated
Comment thread ui/components/recipes/cook-mode.tsx
Comment thread ui/components/recipes/cook-mode.tsx Outdated
Comment thread ui/components/recipes/recipe-content.tsx Outdated
Comment thread ui/components/recipes/recipe-content.tsx Outdated
Comment thread ui/lib/domain/recipe/ingredientDisplay.ts Outdated
User-requested: distinguish and navigate multiple timers.
- Timers now carry their originating step index + instruction text; the
  dock shows a "step N" tag (collapsed and expanded) plus the
  instruction snippet, and each dock timer deep-links to
  ?cook=1&step=N to jump straight to that instruction.

Review fixes (CodeRabbit / Greptile / Qodo):
- Accessibility: cook mode now marks all other top-level content
  `inert`/`aria-hidden` while open (the non-modal <dialog> alone didn't
  make the background inert for AT); the timer dock is portaled to
  <body> so it stays reachable. Exit button gets an aria-label for its
  icon-only mobile state.
- Dock cook-mode lift no longer relies on a stylesheet rule that an
  inline drag position defeated: the footer clearance is computed in the
  component and composes with any dragged position (raised only while
  cooking, not persisted). Removed the dead CSS rule.
- Dock re-clamps to the viewport right after restoring a persisted
  position, not just on resize/expand.
- Wake lock reacquires on the sentinel `release` event when holders
  remain and the tab is visible (not only on visibilitychange).
- Per-timer subscription: useCookingTimer selects a single timer so idle
  inline pills don't re-render on every tick.
- Unique React keys for steps, step tokens, and ingredient rows.
- Guard ambiguous ingredient annotations (same slug, different
  prep/note) instead of letting one row's note leak onto another.
- encodeURIComponent the recipe slug in dock links.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ6MMqQC1fRiqdJDdRqa4K
@sonarqubecloud

sonarqubecloud Bot commented Jul 3, 2026

Copy link
Copy Markdown

@Robbie-Palmer Robbie-Palmer merged commit c5ea7de into main Jul 3, 2026
13 of 14 checks passed
@Robbie-Palmer Robbie-Palmer deleted the claude/recipe-cook-mode-ackyw1 branch July 3, 2026 22:12
@Robbie-Palmer Robbie-Palmer temporarily deployed to preview-recipe-api July 3, 2026 22:12 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants