Add onScroll prop #70
Conversation
WalkthroughThe changes introduce throttled scroll event handling to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AnimatedScrollView
participant ResponsiveGrid
participant useThrottle
participant Virtualization
User->>AnimatedScrollView: Scrolls
AnimatedScrollView->>ResponsiveGrid: onScroll event
ResponsiveGrid->>ResponsiveGrid: Call external onScroll prop (if provided)
ResponsiveGrid->>useThrottle: Throttle onScroll handling (32ms)
useThrottle-->>ResponsiveGrid: Throttled scroll event
ResponsiveGrid->>ResponsiveGrid: Update scroll position & check onEndReached
ResponsiveGrid->>Virtualization: Update visible items (if enabled)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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:
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
🧹 Nitpick comments (1)
package.json (1)
75-76: Constrain peerDependency version range for react-native-reanimated
Currently the peerDependency is"react-native-reanimated": "*", which could inadvertently allow incompatible future versions. It’s safer to pin this to the tested major/minor range. For example:- "react-native-reanimated": "*" + "react-native-reanimated": "^3.17.5"This adjustment helps prevent breaking changes when major updates of Reanimated are released.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.json(2 hunks)src/responsive-grid/index.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/responsive-grid/index.tsx
🔇 Additional comments (1)
package.json (1)
70-70: Verify addition of react-native-reanimated in devDependencies
The devDependencies block now includes"react-native-reanimated": "^3.17.5", which is required for building and type‐checking with Reanimated. Confirm that this version aligns with the one used in your example workspace and any local TS configurations.
|
@robgraeber One thing to talk about, the second commit introduces re-animated, it is a great idea, but the aim of this package was to aim for zero dependency since it's also focused on compatibility across mobile and web. I would also want to know if there's an avoidable need to add this dependency? Let me know your thoughts |
|
Sorry didn't to pull in the reanimated changes. I made another PR with just the onScroll prop: #71 Regarding the reanimated changes, using the default scroll view's onScroll has a one frame delay so if you move the scrollview up and down quickly there's a few pixels of lag between the header position and the scroll position. The native onScroll should be fine for most though. If you did want to add Reanimated, it would support web I believe but it'd be a breaking change as it'd add a peer dependency. I was planning to keep the reanimated stuff to my own fork. |
|
Great @robgraeber Thanks again for your contribution and the explanation! I really appreciate the idea of introducing Reanimated and the clarification around the scroll performance issue. You're right — Reanimated does offer smoother & native-driven animations That said, for now, I think it's best to keep the core package zero-dependency to maximize compatibility and keep it lightweight for both mobile and web users. However, your suggestion has definitely opened up a new roadmap forward. We can consider introducing a separate package — something like an animated version — that would live alongside the core package and offer enhanced features using Reanimated. This way we can support more advanced use cases like draggable items and smoother animated layouts without affecting users who prefer a minimal setup. We might structure that separate package or extract the enhancements in a modular way under the same context (e.g., shared types, core logic, etc.). Thanks again, and feel free to share any ideas you have around this direction! |
|
@robgraeber is that draggable fork of the grid available somewhere? |
|
@ahundt my branch is here: https://github.com/stimulating-ai/react-native-flexible-grid |
3 participants
Hi there,
I added a prop exposing the onScroll handler for the scroll view. This is useful for disappearing page headers that scroll along with the page. Why not a header component? In my case I'm adjusting the header opacity as it scrolls.
I also refactored the scrollEventThrottle so the onScroll prop is unthrottled, this is necessary for smooth animations.
Summary by CodeRabbit
New Features
Refactor