Skip to content

Changed SmoothScrollIntoView method to be truly asynchronous #4129

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@
</Grid>

<StackPanel Grid.Column="1" Margin="5,10,10,0" Width="200">
<TextBlock Text="Smooth Scroll Settings" FontSize="{StaticResource TextStyleLargeFontSize}" Margin="0,0,0,10"/>
<TextBlock Text="Smooth Scroll Settings" FontSize="{StaticResource TextStyleLargeFontSize}"/>
<TextBox x:Name="IndexInput"
Header="Index"
InputScope="Number"
Text="100" />
Text="100"
Margin="0,10,0,0" />
<ComboBox x:Name="ItemPlacementInput"
Header="Item Placement"
SelectedIndex="0">
SelectedIndex="0"
Margin="0,10,0,0" >
<x:String>Default</x:String>
<x:String>Left</x:String>
<x:String>Top</x:String>
Expand All @@ -52,18 +54,25 @@
</ComboBox>
<CheckBox x:Name="DisableAnimationInput"
Content="Disable Animation"
IsChecked="False" />
IsChecked="False"
Margin="0,10,0,0" />
<CheckBox x:Name="ScrollIfVisibileInput"
Content="Scroll If Visible"
IsChecked="True" />
<TextBox x:Name="AdditionalHorizontalOffsetInput"
Header="Horizontal Offset"
InputScope="Number"
Text="0" />
Text="0"
Margin="0,10,0,0" />
<TextBox x:Name="AdditionalVerticalOffsetInput"
Header="Vertical Offset"
InputScope="Number"
Text="0" />
Text="0"
Margin="0,10,0,0" />
<StackPanel Orientation="Horizontal" Margin="0,10,0,0">
<Ellipse x:Name="ScrollIndicator" Fill="Red" Width="15" Height="15"/>
<TextBlock x:Name="ScrollIndicatorTest" Text="Not Scrolling" Margin="10,0,0,0"/>
</StackPanel>
</StackPanel>
</Grid>
</Page>
</Page>
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
using System.Collections.ObjectModel;
using System.Windows.Input;
using Microsoft.Toolkit.Uwp.UI;
using Windows.UI;
using Windows.UI.Popups;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Media;

namespace Microsoft.Toolkit.Uwp.SampleApp.SamplePages
{
Expand Down Expand Up @@ -37,7 +39,7 @@ public void OnXamlRendered(FrameworkElement control)

private void Load()
{
SampleController.Current.RegisterNewCommand("Start Smooth Scroll", (sender, args) =>
SampleController.Current.RegisterNewCommand("Start Smooth Scroll", async (sender, args) =>
{
var index = int.TryParse(IndexInput.Text, out var i) ? i : 0;
var itemPlacement = ItemPlacementInput.SelectedItem switch
Expand All @@ -55,7 +57,9 @@ private void Load()
var scrollIfVisibile = ScrollIfVisibileInput.IsChecked ?? true;
var additionalHorizontalOffset = int.TryParse(AdditionalHorizontalOffsetInput.Text, out var ho) ? ho : 0;
var additionalVerticalOffset = int.TryParse(AdditionalVerticalOffsetInput.Text, out var vo) ? vo : 0;
sampleListView.SmoothScrollIntoViewWithIndexAsync(index, itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);
UpdateScrollIndicator(true);
await sampleListView.SmoothScrollIntoViewWithIndexAsync(index, itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);
UpdateScrollIndicator(false);
});

if (sampleListView != null)
Expand All @@ -64,6 +68,20 @@ private void Load()
}
}

private void UpdateScrollIndicator(bool isScrolling)
{
if (isScrolling)
{
ScrollIndicatorTest.Text = "Scrolling";
ScrollIndicator.Fill = new SolidColorBrush(Colors.Green);
}
else
{
ScrollIndicator.Fill = new SolidColorBrush(Colors.Red);
ScrollIndicatorTest.Text = "Not Scolling";
}
}

private ObservableCollection<string> GetOddEvenSource(int count)
{
var oddEvenSource = new ObservableCollection<string>();
Expand Down Expand Up @@ -103,4 +121,4 @@ private static async void OnExecuteSampleCommand(string item)
await new MessageDialog($"You clicked {item} via the 'ListViewExtensions.Command' binding", "Item Clicked").ShowAsync();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static partial class ListViewExtensions
/// <param name="scrollIfVisible">Set false to disable scrolling when the corresponding item is in view</param>
/// <param name="additionalHorizontalOffset">Adds additional horizontal offset</param>
/// <param name="additionalVerticalOffset">Adds additional vertical offset</param>
/// <returns>Note: Even though this return <see cref="Task"/>, it will not wait until the scrolling completes</returns>
/// <returns>Returns <see cref="Task"/> that completes after scrolling</returns>
public static async Task SmoothScrollIntoViewWithIndexAsync(this ListViewBase listViewBase, int index, ScrollItemPlacement itemPlacement = ScrollItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisible = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0)
{
if (index > (listViewBase.Items.Count - 1))
Expand Down Expand Up @@ -58,7 +58,7 @@ public static async Task SmoothScrollIntoViewWithIndexAsync(this ListViewBase li

var tcs = new TaskCompletionSource<object>();

void ViewChanged(object obj, ScrollViewerViewChangedEventArgs args) => tcs.TrySetResult(result: null);
void ViewChanged(object _, ScrollViewerViewChangedEventArgs __) => tcs.TrySetResult(result: default);

try
{
Expand All @@ -80,20 +80,7 @@ public static async Task SmoothScrollIntoViewWithIndexAsync(this ListViewBase li
// Scrolling back to previous position
if (isVirtualizing)
{
var tcs = new TaskCompletionSource<object>();

void ViewChanged(object obj, ScrollViewerViewChangedEventArgs args) => tcs.TrySetResult(result: null);

try
{
scrollViewer.ViewChanged += ViewChanged;
scrollViewer.ChangeView(previousXOffset, previousYOffset, zoomFactor: null, disableAnimation: true);
await tcs.Task;
}
finally
{
scrollViewer.ViewChanged -= ViewChanged;
}
await scrollViewer.ChangeViewAsync(previousXOffset, previousYOffset, zoomFactor: null, disableAnimation: true);
}

var listViewBaseWidth = listViewBase.ActualWidth;
Expand Down Expand Up @@ -185,7 +172,7 @@ public static async Task SmoothScrollIntoViewWithIndexAsync(this ListViewBase li
}
}

scrollViewer.ChangeView(finalXPosition, finalYPosition, zoomFactor: null, disableAnimation);
await scrollViewer.ChangeViewAsync(finalXPosition, finalYPosition, zoomFactor: null, disableAnimation);
}

/// <summary>
Expand All @@ -198,10 +185,68 @@ public static async Task SmoothScrollIntoViewWithIndexAsync(this ListViewBase li
/// <param name="scrollIfVisibile">Set true to disable scrolling when the corresponding item is in view</param>
/// <param name="additionalHorizontalOffset">Adds additional horizontal offset</param>
/// <param name="additionalVerticalOffset">Adds additional vertical offset</param>
/// <returns>Note: Even though this return <see cref="Task"/>, it will not wait until the scrolling completes</returns>
/// <returns>Returns <see cref="Task"/> that completes after scrolling</returns>
public static async Task SmoothScrollIntoViewWithItemAsync(this ListViewBase listViewBase, object item, ScrollItemPlacement itemPlacement = ScrollItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisibile = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0)
{
await SmoothScrollIntoViewWithIndexAsync(listViewBase, listViewBase.Items.IndexOf(item), itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);
}

/// <summary>
/// Changes the view of <see cref="ScrollViewer"/> asynchronous.
/// </summary>
/// <param name="scrollViewer">The scroll viewer.</param>
/// <param name="horizontalOffset">The horizontal offset.</param>
/// <param name="verticalOffset">The vertical offset.</param>
/// <param name="zoomFactor">The zoom factor.</param>
/// <param name="disableAnimation">if set to <c>true</c> disable animation.</param>
private static async Task ChangeViewAsync(this ScrollViewer scrollViewer, double? horizontalOffset, double? verticalOffset, float? zoomFactor, bool disableAnimation)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you guys think this method should be public? Will this extension method be useful for developers as well?

Copy link
Member

@michael-hawker michael-hawker Jul 27, 2021

Choose a reason for hiding this comment

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

Yeah, we could expose this. I guess the idea is if you have animation the original ChangeView can take longer to return? (Like if that's the case, we should probably call that out in the method doc comment so folks know when to use this over the built-in one.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess the idea is if you have animation the original ChangeView can take longer to return?

I am not sure what you mean exactly. If we have animation in the original ChangeView then it will act as fire and forget not as a synchronous call. There is no built-in way to asynchronously wait until the animation completes. ChangeViewAsync methods does that, it can asynchronously wait until the animation completes.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, in either case, this could be helpful. @JustinXinLiu would you use something like this?

Since this PR is good to go right now, we can always open a different one later, as we'd want to improve docs with an example maybe.

{
if (horizontalOffset > scrollViewer.ScrollableWidth)
{
horizontalOffset = scrollViewer.ScrollableWidth;
}
else if (horizontalOffset < 0)
{
horizontalOffset = 0;
}

if (verticalOffset > scrollViewer.ScrollableHeight)
{
verticalOffset = scrollViewer.ScrollableHeight;
}
else if (verticalOffset < 0)
{
verticalOffset = 0;
}

// MUST check this and return immediately, otherwise this async task will never complete because ViewChanged event won't get triggered
if (horizontalOffset == scrollViewer.HorizontalOffset && verticalOffset == scrollViewer.VerticalOffset)
{
return;
}

var tcs = new TaskCompletionSource<object>();

void ViewChanged(object _, ScrollViewerViewChangedEventArgs e)
{
if (e.IsIntermediate)
{
return;
}

tcs.TrySetResult(result: default);
}

try
{
scrollViewer.ViewChanged += ViewChanged;
scrollViewer.ChangeView(horizontalOffset, verticalOffset, zoomFactor, disableAnimation);
await tcs.Task;
}
finally
{
scrollViewer.ViewChanged -= ViewChanged;
}
}
}
}