Phone Choose mode: new dense-glance YES mockups - #1650
Conversation
Replace the sectioned Find/Diagnose/Care study with two flat directions: dense title-only list (YES 01) and icon tiles (YES 02). Both keep modes visible at a glance without clinical-lane organisation.
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR replaces sectioned-list and icon-deck phone mode mockups with dense title-list and flat icon-tile compositions. It adds synchronized selection headers, keyboard navigation, preview states, selected indicators, updated phone-frame routing, and revised study documentation. ChangesPhone mode sheet compositions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DenseListSheet
participant DenseModeRow
participant CurrentlyLine
participant Preview
User->>DenseListSheet: Focus or select a mode
DenseListSheet->>DenseModeRow: Update selected row
DenseModeRow->>CurrentlyLine: Pass selected mode
CurrentlyLine->>Preview: Update current-mode header and preview
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/phone-mode-sheet-yes-mockups.tsx (1)
461-498: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the shared roving-focus logic.
DenseListSheetandFlatTileSheetduplicateoptionRefs,focusIndex,focusModeOption, andselectMode. Only the key-to-delta mapping differs. A small hook, for exampleuseRovingModeFocus(selected, onSelectedChange), would keep both sheets in sync when the navigation rules change.This is design scratch, so treat it as optional.
♻️ Sketch of a shared hook
functionuseRovingModeFocus(selected: AppModeId,onSelectedChange: (id: AppModeId)=>void){constoptionRefs=useRef<Array<HTMLButtonElement|null>>([]);const[focusIndex,setFocusIndex]=useState(Math.max(0,FLAT_MODE_IDS.indexOf(selected)));constfocusModeOption=(index: number)=>{constnext=Math.max(0,Math.min(FLAT_MODE_IDS.length-1,index));setFocusIndex(next);optionRefs.current[next]?.focus();};constselectMode=(id: AppModeId)=>{onSelectedChange(id);constnextIndex=FLAT_MODE_IDS.indexOf(id);if(nextIndex>=0)setFocusIndex(nextIndex);};return{ optionRefs, focusIndex, focusModeOption, selectMode };}🤖 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 `@src/components/phone-mode-sheet-yes-mockups.tsx` around lines 461 - 498, Optionally extract the duplicated roving-focus state and behavior from DenseListSheet and FlatTileSheet into a shared useRovingModeFocus hook. Centralize optionRefs, focusIndex, focusModeOption, and selectMode, while keeping each sheet’s distinct keyboard key-to-delta mapping in its own handler.
🤖 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.
Nitpick comments:
In `@src/components/phone-mode-sheet-yes-mockups.tsx`:
- Around line 461-498: Optionally extract the duplicated roving-focus state and
behavior from DenseListSheet and FlatTileSheet into a shared useRovingModeFocus
hook. Centralize optionRefs, focusIndex, focusModeOption, and selectMode, while
keeping each sheet’s distinct keyboard key-to-delta mapping in its own handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1bd437b9-596f-462d-bb9e-53b892f17721
📒 Files selected for processing (3)
mockups/README.mdsrc/app/mockups/phone-mode-sheet-yes/page.tsxsrc/components/phone-mode-sheet-yes-mockups.tsx
DenseListSheet and FlatTileSheet now share useRovingModeFocus so optionRefs/focusIndex/select stay in sync; each sheet keeps its own keyboard key map. Addresses CodeRabbit maintainability nit. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
commented
Aug 6, 2026
Addressed the CodeRabbit maintainability nit in |
Summary
min-h-12rows,Currently · {mode}in the sheet header, interactive selection that lifts into the top-bar pill, Rest / Scrolled / Switched proof frames (~12 modes visible on Rest)./mockups/phone-mode-sheet-yes); shared mockup chrome is suppressed.useRovingModeFocusso dense and tile sheets keep option refs / focus / select in sync (CodeRabbit maintainability nit).Clinical Governance Preflight
/mockups/*404 in production)Test plan
npm run ensure→/mockups/phone-mode-sheet-yesloadsnpx eslinton changed TSX — cleanRAG impact
RAG impact: no retrieval behaviour change — mockups-only design scratch; no RAG surfaces touched.
Summary by CodeRabbit
New Features
Documentation