Skip to content

[Fabric] Implement snapToStart, snapToEnd, snapToOffsets property for ScrollView #14800

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Fabric] Implement snapToStart, snapToEnd, snapToOffsets property for ScrollView #14800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/react-native-windows-aa754bc8-86a7-4d0f-975d-ea69252b5b32.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Add e2e test cases for snapToStart property in ScrollView fabric implementation",
"packageName": "react-native-windows",
"email": "[email protected]",
"dependentChangeType": "patch"
}
3 changes: 3 additions & 0 deletions vnext/Microsoft.ReactNative/CompositionSwitcher.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ namespace Microsoft.ReactNative.Composition.Experimental
void SetMaximumZoomScale(Single maximumZoomScale);
void SetMinimumZoomScale(Single minimumZoomScale);
Boolean Horizontal;
void SnapToStart(Boolean snapToStart);
void SnapToEnd(Boolean snapToEnd);
void SnapToOffsets(Windows.Foundation.Collections.IVectorView<Single> offsets);
}

[webhosthidden]
Expand Down
131 changes: 131 additions & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionContextHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@

#include "pch.h"
#include "CompositionContextHelper.h"
#include <algorithm>
#if __has_include("Composition.Experimental.SystemCompositionContextHelper.g.cpp")
#include "Composition.Experimental.SystemCompositionContextHelper.g.cpp"
#endif
Expand Down Expand Up @@ -74,6 +75,10 @@ struct CompositionTypeTraits<WindowsTypeTag> {
winrt::Windows::UI::Composition::Interactions::InteractionTrackerRequestIgnoredArgs;
using InteractionTrackerValuesChangedArgs =
winrt::Windows::UI::Composition::Interactions::InteractionTrackerValuesChangedArgs;
using InteractionTrackerInertiaRestingValue =
winrt::Windows::UI::Composition::Interactions::InteractionTrackerInertiaRestingValue;
using InteractionTrackerInertiaModifier =
winrt::Windows::UI::Composition::Interactions::InteractionTrackerInertiaModifier;
using ScalarKeyFrameAnimation = winrt::Windows::UI::Composition::ScalarKeyFrameAnimation;
using ShapeVisual = winrt::Windows::UI::Composition::ShapeVisual;
using SpriteVisual = winrt::Windows::UI::Composition::SpriteVisual;
Expand Down Expand Up @@ -143,6 +148,10 @@ struct CompositionTypeTraits<MicrosoftTypeTag> {
winrt::Microsoft::UI::Composition::Interactions::InteractionTrackerRequestIgnoredArgs;
using InteractionTrackerValuesChangedArgs =
winrt::Microsoft::UI::Composition::Interactions::InteractionTrackerValuesChangedArgs;
using InteractionTrackerInertiaRestingValue =
winrt::Microsoft::UI::Composition::Interactions::InteractionTrackerInertiaRestingValue;
using InteractionTrackerInertiaModifier =
winrt::Microsoft::UI::Composition::Interactions::InteractionTrackerInertiaModifier;
using ScalarKeyFrameAnimation = winrt::Microsoft::UI::Composition::ScalarKeyFrameAnimation;
using ShapeVisual = winrt::Microsoft::UI::Composition::ShapeVisual;
using SpriteVisual = winrt::Microsoft::UI::Composition::SpriteVisual;
Expand Down Expand Up @@ -785,6 +794,7 @@ struct CompScrollerVisual : winrt::implements<
m_horizontal = value;

UpdateInteractionModes();
ConfigureSnapInertiaModifiers(); // Reconfigure modifiers when direction changes
}

void UpdateInteractionModes() noexcept {
Expand Down Expand Up @@ -855,6 +865,26 @@ struct CompScrollerVisual : winrt::implements<
m_interactionTracker.MinScale(minimumZoomScale);
}

void SnapToStart(bool snapToStart) noexcept {
m_snapToStart = snapToStart;
ConfigureSnapInertiaModifiers();
}

void SnapToEnd(bool snapToEnd) noexcept {
m_snapToEnd = snapToEnd;
ConfigureSnapInertiaModifiers();
}

void SnapToOffsets(winrt::Windows::Foundation::Collections::IVectorView<float> const &offsets) noexcept {
m_snapToOffsets.clear();
if (offsets) {
for (auto const &offset : offsets) {
m_snapToOffsets.push_back(offset);
}
}
ConfigureSnapInertiaModifiers();
}

void Opacity(float opacity) noexcept {
m_visual.Opacity(opacity);
}
Expand Down Expand Up @@ -1050,8 +1080,109 @@ struct CompScrollerVisual : winrt::implements<
0});
}

void ConfigureSnapInertiaModifiers() noexcept {
auto compositor = m_visual.Compositor();

// Collect all snap positions
std::vector<float> snapPositions;

if (!m_snapToOffsets.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You dont need this if. You can just always use this code. The current logic wouldn't work if m_snapToOffsets is empty, and m_snapToEnd is set. You wouldn't add the end snap point.

Better to just keep it simpler and have one code path here.

// When snapToOffsets is used, snapToStart/snapToEnd control whether to include start/end positions
if (m_snapToStart) {
snapPositions.push_back(0.0f);
}

// Add all the offset positions
for (const auto &offset : m_snapToOffsets) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is potentially more efficient:
snapPositions.insert(snapPositions.end(), m_snapToOffsets.begin(), m_snapToOffsets.end());

snapPositions.push_back(offset);
}

// When snapToOffsets is used, snapToEnd controls whether to include the end position
if (m_snapToEnd) {
const float maxPosition = m_horizontal ? std::max<float>(m_contentSize.x - m_visualSize.x, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually rather than doing that, which sounds expensive. -- Maybe 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.

: std::max<float>(m_contentSize.y - m_visualSize.y, 0);
if (maxPosition > 0) {
snapPositions.push_back(maxPosition);
}
}

// Sort snap positions to ensure proper ordering and remove duplicates
std::sort(snapPositions.begin(), snapPositions.end());
snapPositions.erase(std::unique(snapPositions.begin(), snapPositions.end()), snapPositions.end());
} else if (m_snapToStart) {
// Legacy behavior: just snap to start when no offsets are provided
snapPositions.push_back(0.0f);
}

if (!snapPositions.empty()) {
std::vector<typename TTypeRedirects::InteractionTrackerInertiaRestingValue> restingValues;

// Create a resting value for each snap position with proper conditions
for (size_t i = 0; i < snapPositions.size(); ++i) {
const auto position = snapPositions[i];
auto restingValue = TTypeRedirects::InteractionTrackerInertiaRestingValue::Create(compositor);

// Create condition that determines when to snap to this position
winrt::hstring condition;
if (snapPositions.size() == 1) {
// Single snap point - use simple distance condition
condition = winrt::hstring(L"Abs(this.Target.NaturalRestingPosition - ") + winrt::to_hstring(position) +
winrt::hstring(L") < 50");
} else {
// Multiple snap points - use range-based conditions
if (i == 0) {
// First snap point
const auto nextPosition = snapPositions[i + 1];
const auto midpoint = (position + nextPosition) / 2.0f;
condition = winrt::hstring(L"this.Target.NaturalRestingPosition < ") + winrt::to_hstring(midpoint);
} else if (i == snapPositions.size() - 1) {
// Last snap point
const auto prevPosition = snapPositions[i - 1];
const auto midpoint = (prevPosition + position) / 2.0f;
condition = winrt::hstring(L"this.Target.NaturalRestingPosition >= ") + winrt::to_hstring(midpoint);
} else {
// Middle snap point
const auto prevPosition = snapPositions[i - 1];
const auto nextPosition = snapPositions[i + 1];
const auto prevMidpoint = (prevPosition + position) / 2.0f;
const auto nextMidpoint = (position + nextPosition) / 2.0f;
condition = winrt::hstring(L"this.Target.NaturalRestingPosition >= ") + winrt::to_hstring(prevMidpoint) +
winrt::hstring(L" && this.Target.NaturalRestingPosition < ") + winrt::to_hstring(nextMidpoint);
}
}

restingValue.Condition(compositor.CreateExpressionAnimation(condition));
restingValue.RestingValue(compositor.CreateExpressionAnimation(winrt::to_hstring(position)));

restingValues.push_back(restingValue);
}

// Configure the appropriate axis based on scroll direction
std::vector<typename TTypeRedirects::InteractionTrackerInertiaModifier> modifiers;
modifiers.reserve(restingValues.size());
for (auto &&v : restingValues) {
modifiers.push_back(v);
}
if (m_horizontal) {
m_interactionTracker.ConfigurePositionXInertiaModifiers(winrt::single_threaded_vector(std::move(modifiers)));
} else {
m_interactionTracker.ConfigurePositionYInertiaModifiers(winrt::single_threaded_vector(std::move(modifiers)));
}
} else {
// Clear inertia modifiers when no snapping is configured
if (m_horizontal) {
m_interactionTracker.ConfigurePositionXInertiaModifiers({});
} else {
m_interactionTracker.ConfigurePositionYInertiaModifiers({});
}
}
}

bool m_isScrollEnabled{true};
bool m_horizontal{false};
bool m_snapToStart{true};
bool m_snapToEnd{true};
std::vector<float> m_snapToOffsets;
bool m_inertia{false};
bool m_custom{false};
winrt::Windows::Foundation::Numerics::float3 m_targetPosition;
Expand Down
17 changes: 17 additions & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/ScrollViewComponentView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,23 @@ void ScrollViewComponentView::updateProps(
if (oldViewProps.zoomScale != newViewProps.zoomScale) {
m_scrollVisual.Scale({newViewProps.zoomScale, newViewProps.zoomScale, newViewProps.zoomScale});
}

if (!oldProps || oldViewProps.snapToStart != newViewProps.snapToStart) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause the snappoints to be calculated 3 times when using all three snappoint properties.
Instead we should set them all at once.
So on IScrollVisual have a single SetSnapPoints function that takes all 3 properties as arguments, and here check if any of the 3 properties changed, and if so call SetSnapPoints.

m_scrollVisual.SnapToStart(newViewProps.snapToStart);
}

if (!oldProps || oldViewProps.snapToOffsets != newViewProps.snapToOffsets) {
// Convert from std::vector<facebook::react::Float> to winrt::IVectorView<float>
const auto snapToOffsets = winrt::single_threaded_vector<float>();
for (const auto &offset : newViewProps.snapToOffsets) {
snapToOffsets.Append(static_cast<float>(offset));
}
m_scrollVisual.SnapToOffsets(snapToOffsets.GetView());
}

if (!oldProps || oldViewProps.snapToEnd != newViewProps.snapToEnd) {
m_scrollVisual.SnapToEnd(newViewProps.snapToEnd);
}
}

void ScrollViewComponentView::updateState(
Expand Down
Loading