-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #1660 [Mobile Frontend Issue] : Section-Dialog Overflow In Mobile Device Along With Some Changes In Spacing And Font Size Of Sidebar #2114
Conversation
…nd adjust sidebar font and spacing styles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Headings size reduced for mobile device
- Added ScrollContainer
📝 WalkthroughWalkthroughThe pull request introduces a series of consistent styling and layout modifications across multiple sidebar components in the builder interface. The changes primarily focus on responsive design, adjusting font sizes and vertical spacing for better visual hierarchy and adaptability across different screen sizes. The modifications include reducing the default heading size from Changes
Possibly related PRs
Poem
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/client/src/pages/builder/sidebars/left/sections/shared/section-dialog.tsx (1)
152-152
: Consider additional mobile UX improvements.While the current implementation addresses the overflow issue, consider these enhancements for better mobile experience:
- The
space-y-6
spacing might be tight on mobile. Consider using responsive spacing likespace-y-4 lg:space-y-6
.- Add a visual indicator when content is scrollable (e.g., fade effect or scroll indicator).
- className=" max-h-[60vh] space-y-6 lg:max-h-fit" + className="max-h-[60vh] space-y-4 lg:space-y-6 lg:max-h-fit relative"apps/client/src/pages/builder/sidebars/right/sections/typography.tsx (1)
78-80
: Consider enhancing focus visibility for accessibilityThe responsive text sizing (text-xs/lg:text-sm) improves mobile usability while maintaining desktop readability. The transition and focus states are well implemented.
Consider adding a slightly more prominent focus indicator for better accessibility, such as a higher contrast ring color or larger ring-offset.
- "flex h-12 items-center justify-center overflow-hidden rounded border text-center text-xs ring-primary transition-colors hover:bg-secondary-accent focus:outline-none focus:ring-1 disabled:opacity-100 lg:text-sm", + "flex h-12 items-center justify-center overflow-hidden rounded border text-center text-xs ring-primary transition-colors hover:bg-secondary-accent focus:outline-none focus:ring-2 focus:ring-offset-2 disabled:opacity-100 lg:text-sm",apps/client/src/pages/builder/sidebars/right/sections/statistics.tsx (1)
23-23
: LGTM! Consider removing extra space in className.The responsive font sizing is consistent with other sections, but there's an extra space in the className string.
- <h2 className="line-clamp-1 text-2xl font-bold lg:text-3xl">{t`Statistics`}</h2> + <h2 className="line-clamp-1 text-2xl font-bold lg:text-3xl">{t`Statistics`}</h2>apps/client/src/pages/builder/sidebars/right/sections/page.tsx (1)
26-26
: LGTM! Consider removing extra spaces in className.The responsive font sizing is consistent with other sections, but there are extra spaces in the className string.
- <h2 className="line-clamp-1 text-2xl font-bold lg:text-3xl">{t`Page`}</h2> + <h2 className="line-clamp-1 text-2xl font-bold lg:text-3xl">{t`Page`}</h2>apps/client/src/pages/builder/sidebars/left/sections/basics.tsx (1)
30-30
: Consider maintaining consistent spacing across form fields.The full name field uses
space-y-4
while other fields usespace-y-1.5
. This inconsistency might affect the visual rhythm of the form.- <div className="space-y-4 sm:col-span-2"> + <div className="space-y-1.5 sm:col-span-2">apps/client/src/pages/builder/sidebars/right/sections/notes.tsx (1)
17-17
: LGTM! Minor formatting suggestion.The responsive heading size adjustment aligns well with the PR objectives and maintains consistency across sidebar sections.
Consider removing the extra space in
text-2xl font-bold
for better code formatting:- <h2 className="line-clamp-1 text-2xl font-bold lg:text-3xl">{t`Notes`}</h2> + <h2 className="line-clamp-1 text-2xl font-bold lg:text-3xl">{t`Notes`}</h2>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/client/src/pages/builder/sidebars/left/index.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/left/sections/basics.tsx
(2 hunks)apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx
(2 hunks)apps/client/src/pages/builder/sidebars/left/sections/shared/section-dialog.tsx
(2 hunks)apps/client/src/pages/builder/sidebars/left/sections/shared/section-options.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/left/sections/summary.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/export.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/information.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/layout.tsx
(2 hunks)apps/client/src/pages/builder/sidebars/right/sections/notes.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/page.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/sharing.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/statistics.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/template.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/theme.tsx
(1 hunks)apps/client/src/pages/builder/sidebars/right/sections/typography.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (5)
- apps/client/src/pages/builder/sidebars/right/sections/sharing.tsx
- apps/client/src/pages/builder/sidebars/right/sections/export.tsx
- apps/client/src/pages/builder/sidebars/left/sections/shared/section-options.tsx
- apps/client/src/pages/builder/sidebars/left/index.tsx
- apps/client/src/pages/builder/sidebars/right/sections/layout.tsx
🔇 Additional comments (14)
apps/client/src/pages/builder/sidebars/left/sections/shared/section-dialog.tsx (2)
23-23
: LGTM! ScrollArea import added correctly.The ScrollArea component import is properly added from the UI library, which will enable the scrolling functionality needed for mobile responsiveness.
150-154
: LGTM! ScrollArea implementation addresses the overflow issue.The ScrollArea wrapper with responsive height constraints (60vh for mobile, fit-content for desktop) effectively addresses the mobile overflow issue while maintaining desktop functionality.
apps/client/src/pages/builder/sidebars/right/sections/typography.tsx (2)
61-61
: LGTM: Consistent spacing adjustmentThe increased vertical gap improves the visual hierarchy and matches the spacing adjustments across other sidebar components.
65-65
: LGTM: Proper responsive typography implementationGood use of Tailwind's responsive design patterns to adjust heading size based on viewport. The text-2xl on mobile and lg:text-3xl on desktop provides better readability across devices.
apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (1)
23-23
: LGTM! Responsive font sizing improvement.The change from
text-3xl
totext-2xl lg:text-3xl
improves mobile responsiveness while maintaining desktop visual hierarchy.Let's verify consistent application across other section headers:
✅ Verification successful
Font sizing is consistently implemented across all section headers ✓
The responsive font sizing pattern (
text-2xl lg:text-3xl
) is uniformly applied across all 13 section headers in both left and right sidebars, maintaining a consistent heading hierarchy throughout the application.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent responsive font sizing across section headers # Expect: All section headers should use text-2xl lg:text-3xl pattern rg -U 'className="[^"]*h2[^"]*"' apps/client/src/pages/builder/sidebarsLength of output: 73
Script:
#!/bin/bash # First, let's verify the file structure fd . apps/client/src/pages/builder/sidebars # Then search for any heading elements and font size classes rg -U '<h[1-6].*class.*text-[2-3]xl' apps/client/src/pages/builder/sidebarsLength of output: 5163
apps/client/src/pages/builder/sidebars/right/sections/template.tsx (2)
19-19
: LGTM! Consistent responsive typography.The font size adjustment maintains consistency with other sidebar sections.
23-23
: LGTM! Improved template grid spacing.The increased gap and responsive grid layout enhance the visual spacing between templates, particularly beneficial for mobile devices.
apps/client/src/pages/builder/sidebars/right/sections/page.tsx (1)
30-30
: LGTM! Improved vertical spacing.The increased gap enhances readability and spacing between form elements.
apps/client/src/pages/builder/sidebars/left/sections/basics.tsx (1)
21-21
: LGTM! Responsive typography implementation.The mobile-first approach with
text-2xl lg:text-3xl
improves readability on smaller screens while maintaining the visual hierarchy on larger displays.apps/client/src/pages/builder/sidebars/right/sections/information.tsx (1)
117-117
: LGTM! Consistent responsive design implementation.The heading's font size adjustment matches the mobile-first approach implemented across other sections, maintaining a consistent visual hierarchy.
apps/client/src/pages/builder/sidebars/right/sections/theme.tsx (2)
20-20
: LGTM! Consistent typography implementation.The heading's responsive font size adjustment maintains consistency with other sections.
24-24
: LGTM! Improved spacing for better mobile interaction.The increased gap (
gap-y-6
) provides better touch target spacing and visual separation between theme controls, enhancing mobile usability.apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx (2)
103-103
: LGTM! Consistent responsive typography.The heading's font size implementation maintains consistency with other sections, following the mobile-first approach.
164-168
: LGTM! Enhanced button readability for mobile.The button's text size adjustment (
text-xs lg:text-sm
) improves readability on mobile devices while maintaining appropriate sizing on larger screens.
Thank you for this contribution! :) Will make sure it gets to production ASAP! |
@AmruthPillai Thanks for considering my contribution |
This Is The Fix For #1660
Issue : Form Prompt For Details Overflowing The Screen Without Scrollable Area With Fixed Height [Unresponsive In Mobile]
Fix : Added Max Height To The Section-Dialog.tsx For Mobile Device 60vh and Desktop Fit Content , Wrapping The Form Element With Scroll Area Allowing User To Scroll The Details Form When Exceeding The Height Of 60vh and Preventing The Overflow
Other Fix : Changes In Font Size For Mobile Devices And Some Layout Spacing Between The Section Of Left And Right Sidebar For All Devices
LEFT SIDEBAR:-
Before
rxresu.mp4
After
rxresu-1.mp4
RIGHT SIDEBAR:-
Before
rxresu-2.mp4
After
rxresu-3.mp4
Summary by CodeRabbit
Style
text-3xl
totext-2xl
for smaller screens while maintainingtext-3xl
for larger screensUI/UX Improvements