-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Implement snapToInterval property for ScrollView in Fabric #14829
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
Changes from 6 commits
20c85f7
66d484b
987489b
1916f7a
5c63450
8ba9bcd
0424e18
4bbc0b9
34e16db
b2eed31
e6607dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "prerelease", | ||
| "comment": "Implement snapToInterval property for Fabric ScrollView", | ||
| "packageName": "react-native-windows", | ||
| "email": "[email protected]", | ||
| "dependentChangeType": "patch" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -807,12 +807,9 @@ void ScrollViewComponentView::updateProps( | |
| } | ||
|
|
||
| if (oldViewProps.snapToStart != newViewProps.snapToStart || oldViewProps.snapToEnd != newViewProps.snapToEnd || | ||
| oldViewProps.snapToOffsets != newViewProps.snapToOffsets) { | ||
| const auto snapToOffsets = winrt::single_threaded_vector<float>(); | ||
| for (const auto &offset : newViewProps.snapToOffsets) { | ||
| snapToOffsets.Append(static_cast<float>(offset)); | ||
| } | ||
| m_scrollVisual.SetSnapPoints(newViewProps.snapToStart, newViewProps.snapToEnd, snapToOffsets.GetView()); | ||
| oldViewProps.snapToOffsets != newViewProps.snapToOffsets || | ||
| oldViewProps.snapToInterval != newViewProps.snapToInterval) { | ||
| updateSnapPoints(); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -863,6 +860,9 @@ void ScrollViewComponentView::updateContentVisualSize() noexcept { | |
| m_verticalScrollbarComponent->ContentSize(contentSize); | ||
| m_horizontalScrollbarComponent->ContentSize(contentSize); | ||
| m_scrollVisual.ContentSize(contentSize); | ||
|
|
||
| // Update snap points if snapToInterval is being used, as content size affects the number of snap points | ||
| updateSnapPoints(); | ||
| } | ||
|
|
||
| void ScrollViewComponentView::prepareForRecycle() noexcept {} | ||
|
|
@@ -1435,4 +1435,39 @@ void ScrollViewComponentView::updateShowsVerticalScrollIndicator(bool value) noe | |
| void ScrollViewComponentView::updateDecelerationRate(float value) noexcept { | ||
| m_scrollVisual.SetDecelerationRate({value, value, value}); | ||
| } | ||
|
|
||
| void ScrollViewComponentView::updateSnapPoints() noexcept { | ||
| const auto &viewProps = *std::static_pointer_cast<const facebook::react::ScrollViewProps>(this->viewProps()); | ||
| const auto snapToOffsets = winrt::single_threaded_vector<float>(); | ||
|
|
||
| // snapToOffsets has priority over snapToInterval (matches React Native behavior) | ||
| if (viewProps.snapToOffsets.size() > 0) { | ||
| // Use explicit snapToOffsets | ||
| for (const auto &offset : viewProps.snapToOffsets) { | ||
| snapToOffsets.Append(static_cast<float>(offset)); | ||
| } | ||
| } else if (viewProps.snapToInterval > 0) { | ||
| // Generate snap points based on interval | ||
| // Calculate the content size to determine how many intervals to create | ||
| float contentLength = viewProps.horizontal | ||
| ? std::max(m_contentSize.width, m_layoutMetrics.frame.size.width) * m_layoutMetrics.pointScaleFactor | ||
| : std::max(m_contentSize.height, m_layoutMetrics.frame.size.height) * m_layoutMetrics.pointScaleFactor; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this snappoint is dependent on the size, you'll have to ensure that this function is rerun when the size changes.. we should do a special InteractionTrackerInertiaRestingValue for the end snap point, which uses some references to the size so we dont need to constantly recalculate all of this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're absolutely right about the performance concern. The current implementation recalculates all snap points on every content size change, which is inefficient. I'll implement the optimization using Working on this now. |
||
|
|
||
| float interval = static_cast<float>(viewProps.snapToInterval) * m_layoutMetrics.pointScaleFactor; | ||
|
|
||
| // Ensure we have a reasonable minimum interval to avoid infinite loops or excessive memory usage | ||
| if (interval >= 1.0f && contentLength > 0) { | ||
| // Generate offsets at each interval, but limit the number of snap points to avoid excessive memory usage | ||
| const int maxSnapPoints = 1000; // Reasonable limit | ||
| int snapPointCount = 0; | ||
|
|
||
| for (float offset = 0; offset <= contentLength && snapPointCount < maxSnapPoints; offset += interval) { | ||
| snapToOffsets.Append(offset); | ||
| snapPointCount++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| m_scrollVisual.SetSnapPoints(viewProps.snapToStart, viewProps.snapToEnd, snapToOffsets.GetView()); | ||
| } | ||
| } // namespace winrt::Microsoft::ReactNative::Composition::implementation | ||
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.
don't remove this
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.
Restored the original inline snap points logic as requested. The code now preserves the existing logic for basic snapToOffsets cases and only uses the new updateSnapPoints() method when snapToInterval is involved. Commit 0424e18.