Skip to content
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

New ListViewBase extensions - SmoothScrollIntoView #1499

Closed
Vijay-Nirmal opened this issue Sep 16, 2017 · 47 comments · Fixed by #3222
Closed

New ListViewBase extensions - SmoothScrollIntoView #1499

Vijay-Nirmal opened this issue Sep 16, 2017 · 47 comments · Fixed by #3222

Comments

@Vijay-Nirmal
Copy link
Contributor

Vijay-Nirmal commented Sep 16, 2017

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

smoothscrollintoviewsampleoutput

Properties

Properties Type Description
Index int Index of the item to be scrolled
ItemPosition Enum Specify the position of the Item after scrolling
DisableAnimation bool To disable the scrolling animation
ItemPosition Description
Default If visible then it will not scroll, if not then the item will be aligned to the nearest edge
Left Aligned left
Top Aligned top
Centre Aligned centre
Right Aligned right
Bottom Aligned bottom

Sample code - SmoothScrollingHelper

@IbraheemOsama
Copy link
Member

Seems interesting to me :) maybe we can also include delay in scroll so the developer can control the scroll speed. let see what the community members think

@michael-hawker
Copy link
Member

Based on discussions on this topic I've had, you probably want to make the ScrollIfVisibile a Boolean to separate it from where to scroll the object to. e.g. you may want to scroll the item to the center, but only if it's not already visible.

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Sep 19, 2017

@IbraheemOsama We should add 'Speed' (pixels per second) instead for 'Delay' or 'Duration'. So the duration of the animation will be (Math.Abs(previousOffset - newPosition)*1000)/speed. Also, we could have MaxDuration property to override calculated duration if it exceeds MaxDuration.

@michael-hawker I like the idea and it would be easy to do.

I am sorry guys. I can't create a PR for this because I don't think I have enough knowledge to do it. I have never created a helper or control other than for my own use.

@IbraheemOsama
Copy link
Member

IbraheemOsama commented Sep 20, 2017

@Vijay-Nirmal everyone had this moment of 'First PR' :) when I created a gitHub account I didn't have any previous PR :P
Submit a PR and we'll help and share the knowledge through reviewing the PR.

@nmetulev
Copy link
Contributor

@Vijay-Nirmal, please let us know if you would be able to submit a PR for this. Most of us would be more than happy to help :)

@Vijay-Nirmal
Copy link
Contributor Author

Thanks, but I don't think I have enough knowledge about UWP development. I am sorry I can't create a PR for this. But in the future, when I am well versed in UWP development, I will create a PR to create a different feature.

@thecodrr
Copy link
Contributor

thecodrr commented Oct 7, 2017

@nmetulev I can take a look at this as soon as possible, if you want? This will be an exciting adventure.

@nmetulev
Copy link
Contributor

nmetulev commented Oct 7, 2017

Feel free to take this one @theweavrs

@Odonno Odonno added the help wanted Issues identified as good community contribution opportunities label Oct 19, 2017
@nmetulev
Copy link
Contributor

@theweavrs, were you planing on implementing this one? If so, I'll set the milestone for 2.1

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Jan 4, 2018

@nmetulev I will do it.

When I build the solution it gives me errors and warnings. I didn't change any code.

devenv_2018-01-04_17-31-07

Here is the full error list

Error List pdf

@nmetulev
Copy link
Contributor

nmetulev commented Jan 4, 2018

That's great @Vijay-Nirmal.

This is a known issue with the new design project. We believe it's a VS/NuGet bug and those errors can be safely ignored.

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Jan 9, 2018

For some reason, I can't navigate to my helper page. Update: I fixed navigation part. Now, .bind file is causing an exception.

gif

Also in .csproj file, it shows <None> tag instead of <Content> tag.

githubdesktop_2018-01-09_11-23-27

@nmetulev Is this method should be part of Microsoft.Toolkit.Uwp.UI.Helpers or Microsoft.Toolkit.Uwp.UI.Extensions?

@nmetulev Is there is a way to manually create controls in Property panel and link it to .xaml.cs file?

because SmoothScrollIntoView takes 5 parameters (item: object, itemPlacement: ItemPlacement, disableAnimation: bool, scrollIfVisibile: bool, horizontalOffset: int, verticalOffset: int) I want to create corresponding controls for every parameter in Property panel.

@IbraheemOsama I use ChangeView method in ScrollViewer to change the position. So I can't control the speed. Also, using ScrollViewer.ChangeView will standardize the scrolling animation.

@michael-hawker
Copy link
Member

@Vijay-Nirmal since it's effectively an extension to ListViewBase, why not create a ListViewBase.Methods.cs class in the ListViewBase Extension directory?

Also, I didn't see scrollIfVisibile in the repo yet, is that on the todo list still?

As for the properties panel, they get added from looking at the .bind file. If you have a reference to the 'CurrentSample' then you could modify the descriptor manually after the XAML rendered event. @nmetulev do we expose that to the Sample Page? I couldn't find an example.

I'd see the sample maybe exposing the placement enum and the checkboxes and then just having a button (with the Shell.Current.RegisterNewCommand) which 'Scrolls Into View'?

@Vijay-Nirmal
Copy link
Contributor Author

@michael-hawker Good idea.

I didn't see scrollIfVisibile in the repo yet, is that on the todo list still?

No. I have finished the whole implementation except sample app part. It is on my local computer.

they get added from looking at the .bind file

But this is a method I don't have xaml .bind only c# .bind to show the syntax of the method. I have planned to remove the c# .bind because syntax can be easily seen on the documentation panel.

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Jan 9, 2018

@michael-hawker Hmm... It's not a good idea. If I want to create this helper method inside Extensions/ListViewBase then do I have to share the same sample page? If not, I will create a separate page (SmoothScrollIntoView.xaml) but the file name (ListViewBase.Methods.cs) and the sample page name (SmoothScrollIntoView) won't be same. This may cause some confusion.

@michael-hawker
Copy link
Member

@Vijay-Nirmal We don't really explicitly point to where the code lives via sample name, it's just been a convention. That's what the doc tab is supposed to be for. However, @nmetulev was planning to expand the samples so that each page could provide more than one sample, so that would be another solution because I would think code structure wise it would make sense if it's a ListViewBase method to stay with ListViewBase.

@nmetulev thoughts? (Also, when we do get to building the multiple samples per page, they should still be individually searchable for discoverability...?)

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Jan 9, 2018

@michael-hawker So the only option is to wait until next sample app update? or I will create a PR with a separate sample page and after update sample app update we can merge the two pages.

when we do get to building the multiple samples per page, they should still be individually searchable for discoverability?

Use folders like live tiles folder when we click on it, it will show all the pages containing in that folder.

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Jan 9, 2018

@michael-hawker Since this is an extension method I want to change LisViewBase class as static. It causes different problems like ListViewBaseExtensions can't be derived from a static ListViewBase class, I need using static Microsoft.Toolkit.Uwp.UI.Extensions.ListViewBase to access the enum inside the ListViewBase.Data.cs

devenv_2018-01-09_23-13-33

devenv_2018-01-09_23-13-44

devenv_2018-01-09_23-18-13

How can I resolve it?

@nmetulev
Copy link
Contributor

Makes sense for this to be in the ListViewBaseExtensions class. There is a naming collision between the extension class and the ListViewBase platform class (we are going to resolve that with #1561), I think that's what you are running into here.

#1438 is tracking multiple samples per feature. I'd recommend adding the sample on the existing page if possible and then we can separate them in a it's own sample once possible.

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Jan 12, 2018

Since I am creating Extension methods, I want a static class. So even though I use ListViewBaseExtensions class I want to convert it to a static class. Creating a static partial class is not a good idea because only Extension methods need a static class, properties and enums don't need a static class. So the only solution I can see is to create a different standalone (without partial) class for Extension methods.

I'd recommend adding the sample on the existing page if possible

Yes, it is possible but it will not a good idea because it will not explain the method properly.

@Vijay-Nirmal
Copy link
Contributor Author

@nmetulev What shall I do?

@nmetulev
Copy link
Contributor

Seeing how we will be changing the ListVIewBase class to ListViewBaseEx, it's a good opportunity to make it a static class. My recommendation here is to create a new static class in the ListViewBase folder called ListViewBaseEx and add your code there.

I can then go in an mark the old class obsolete and move the appropriate methods there to prepare them for 3.0.

What do you think?

@nmetulev
Copy link
Contributor

Actually, per this comment, we will not be using the Ex extension, so the name will go back to ListViewBaseExtensions per the naming convention.

Is there a reason why the class needs to be static for your extensions @Vijay-Nirmal ?

@Vijay-Nirmal
Copy link
Contributor Author

@nmetulev Because I am creating an extension method for ListViewBase just link LogicalTree for FrameworkElement.

@Vijay-Nirmal
Copy link
Contributor Author

@nmetulev Any ideas?

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Mar 3, 2018

@michael-hawker

I didn't see scrollIfVisibile in the repo yet, is that on the todo list still?

I have updated my SmoothScrollingHelper repository.


If I use ItemsStackPanel as ItemsPanel then it is not working properly. For more info: Vijay-Nirmal/SmoothScrollingHelper#1
gif2

I can't debug this. I have tested all the value at runtime using breakpoints. My code is working properly. I think it is a bug with ItemsStackPanel. I need help to fix this issue.

@Kyaa-dost
Copy link
Contributor

@Vijay-Nirmal What's the status on this?

@Vijay-Nirmal
Copy link
Contributor Author

@Kyaa-dost I will do it. Currently, I am working in ASP.Net Core and .Net Core Framework. I can't find my motivation to get back to UWP. But I promise my self that I will do this. ✌

@Kyaa-dost
Copy link
Contributor

@Vijay-Nirmal, not a problem at all. Thank you so much for responding 🙂

@Vijay-Nirmal
Copy link
Contributor Author

Can I use Visual Studio 2019? Or Visual Studio 2017 only.

Because I am getting the below issue in VS 2019.

Unable to locate the .NET Core SDK. Check that it is installed and that the version specified in global.json (if any) matches the installed version.

image

I don't have Visual Studio 2017 installed. So I can't test it.

@Kyaa-dost
Copy link
Contributor

@Vijay-Nirmal It has to do with the certain SDK dependency which VS 2019 doesn't install by default. See the issue and resolution provided by Michael.

#3132 (comment)

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Feb 21, 2020

I was actually thinking, you could make the current ListViewExtensions sample a longer list, you could add command buttons to call the new method and demonstrate it.

@michael-hawker Is there a way to create Properties Panel items from code (not from xaml.bind)? Because I need to get the inputs for my extension method.

Update: Currently, I am using hidden TextBlock to bind value and get it through c#. Using this workaround, except enum every other thing is working

image

Update: Found the reason for enum not showing in the Properties Panel. Problem is that the below line of code checks enum in Microsoft.Toolkit.Uwp.UI.Media not in Microsoft.Toolkit.Uwp.UI

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/ef6527227539362125a0617d9b9821b2c009e11e/Microsoft.Toolkit.Uwp.SampleApp/Models/Sample.cs#L696-L697

Current Sample Page with the workaround
image

@Vijay-Nirmal
Copy link
Contributor Author

@Kyaa-dost @michael-hawker Any update?

@Vijay-Nirmal
Copy link
Contributor Author

@michael-hawker Update?

@michael-hawker
Copy link
Member

@Vijay-Nirmal can you summarize what items are you waiting on from me at the moment? It sounds like you worked around most of your sample app issues?

If you're pretty close to completeness, can you open a Draft PR with the changes? It'll make it easier to review and point-out any assistance with any open questions you have as well.

Thanks!

@Vijay-Nirmal
Copy link
Contributor Author

@michael-hawker I have created Draft PR #3222. I have two questions

  1. I am using invisible TextBlock to create the properties in Properties Panel. Is it ok to do it?

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/9ad0b20029c9266d0f3d162520fe549a9a9243fe/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ListViewExtensions/ListViewExtensionsXaml.bind#L20-L21
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/9ad0b20029c9266d0f3d162520fe549a9a9243fe/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/ListViewExtensions/ListViewExtensionsPage.xaml.cs#L32-L33

  1. Not working properly when using ItemsStackPanel as ItemsPanel. See SmoothScrollingHelper#1 for more details. I don't think it is the problem of the virtualized panel because ItemsWrapGrid works properly.

ItemsStackPanel

@Kyaa-dost Kyaa-dost removed this from the future milestone Jun 19, 2020
@ghost ghost removed the help wanted Issues identified as good community contribution opportunities label Jun 19, 2020
@Kyaa-dost Kyaa-dost added this to the 7.0 milestone Jun 19, 2020
@ghost ghost added In-PR 🚀 and removed In-PR 🚀 labels Jun 21, 2020
@ghost ghost added the in progress 🚧 label Dec 9, 2020
@michael-hawker michael-hawker modified the milestones: 7.0, 7.1 Mar 2, 2021
@ghost ghost removed the in progress 🚧 label Mar 2, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants