feat(settings): edge-to-edge mobile chrome for the settings sheet#958
Conversation
The fullscreen settings sheet had broken mobile chrome: the header lived inside the scroll region (so it scrolled away) with a floating close button and no status-bar coverage, letting content bleed up behind the notch; the bottom stopped short of the edge instead of running under the home indicator. - Header: reuse the app's `.edge-glass-header` + `.edge-glass-header-backdrop` scrim as a sticky, full-bleed glass header that covers the status-bar/notch band, holding the title + the (now single, integrated) close button. It hides on scroll-down and returns on scroll-up via `useScrollHideReporter` (phone-gated), so the top runs fully edge-to-edge while scrolling — the same behaviour as the app's search bar. Desktop reverts to a static in-panel header (`lg:` resets); the two-column nav layout is unchanged. - Bottom: drop the panel's mobile `pb-safe` (`max-lg:!pb-0`) and carry a safe-area-aware pad on the content wrapper so content fills edge-to-edge to the physical bottom under the home indicator. - Restructure the mobile scroll region full-bleed (scroll container loses its horizontal padding/max-width; a centred content wrapper insets the sections) so the header scrim spans the true width. - Lighten the repeated "Saved for later — not active yet" markers from a heavy bordered pill to a quiet muted caption (text/id/aria-describedby and per-row placement unchanged, so the inert-preferences contract holds). Preserves the settings test contract: dialog name "Account & app", the four section headings, the row test ids, a single "Close settings" button, Escape-to-close, the 390x820 fullscreen box, and no horizontal overflow — all verified in Chromium. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RiRMGogS7jxgngFRYEHjw3
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesSettings dialog updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant User
participant ScrollContainer
participant SettingsDialog
participant useScrollHideReporter
participant StickyMobileHeader
User->>ScrollContainer: scroll settings content
ScrollContainer->>SettingsDialog: send scroll metrics
SettingsDialog->>useScrollHideReporter: report scroll position and max offset
useScrollHideReporter-->>SettingsDialog: update headerHidden
SettingsDialog->>StickyMobileHeader: apply hidden or visible state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
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.
Actionable comments posted: 1
🤖 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/clinical-dashboard/settings-dialog.tsx`:
- Around line 322-334: Update the sticky header class list in the header element
to use Tailwind v4 important-modifier syntax, replacing lg:!bg-transparent and
lg:!px-0 with the corresponding trailing-! forms so the desktop transparent
background and zero horizontal padding overrides apply.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd954634-6014-43ee-b6dc-cc57bd21c0d9
📒 Files selected for processing (1)
src/components/clinical-dashboard/settings-dialog.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Summary
On device the fullscreen settings sheet (
src/components/clinical-dashboard/settings-dialog.tsx) had broken mobile chrome: the header lived inside the scroll region (so it scrolled away) with a floating close button and no status-bar coverage, so content bled up behind the notch/status bar; the bottom stopped short of the edge instead of running under the home indicator.This makes the settings sheet edge-to-edge on mobile, reusing the app's existing header primitive:
.edge-glass-header+.edge-glass-header-backdropscrim as a sticky, full-bleed glass header covering the status-bar/notch band, holding the title + a single integrated close button. It hides on scroll-down and returns on scroll-up viauseScrollHideReporter(phone-gated), so the top runs fully edge-to-edge while scrolling — the same behaviour as the app's search bar. Desktop reverts to a static in-panel header (lg:resets); the two-column nav layout is unchanged.pb-safe(max-lg:!pb-0) and carry a safe-area-aware pad on the content wrapper so content fills edge-to-edge to the physical bottom under the home indicator.100vw/negative-margin overflow).id,aria-describedby, and per-row placement are unchanged so the inert-preferences contract holds.No
globals.csschanges — it reuses existing classes (which already carry no-blur / reduced-transparency / forced-colors fallbacks).Verification
npm run typecheck— clean for allsrc/files.npm run lint+prettier --check— clean on the changed file.documentElementhorizontal overflow is 0.npm run verify:pr-local— Verification not run: this environment'snode_modulesis missing the@testing-library/@axe-coredev-dependencies, so the DOM/UI suites and the build's TS-of-tests pass cannot run here; substituted the checks above.npm run verify:ui— UI verification not run: the@testing-library/@axe-coredev-dependencies are absent from this environment, so the Chromium UI gate cannot run here; substituted the manual cross-theme, cross-viewport Playwright QA above and recommend running the formal gate in CI.Risk and rollout
lg:resets.Clinical Governance Preflight
This is a local, presentation-only settings UI change and does not alter ingestion, answer generation, search/ranking, source rendering, document access, privacy, or clinical output logic; every item below remains true and unchanged.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
aria-describedbywiring on the same 7 inert rows, sotests/settings-inert-preferences.dom.test.tsx(marker text, count = 7, aria) stays green.Generated by Claude Code
Summary by CodeRabbit