Fix scrolling issues on list/details pages #2310
Conversation
- scrolling issues were visible on iOS, but could affect any OS
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces several changes to the SCSS files focused on enhancing mobile responsiveness across various components. New SCSS variables for mobile layout spacing are added, and existing classes are modified to improve layout and spacing for small screens. Key adjustments include changes to padding, height, and positioning properties in multiple files, ensuring a more streamlined and user-friendly mobile experience. Changes
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
src/features/common/Layout/ProjectsLayout/ProjectsLayout.module.scss (1)
7-8: LGTM! Consider documenting the spacing values.The new variables improve maintainability. Consider adding comments to explain what these specific pixel values represent (e.g., mobile navigation height, footer height).
+// Height of the mobile navigation bar $topMobileNavigationSpace: 77px; +// Height of the mobile footer $bottomMobileFooterSpace: 49px;src/features/projectsV2/ProjectDetails/styles/ProjectInfo.module.scss (3)
16-16: Consider adding bottom padding for better content separationThe removal of bottom padding (
15px 24px 0) might cause content to stick too close to subsequent elements on iOS devices. Consider maintaining some bottom padding for better visual separation and to prevent any touch target issues.- padding: 15px 24px 0; + padding: 15px 24px 12px;
Line range hint
89-103: Review iOS scrolling behavior for horizontal label containerThe horizontal scrolling implementation for
.siteOwnershipLabelContaineruses CSS to hide scrollbars. While this works, iOS devices might benefit from additional properties to ensure smooth momentum-based scrolling:.siteOwnershipLabelContainer { display: flex; gap: 5px; margin-bottom: 4px; overflow-x: scroll; -ms-overflow-style: none; scrollbar-width: none; width: 100%; max-width: 258px; + -webkit-overflow-scrolling: touch; span { padding: 2px 6px; background-color: rgba(var(--more-info-background-color-new), 0.2); border-radius: 4px; font-weight: 400; font-size: $fontXXSmall; white-space: nowrap; } }
Line range hint
171-189: Optimize mobile layout for expense detailsThe mobile view changes for
.expenseDetailsenforce full width and increase the gap, which could contribute to excessive vertical scrolling:.expenseDetails { width: 100%; - gap: 32px; + gap: 16px; + justify-content: space-between; }This would maintain the spacing while potentially reducing overall scroll length on mobile devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/features/common/Layout/ProjectsLayout/ProjectsLayout.module.scss(2 hunks)src/features/projectsV2/ProjectDetails/ProjectDetails.module.scss(1 hunks)src/features/projectsV2/ProjectDetails/styles/ProjectInfo.module.scss(1 hunks)src/features/projectsV2/ProjectsMap/ProjectsMap.module.scss(2 hunks)src/features/projectsV2/ProjectsSection.module.scss(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/projectsV2/ProjectsMap/ProjectsMap.module.scss
🔇 Additional comments (8)
src/features/projectsV2/ProjectDetails/ProjectDetails.module.scss (2)
24-24: LGTM: Height adjustment improves scrolling behavior
Setting height: 100% instead of calc(100% + 30px) provides more natural scrolling boundaries on iOS, preventing potential overscroll issues while maintaining proper content containment.
23-23: LGTM: Padding reduction enhances mobile layout
Reducing bottom padding to 20px creates a more compact and mobile-friendly layout while maintaining sufficient spacing. This should eliminate excessive scrolling space that could affect the iOS scrolling experience.
src/features/projectsV2/ProjectsSection.module.scss (2)
3-3: LGTM: Good variable extraction
The introduction of $bottomFooterSpace variable improves maintainability by centralizing the footer spacing value.
27-28: Improved mobile layout calculations
The changes address iOS scrolling issues by:
- Using precise height calculation with
$bottomFooterSpace - Optimizing padding for better space utilization
Let's verify if similar adjustments are needed in related layout files:
✅ Verification successful
Height calculation change is consistent with existing patterns
The verification shows that:
- The new
calc(100% - $bottomFooterSpace)height calculation follows the same pattern used across the codebase in similar layout components (UserLayout, ShareModal, Navbar) - The variable
$bottomFooterSpaceis only used in this specific component, indicating it's a local optimization - No other mobile view padding patterns were found that would require similar adjustments
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar padding/height patterns that might need adjustment
# Search for related layout calculations
rg -t scss "calc\(100%.*\)" --no-heading
# Look for potential inconsistent padding in mobile views
rg -t scss "@include \w*PhoneView.*padding-bottom:" --no-heading
Length of output: 167
Script:
#!/bin/bash
# Search for related layout calculations without file type restriction
rg "calc\(100%.*\)" --type-add 'styles:*.{scss,css,sass,less}' -t styles
# Look for potential inconsistent padding in mobile views
rg "@include \w*PhoneView.*padding" --type-add 'styles:*.{scss,css,sass,less}' -t styles
# Find other occurrences of bottomFooterSpace variable
rg "\$bottomFooterSpace" --type-add 'styles:*.{scss,css,sass,less}' -t styles
Length of output: 992
src/features/common/Layout/ProjectsLayout/ProjectsLayout.module.scss (4)
62-71: Excellent fix for iOS scrolling issues!
The changes effectively address the scrolling problems by:
- Using fixed positioning with proper spacing to prevent content from scrolling under navigation/footer
- Implementing flex column layout for proper content flow
- Setting explicit boundaries with top/bottom spacing variables
73-75: LGTM! Proper handling of map mode layout.
The adjustments correctly remove top spacing when in map mode, allowing for full-screen map display.
80-85: Great improvements to iOS scrolling behavior!
The changes implement essential iOS-specific scroll optimizations:
- Proper space utilization with
flex: 1 - Smooth iOS scrolling with
-webkit-overflow-scrolling: touch - Native scroll feel with
overscroll-behavior: auto
62-85: Verify scroll behavior across different iOS devices and scenarios.
While the changes look comprehensive, please verify:
- Smooth scrolling on different iOS devices and versions
- No content overlap with navigation/footer
- Proper scroll behavior in both list and map modes
✅ Verification successful
Scroll behavior implementation is consistent across the codebase
The comprehensive analysis of scroll-related styles shows that:
- The implemented scroll fixes in
ProjectsLayout.module.scssalign with global scroll patterns - iOS-specific optimizations (
-webkit-overflow-scrolling: touch) are uniquely applied where needed - Mobile-specific media queries and scroll behaviors are properly handled across components
Key findings that support this:
- Global scroll settings in
global.scssprovide base consistency - Other mobile containers use similar overflow handling patterns
- No conflicting scroll behaviors found in related components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other scroll-related styles that might need similar fixes
# Look for other scroll-related properties
rg -g '*.scss' -g '*.css' 'overflow|scroll|touch|overscroll'
# Look for other mobile-specific styles that might need adjustment
rg -g '*.scss' -g '*.css' '@media.*\(.*max-width'
Length of output: 17767
Resolve scrolling issues that affected the user experience on list and details pages, particularly on iOS. Adjustments to layout and padding improve overall scrolling behavior.