-
-
Notifications
You must be signed in to change notification settings - Fork 7
Add removeClippedSubviews option to ResponsiveGrid #80
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
Add removeClippedSubviews option to ResponsiveGrid #80
Conversation
WalkthroughAdds a new optional prop removeClippedSubviews (default true) to ResponsiveGrid and forwards it to the internal ScrollView. Type definitions updated to include the prop. No other logic or rendering changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant RG as ResponsiveGrid
participant SV as ScrollView (native)
Dev->>RG: render <ResponsiveGrid removeClippedSubviews?=X>
Note right of RG: Defaults to true when omitted
RG->>SV: render <ScrollView removeClippedSubviews=X>
SV-->>RG: Mount with clipping behavior
RG-->>Dev: Rendered grid content
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
LGTM! |
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 (2)
src/responsive-grid/types.ts (1)
89-95: Clarify platform behavior and caveats; consider Android-only default.Doc is good, but removeClippedSubviews can cause issues with sticky headers/animations on iOS. If you adopt the Android-only default suggested in index.tsx, please align the JSDoc:
- /** - * When true, off-screen child views (whose overflow value is hidden) are automatically removed from the native view hierarchy. - * This can improve performance for long lists, especially on Android when rendering remote images. - * @default true - */ + /** + * When true, off-screen child views are removed from the native view hierarchy to reduce memory/overdraw. + * Most beneficial on Android with remote images; can conflict with sticky headers/animations. + * @default Android: true, iOS: false (override via prop) + */src/responsive-grid/index.tsx (1)
37-38: Default to Android to minimize iOS regressions; keep user override.Set the default to true on Android only; callers can still opt-in on iOS.
- removeClippedSubviews = true, + removeClippedSubviews = Platform.OS === 'android',Add the import (outside this hunk):
import { ScrollView, View, StyleSheet, Platform } from 'react-native';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/responsive-grid/index.tsx(2 hunks)src/responsive-grid/types.ts(1 hunks)
🔇 Additional comments (1)
src/responsive-grid/index.tsx (1)
243-244: LGTM: Prop correctly forwarded to ScrollView.Pass-through is correct and preserves consumer control.
Add removeClippedSubviews prop to ResponsiveGrid for Android performance
Problem
ResponsiveGrid scrolling becomes laggy on Android 15 when rendering lists with remote image URIs according to this issue reported here #77
Changes
removeClippedSubviews?: booleanto ResponsiveGridPropstruefor better performance out of the boxImpact
Summary by CodeRabbit