Skip to content

fix(frontend/mobile): bottom-sheet modal at <=480px + vw cap on .modal-confirm - #1497

Merged
cristim merged 2 commits into
mainfrom
fix/985-modal-bottom-sheet
Jul 23, 2026
Merged

cristim merged 2 commits into
mainfrom
fix/985-modal-bottom-sheet

Conversation

@cristim

@cristim cristim commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

  • .modal-confirm now caps at min(480px, calc(100vw - 2rem)) instead of a flat 480px, so it never overflows narrow viewports.
  • At <=480px, .modal / .modal-confirm-backdrop and .modal-content / .modal-confirm switch to a bottom-sheet layout (flush to the bottom edge, full width, top corners rounded).
  • .modal-confirm-close now has a 44x44px minimum tap target (related to mobile: touch targets below 44pt on .btn-small, .toggle-password, .modal-confirm-close #983).

Closes #985.

Test plan

  • npx jest src/__tests__/css.test.ts - added 3 assertions (vw cap, 44px tap target, bottom-sheet media query); confirmed they fail on pre-fix CSS and pass post-fix.
  • npm test (full suite) - no new failures; 8 pre-existing currency-formatting failures in approval-details.test.ts reproduce identically on unmodified main, unrelated to this change.

@cristim cristim added bug Something isn't working triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/this-quarter Within the quarter impact/many Affects most users labels Jul 22, 2026
@cristim

cristim commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 7 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0a9f27a-6ca0-4e9d-831f-84560b99b56b

📥 Commits

Reviewing files that changed from the base of the PR and between 70c9a7b and 1f33433.

📒 Files selected for processing (3)
  • frontend/src/__tests__/css.test.ts
  • frontend/src/styles/components.css
  • frontend/src/styles/responsive.css
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/985-modal-bottom-sheet

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

@cristim cristim added severity/medium Moderate harm effort/s Hours type/bug Defect and removed severity/low Minor harm labels Jul 22, 2026
#985)

The <=480px bottom-sheet block lived in components.css, which index.css
imports before modals.css and responsive.css. With equal selector
specificity, the later-imported responsive.css 768px rule
(.modal-content{width:95%}) always won, so only .modal-confirm actually
flipped to a bottom sheet; every real modal (#user-modal,
#purchase-modal, #account-modal, #plan-modal, #override-modal, login,
profile) stayed centered.

Move the block into responsive.css, placed after the existing 768px
block so source order lets it win. Strengthen the regression test to
check the rule lives in responsive.css (not components.css) and that
it appears after the 768px block, instead of only string-matching
components.css.
@cristim

cristim commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

Adversarial review finding (HIGH) and fix

An adversarial Opus review flagged that the new @media (max-width: 480px) bottom-sheet block in frontend/src/styles/components.css was silently overridden by the cascade:

  • frontend/src/styles/index.css imports components.css (3rd) before modals.css (6th) and responsive.css (last).
  • The base .modal / .modal-content rules live in modals.css, and responsive.css already has @media (max-width: 768px) { .modal-content { width: 95%; } }.
  • At equal selector specificity, later source order wins the cascade, so modals.css / responsive.css beat the bottom-sheet declarations for .modal / .modal-content at <=480px. Only .modal-confirm actually flipped to a bottom sheet; every real modal (#user-modal, #purchase-modal, #account-modal, #plan-modal, #override-modal, login, profile) stayed centered.
  • The original test only asserted the CSS string contained the media query, not that it won the cascade, so it passed despite the bug.

Fix: moved the @media (max-width: 480px) bottom-sheet block out of components.css and into responsive.css, placed after the existing 768px block so it wins by source order. Strengthened frontend/src/__tests__/css.test.ts to assert the rule lives in responsive.css (not components.css) and appears after the 768px block, instead of just string-matching. Verified the new test fails against the pre-fix layout and passes against the fix.

Cross-PR interaction with #1499: this PR's .modal-confirm-close rule (min-width/min-height: 44px) collides with the test.each list in #1499's frontend/src/__tests__/css.test.ts, which asserts that selector declares no sizing props. #1499 has been updated to drop .modal-confirm-close from that list, since this PR's explicit 44x44 is a legitimate, stronger guarantee than the generic coarse-pointer rule. Verified in a local merge simulation that both branches merge cleanly and the full css.test.ts suite (84 tests) is green after both land.

Gates run locally: npm run typecheck, npm run build, npx jest --no-coverage src/__tests__/css.test.ts — all green.

@cristim

cristim commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 1 minute.

@cristim

cristim commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 10 seconds.

@cristim
cristim merged commit d9eb097 into main Jul 23, 2026
21 checks passed
@cristim
cristim deleted the fix/985-modal-bottom-sheet branch July 27, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working effort/s Hours impact/many Affects most users priority/p3 Polish / idea / may never ship severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mobile: modals not full-screen on narrow viewports; .modal-confirm has no vw cap

1 participant