fix: modal widget width mobile#2799
Conversation
WalkthroughBoth modal components update their container minimum width styling from full available width to a viewport-constrained value capped at 360px, improving responsive layout behavior on smaller screens. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
✅ All snapshot tests passed |
Playwright test results
Details
Flaky testschromium › e2e/earnPage.spec.ts › Protocols filters on Earn page › Should be able to filter by Aave protocol (Qase ID: 39) Skipped testschromium › e2e/earnPage.spec.ts › Chains filters on Earn page › Should be able to navigate to the earn page (Qase ID: 40) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/composite/WithdrawModal/WithdrawModal.tsx (1)
38-49: Consider centralizing the modal container sizing config.This style block is duplicated with
DepositModal; extracting it will reduce future drift when responsive sizing is adjusted again.Optional refactor sketch
+// shared helper (example): src/components/composite/EarnModal/buildModalContainerTheme.ts +export const buildEarnModalContainerTheme = (theme: Theme) => ({ + maxHeight: 'calc(100vh - 6rem)', + minWidth: 'min(100vw, 360px)', + maxWidth: 400, + borderRadius: `${theme.shape.cardBorderRadiusLarge}px`, + [theme.breakpoints.up('sm')]: { minWidth: 400 }, +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/composite/WithdrawModal/WithdrawModal.tsx` around lines 38 - 49, The modal container sizing object inside WithdrawModal's useMemo (the ctx -> theme -> container with maxHeight, minWidth, maxWidth, borderRadius and the [theme.breakpoints.up('sm')] override) is duplicated in DepositModal; extract this object into a single exported constant or helper (e.g., modalContainerStyle or getModalContainer(theme)) and import it into both WithdrawModal and DepositModal, replacing the inline container block in the ctx memo so both components reuse the same responsive sizing config and avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/composite/WithdrawModal/WithdrawModal.tsx`:
- Around line 38-49: The modal container sizing object inside WithdrawModal's
useMemo (the ctx -> theme -> container with maxHeight, minWidth, maxWidth,
borderRadius and the [theme.breakpoints.up('sm')] override) is duplicated in
DepositModal; extract this object into a single exported constant or helper
(e.g., modalContainerStyle or getModalContainer(theme)) and import it into both
WithdrawModal and DepositModal, replacing the inline container block in the ctx
memo so both components reuse the same responsive sizing config and avoid future
drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5bfcc25e-a4b6-452a-8ced-aeabbf2fdaca
📒 Files selected for processing (2)
src/components/composite/DepositModal/DepositModal.tsxsrc/components/composite/WithdrawModal/WithdrawModal.tsx
Verified at the bundle layer + with a synthetic CSS reproduction at iPhone SE viewport (375×667).
Code diff is live where expected:
Synthetic CSS reproduction at 375×667 — same minWidth rule each side, placed inside a 240px-wide parent to simulate the narrow-parent scenario the route-details view triggers:
Real-world visual on preview at iPhone 14 Pro Max (430×932): Quick deposit modal centered at ~360px wide, not squashed. [screenshot: quick-deposit-preview-430.png] ✅ Visually + bundle confirmed. Width math at iPhone 14 Pro Max (430) / iPhone SE (375): |
2 participants
Which Jira task belongs to this PR?
Closes https://linear.app/lifi-linear/issue/JUM-785/depositwithdraw-squashed-modal-widget-on-mobile
Testing steps
/earnor an/earn/[slug]pageWhy did I implement it this way?
Checklist before requesting a review
Summary by CodeRabbit