SmoothScrollIntoView ListView Extension #3222

Vijay-Nirmal · 2020-04-05T06:42:38Z

Fixes #1499

SmoothScrollIntoView extensions allow scrolling the item into the view with animation.

PR Type

What kind of change does this PR introduce?

Feature
Documentation content changes
Sample app changes

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Pull Request has been submitted to the documentation repository instructions. Link: windows-toolkit/WindowsCommunityToolkit#314
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
If new feature, add to Feature List
Contains NO breaking changes

ghost · 2020-04-05T06:42:40Z

Thanks Vijay-Nirmal for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

vgromfeld

This is a great extension but it seems to be a lot of code.
Can you explain more the logic and why all of this is needed ?

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ListViewExtensions/ListViewExtensionsPage.xaml.cs

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/ItemPlacement.cs

vgromfeld · 2020-04-08T21:31:41Z

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/ItemPlacement.cs

+        /// <summary>
+        /// Aligned left
+        /// </summary>
+        Left,


ListView and GridView are usually scrolling only in one direction. What do you think of having only Start, Center End values in the enum ?

Yes, but GridView supports scrolling on both axes at the same time. So, in my opinion, we need to support it.

Would we want to make them independent so they can be combined if needed? Fine leaving this as is too, though we may want to name it ScrollItemPlacement as it's specific to the scroll helper vs. a general ItemPlacement (as it's in the UI namespace which could be broad).

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

vgromfeld · 2020-04-08T21:41:52Z

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

+        /// <returns>Note: Even though this return <see cref="Task"/>, it will not wait until the scrolling completes</returns>
+        public static async Task SmoothScrollIntoViewWithItem(this ListViewBase listViewBase, object item, ItemPlacement itemPlacement = ItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisibile = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0)
+        {
+            await SmoothScrollIntoViewWithIndex(listViewBase, listViewBase.Items.IndexOf(item), itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);


Is it working for grouped collections ?

It will work but header will hide the element if the item is aligned to the top. I need to think of a solution.

@Vijay-Nirmal do you think fixing that scenario is required before we add the feature?

@michael-hawker Your call.... I tried a little bit, I can't figure out a way. I need to look into it more.

vgromfeld · 2020-04-08T21:56:08Z

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

+        /// <returns>Note: Even though this return <see cref="Task"/>, it will not wait until the scrolling completes</returns>
+        public static async Task SmoothScrollIntoViewWithItem(this ListViewBase listViewBase, object item, ItemPlacement itemPlacement = ItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisibile = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0)
+        {
+            await SmoothScrollIntoViewWithIndex(listViewBase, listViewBase.Items.IndexOf(item), itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);


You can directly return the inner task here. You don't need to let the compiler generate the async/await state machine.

Really? I heard it a bad practice like call stack won't be proper etc...

@vgromfeld comment?

I can't find a solution :( it would be nice if I can see how ScrollViewer is working.

@vgromfeld you're just saying to make a direct return here? Can you confirm and maybe just make the code suggestion?

The suggestion is to remove the async/await keywords.:

public static Task SmoothScrollIntoViewWithItem(this ListViewBase listViewBase, object item, ItemPlacement itemPlacement = ItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisibile = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0) { return SmoothScrollIntoViewWithIndex(listViewBase, listViewBase.Items.IndexOf(item), itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset); }

It saves some size on the final binary because the compiler does not have the generate the state machine behind the async/await keywords.
It is not always recommended to do this because if the called method is throwing, you will not see all the methods in the exception stack. Here, it means that if SmoothScrollIntoViewWithIndex throws, its exception will not contains any information about the fact that it has been called by SmoothScrollIntoViewWithItem.

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

michael-hawker

@Vijay-Nirmal there appears to be a merge conflict now, and I can't seem to build it as-is with the latest VS I have. Can you update the PR to the latest master?

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

michael-hawker · 2020-05-28T07:51:12Z

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

+        /// <returns>Note: Even though this return <see cref="Task"/>, it will not wait until the scrolling completes</returns>
+        public static async Task SmoothScrollIntoViewWithItem(this ListViewBase listViewBase, object item, ItemPlacement itemPlacement = ItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisibile = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0)
+        {
+            await SmoothScrollIntoViewWithIndex(listViewBase, listViewBase.Items.IndexOf(item), itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);


@Vijay-Nirmal do you think fixing that scenario is required before we add the feature?

michael-hawker · 2020-05-28T07:51:21Z

Microsoft.Toolkit.Uwp.UI/Extensions/ListViewBase/SmoothScrollIntoView.cs

+        /// <returns>Note: Even though this return <see cref="Task"/>, it will not wait until the scrolling completes</returns>
+        public static async Task SmoothScrollIntoViewWithItem(this ListViewBase listViewBase, object item, ItemPlacement itemPlacement = ItemPlacement.Default, bool disableAnimation = false, bool scrollIfVisibile = true, int additionalHorizontalOffset = 0, int additionalVerticalOffset = 0)
+        {
+            await SmoothScrollIntoViewWithIndex(listViewBase, listViewBase.Items.IndexOf(item), itemPlacement, disableAnimation, scrollIfVisibile, additionalHorizontalOffset, additionalVerticalOffset);


@vgromfeld comment?

michael-hawker · 2020-05-28T07:52:00Z

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ListViewExtensions/ListViewExtensionsPage.xaml.cs

        {
-            var sampleListView = control.FindChildByName("SampleListView") as ListView;
+            sampleListView = control.FindChildByName("SampleListView") as ListView;


Wish I could have built and seen this, trying to think if there's a better way we can show this sample. It almost seems as if we want a static page vs. using the property panel...?

michael-hawker · 2020-05-29T00:25:39Z

@Vijay-Nirmal let's brainstorm with some folks about finding a solution to your remaining open issue and move this to the 7.0 milestone. In the meantime, can you update the PR to merge in from master and have it be current again? Thanks!

ghost · 2020-06-13T03:00:09Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 30 days of this comment.

Vijay-Nirmal · 2020-06-13T05:09:15Z

I will work on this today

ghost · 2020-06-21T07:08:23Z

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

Vijay-Nirmal · 2020-06-21T11:28:54Z

I have updated my branch with latest change from the master. Currently facing an issue in the sample app.

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ListViewExtensions/ListViewExtensionsPage.xaml.cs

Rosuavio · 2021-06-28T16:49:42Z

@Vijay-Nirmal @michael-hawker, I think this is almost over the line. Do we want split up the ItemPlacement directions?

michael-hawker · 2021-06-28T21:25:01Z

@RosarioPulella I'm good with either choice, think we should rename it ScrollItemPlacement though?

Also for the sample, I meant adding a XamlRoot like the ImageCachePage. Then you remove all those textboxes and things from the .bind file and just reference them directly in the .xaml.cs file outside the OnXamlRendered method.

Rosuavio · 2021-06-29T15:41:12Z

@RosarioPulella I'm good with either choice, think we should rename it ScrollItemPlacement though?

Renamed.

Also for the sample, I meant adding a XamlRoot like the ImageCachePage. Then you remove all those textboxes and things from the .bind file and just reference them directly in the .xaml.cs file outside the OnXamlRendered method.

Done.

michael-hawker · 2021-07-01T20:36:30Z

Looks great! Few small comments:

Let's put "Smooth Scroll Settings" header above the properties
We should have the index be non-zero by default so that clicking the button does something first try
Put a right-margin on the settings so they align with the button bar below

michael-hawker · 2021-07-01T20:37:09Z

@vgromfeld I know you looked at these before, did you want to take another quick look to review? Thanks!

As per CommunityToolkit#3222 (comment)

ghost · 2021-07-07T21:37:08Z

Hello @michael-hawker!

Because this pull request has the auto merge :zap: label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost assigned michael-hawker Apr 5, 2020

This was referenced Apr 5, 2020

Documentation for SmoothScrollIntoView ListView Extension MicrosoftDocs/WindowsCommunityToolkitDocs#314

Merged

New ListViewBase extensions - SmoothScrollIntoView #1499

Closed

Vijay-Nirmal added feature 💡 extensions ⚡ labels Apr 5, 2020

Vijay-Nirmal requested a review from michael-hawker April 5, 2020 07:37

vgromfeld reviewed Apr 8, 2020

View reviewed changes

michael-hawker added this to the 6.1 milestone May 18, 2020

michael-hawker requested changes May 28, 2020

View reviewed changes

ghost added the needs author feedback 📝 label May 28, 2020

michael-hawker modified the milestones: 6.1, 7.0 May 29, 2020

ghost added the no-recent-activity 📉 Open Issues that require attention label Jun 13, 2020

ghost removed the no-recent-activity 📉 Open Issues that require attention label Jun 13, 2020

ghost added needs attention 👋 and removed needs author feedback 📝 labels Jun 21, 2020

Vijay-Nirmal force-pushed the SmoothScrollIntoView branch from d5a7dad to 3884163 Compare June 21, 2020 07:20

Vijay-Nirmal closed this Jun 21, 2020

Vijay-Nirmal force-pushed the SmoothScrollIntoView branch from 3884163 to 4aca259 Compare June 21, 2020 09:06

Vijay-Nirmal reopened this Jun 21, 2020

Vijay-Nirmal commented Jul 11, 2020

View reviewed changes

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ListViewExtensions/ListViewExtensionsPage.xaml.cs Show resolved Hide resolved

michael-hawker assigned azchohfi Sep 1, 2020

Rosuavio self-assigned this Dec 16, 2020

Rosuavio force-pushed the SmoothScrollIntoView branch from e38721d to 3d8fa7c Compare June 28, 2021 16:31

Rosuavio force-pushed the SmoothScrollIntoView branch 2 times, most recently from 51bbb4b to e65181b Compare June 29, 2021 15:40

Vijay-Nirmal and others added 13 commits July 2, 2021 11:37

Added SmoothScrollIntoView ListViewBase Extension Method

5aa3a84

Use SampleCommand class dont change DataContext

54944a7

Fix issue when index is greater than Count and Big Fix for Sample Page

24ae708

Updated Default ItemPlacement value

af4d202

Fixes for Pipeline build failed

60fc7c5

Review changes

2531a00

Review Changes - Files Reorg

8e6ef0a

Make scrool controls part of the sample

2ebe4b2

Name SmoothScroll methods with Async

91367b8

Remove over sepcified naming

65af1d2

Use smooth scrool controls in static xaml file

05e250c

Raname ItemPlacement to ScrollItemPlacement

e46bdde

Sample fixes

c6212fc

As per CommunityToolkit#3222 (comment)

Rosuavio force-pushed the SmoothScrollIntoView branch from bfd26b1 to c6212fc Compare July 2, 2021 15:38

michael-hawker added the auto merge ⚡ label Jul 7, 2021

michael-hawker approved these changes Jul 7, 2021

View reviewed changes

michael-hawker assigned vgromfeld Jul 7, 2021

Merge branch 'main' into SmoothScrollIntoView

0687cbf

michael-hawker added the next preview ✈️ Label for marking what we want to include in the next preview release for developers to try. label Jul 15, 2021

michael-hawker merged commit 1abf8f8 into CommunityToolkit:main Jul 15, 2021

Rosuavio removed their assignment Sep 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SmoothScrollIntoView ListView Extension #3222

SmoothScrollIntoView ListView Extension #3222

Vijay-Nirmal commented Apr 5, 2020 •

edited

Loading

ghost commented Apr 5, 2020

vgromfeld left a comment •

edited

Loading

vgromfeld Apr 8, 2020

Vijay-Nirmal Apr 12, 2020

michael-hawker Jun 3, 2021

vgromfeld Apr 8, 2020

Vijay-Nirmal Apr 14, 2020

michael-hawker May 28, 2020

Vijay-Nirmal May 28, 2020

vgromfeld Apr 8, 2020

Vijay-Nirmal Apr 12, 2020

michael-hawker May 28, 2020

Vijay-Nirmal Aug 25, 2020

michael-hawker Sep 8, 2020

vgromfeld Sep 9, 2020 •

edited

Loading

michael-hawker left a comment

michael-hawker May 28, 2020

michael-hawker May 28, 2020

michael-hawker May 28, 2020

michael-hawker commented May 29, 2020

ghost commented Jun 13, 2020

Vijay-Nirmal commented Jun 13, 2020

ghost commented Jun 21, 2020

Vijay-Nirmal commented Jun 21, 2020

Rosuavio commented Jun 28, 2021

michael-hawker commented Jun 28, 2021

Rosuavio commented Jun 29, 2021

michael-hawker commented Jul 1, 2021

michael-hawker commented Jul 1, 2021

ghost commented Jul 7, 2021

SmoothScrollIntoView ListView Extension #3222

SmoothScrollIntoView ListView Extension #3222

Conversation

Vijay-Nirmal commented Apr 5, 2020 • edited Loading

Fixes #1499

PR Type

PR Checklist

ghost commented Apr 5, 2020

vgromfeld left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vgromfeld Sep 9, 2020 • edited Loading

Choose a reason for hiding this comment

michael-hawker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-hawker commented May 29, 2020

ghost commented Jun 13, 2020

Vijay-Nirmal commented Jun 13, 2020

ghost commented Jun 21, 2020

Vijay-Nirmal commented Jun 21, 2020

Rosuavio commented Jun 28, 2021

michael-hawker commented Jun 28, 2021

Rosuavio commented Jun 29, 2021

michael-hawker commented Jul 1, 2021

michael-hawker commented Jul 1, 2021

ghost commented Jul 7, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Vijay-Nirmal commented Apr 5, 2020 •

edited

Loading

vgromfeld left a comment •

edited

Loading

vgromfeld Sep 9, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.