fix: increase card size for link surveys#4285
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
WalkthroughThe pull request introduces modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/ui/components/PreviewSurvey/index.tsx (1)
Line range hint
388-401: Consider consistent sizing between mobile and desktop views.While the desktop view now uses
max-w-lg, the mobile view (line 151) still usesmax-w-md. Consider whether the mobile view should also be updated for consistency in the survey display across different devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/app/s/[surveyId]/components/LinkSurveyWrapper.tsx(1 hunks)packages/ui/components/PreviewSurvey/index.tsx(1 hunks)
🔇 Additional comments (3)
apps/web/app/s/[surveyId]/components/LinkSurveyWrapper.tsx (2)
Line range hint 41-47: LGTM: Clean state management implementation.
The background loading state management is well-implemented using React hooks and follows best practices.
66-66: LGTM: Card size increase implemented correctly.
The width increase from md:max-w-md (28rem) to md:max-w-lg (32rem) perfectly matches the requirements from issue #3293. The change:
- Only affects medium and larger viewports
- Maintains responsiveness within the flex container
- Provides better visibility for link surveys
Let's verify that this is the only instance of card width that needed updating:
✅ Verification successful
Width class change is correctly implemented
The change from md:max-w-md to md:max-w-lg in LinkSurveyWrapper.tsx is appropriate. The search results show that other max-w-md occurrences in the codebase are used in different contexts (error components, sheets, preview components, billing tables) and don't require similar updates as they serve different UI purposes unrelated to the link survey card size requirements from issue #3293.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining max-w-md classes that might need updating
rg "max-w-md" --type tsx --type jsx
Length of output: 63
Script:
#!/bin/bash
# Search for any remaining max-w-md classes using ripgrep without file type filters
rg "max-w-md"
Length of output: 1341
packages/ui/components/PreviewSurvey/index.tsx (1)
388-388: LGTM! Card size increase implemented correctly.
The change from max-w-md to max-w-lg successfully implements the requested card size increase for link surveys in desktop view, addressing issue #3293.
Let's verify that this change maintains proper responsiveness across different screen sizes:
✅ Verification successful
Card size increase looks good and maintains responsiveness
The change from max-w-md to max-w-lg is safe and maintains proper responsiveness:
- The component has separate mobile and desktop views with different styling
- The change only affects the desktop preview mode for link surveys
- No conflicting responsive classes found in the component
- The width increase from 28rem to 32rem is moderate and won't cause layout issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for responsive classes that might conflict with max-w-lg
rg "max-w-\w+" --type tsx
Length of output: 55
Script:
#!/bin/bash
# Search for responsive classes that might conflict with max-w-lg, using correct file extension
rg "max-w-\w+" --type ts --type tsx --type jsx --type js
Length of output: 86
Script:
#!/bin/bash
# Search for responsive classes that might conflict with max-w-lg, without file type filtering
rg "max-w-\w+"
Length of output: 56558
Script:
#!/bin/bash
# Search for mobile-specific classes in PreviewSurvey component
rg -A 5 -B 5 "className=.*mobile.*|className=.*sm:|className=.*md:|className=.*lg:" packages/ui/components/PreviewSurvey/
Length of output: 123
What does this PR do?
Fixes #3293
How should this be tested?
Test card size for link surveys
Before
After
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
LinkSurveyWrapperandPreviewSurveycomponents with a wider display area in medium-sized viewports.Surveycomponent, refining its layout for better adaptability across different screen sizes.SurveyLoadingAnimationcomponent with the addition of a new prop for background loading state.