Mockups: isolate calculator and therapy navigation concepts - #1362
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdded reusable calculator scoring and mockup experiences, three responsive therapy navigation prototypes, route wiring, shared mockup shells, Playwright discovery updates, import-boundary validation, sitemap entries, and viewport tests. ChangesCalculator mockups
Therapy navigation prototypes
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/calculator-mockups/search-page-mockup.tsx (1)
48-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
layoutis a required prop that is never read.It is declared in the props type and passed at Line 262 but never destructured or used, so it only adds friction for future callers.
♻️ Proposed cleanup
label: string; - layout: "scroll"; className?: string;label="Prompts" - layout="scroll" className="smart-search-prompt-row"🤖 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/calculator-mockups/search-page-mockup.tsx` around lines 48 - 61, Remove the unused required layout prop from the MockupSuggestionChips props type and update its call site to stop passing layout, while preserving the component’s existing behavior and other props.src/components/calculator-mockups/popup-sheet-mockup.tsx (1)
32-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider owning Escape + scroll lock inside
CalculatorSheet.Both consumers (
CalculatorsPopupSheetMockupLines 194-205 andsearch-page-mockup.tsxLines 591-602) duplicate the identical Escape listener anddocument.body.style.overflowlock. Co-locating them with the dialog keeps the modal contract in one place and avoids a future consumer shipping a non-dismissible sheet.🤖 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/calculator-mockups/popup-sheet-mockup.tsx` around lines 32 - 57, Move the Escape-key listener and document body scroll-lock lifecycle into CalculatorSheet, applying them while the sheet is mounted and restoring the prior overflow value on cleanup. Remove the duplicated Escape and overflow-handling effects from CalculatorsPopupSheetMockup and the search-page consumer, while preserving their existing close behavior through onClose.
🤖 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 `@src/components/calculator-mockups/search-page-mockup.tsx`:
- Around line 604-611: Update the comment above useMockupHideOnScroll to remove
the inaccurate claim that the hook polls the ref until the shell element
resolves. Keep the explanation that `#main-content` owns phone scrolling and is
the intended hook target; only describe behavior implemented by
useMockupHideOnScroll.
In `@tests/calculator-mockup-boundary.test.ts`:
- Around line 5-10: Update the calculator mockup boundary test around
searchMockups and forbiddenProductionFeatures to include all seven calculator
mockup files and the exact "`@/components/calculators`" module specifier alongside
the existing directory-path check. Ensure the assertions scan every mockup and
catch both direct and nested production imports.
---
Nitpick comments:
In `@src/components/calculator-mockups/popup-sheet-mockup.tsx`:
- Around line 32-57: Move the Escape-key listener and document body scroll-lock
lifecycle into CalculatorSheet, applying them while the sheet is mounted and
restoring the prior overflow value on cleanup. Remove the duplicated Escape and
overflow-handling effects from CalculatorsPopupSheetMockup and the search-page
consumer, while preserving their existing close behavior through onClose.
In `@src/components/calculator-mockups/search-page-mockup.tsx`:
- Around line 48-61: Remove the unused required layout prop from the
MockupSuggestionChips props type and update its call site to stop passing
layout, while preserving the component’s existing behavior and other props.
🪄 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: 2ab60e04-5e3c-45ce-8aad-92f8f68a1521
📒 Files selected for processing (30)
docs/site-map.mdplaywright.config.tssrc/app/mockups/calculators-bedside-sheet/page.tsxsrc/app/mockups/calculators-clinical-console/page.tsxsrc/app/mockups/calculators-directory-grid/page.tsxsrc/app/mockups/calculators-guided-flow/page.tsxsrc/app/mockups/calculators-popup-sheet/page.tsxsrc/app/mockups/calculators-search-page/page.tsxsrc/app/mockups/calculators-search/page.tsxsrc/app/mockups/mockups-layout-client.tsxsrc/app/mockups/therapy-navigation-context/page.tsxsrc/app/mockups/therapy-navigation-dock/page.tsxsrc/app/mockups/therapy-navigation-rail/page.tsxsrc/components/calculator-mockups/bedside-sheet-mockup.tsxsrc/components/calculator-mockups/calculator-fixtures.tssrc/components/calculator-mockups/calculator-pathways.tssrc/components/calculator-mockups/calculator-ui.tsxsrc/components/calculator-mockups/clinical-console-mockup.tsxsrc/components/calculator-mockups/directory-grid-mockup.tsxsrc/components/calculator-mockups/guided-flow-mockup.tsxsrc/components/calculator-mockups/index.tssrc/components/calculator-mockups/popup-sheet-mockup.tsxsrc/components/calculator-mockups/search-detail-mockup.tsxsrc/components/calculator-mockups/search-page-mockup.tsxsrc/components/therapy-navigation-mockups/context.tsxsrc/components/therapy-navigation-mockups/dock.tsxsrc/components/therapy-navigation-mockups/rail.tsxsrc/components/therapy-navigation-mockups/shared.tsxtests/calculator-mockup-boundary.test.tstests/ui-therapy-navigation-mockup.spec.ts
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #6373 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…sert Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
Sources reviewed
#1327 and mockup-only calculator work from #1331.
Retained
Reachable calculator and therapy-navigation mockups, mockup Playwright coverage, sitemap entries, and the production import-boundary test.
Rejected
Production calculator replacement/removal and any production navigation coupling.
Verification
npm run check:design-system-contract: passednpm run sitemap:check: passednpm run verify:cheap: passed; 413 test files, 4201 tests passed, 3 skippedaria-hiddenNot run
No hosted CI reruns and no physical-device testing.
Risk
This adds a large amount of design-scratch code, but it remains under mockup-only routes and is statically barred from production imports.
Summary by CodeRabbit