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

Deprecating PullToRefreshListView for controls available in next Windows release #1981

Merged
merged 16 commits into from
Apr 30, 2018

Conversation

kbrons
Copy link
Contributor

@kbrons kbrons commented Apr 17, 2018

Issue: #1946

PR Type

What kind of change does this PR introduce?

  • Feature
  • Build or CI related changes
  • Documentation content changes
  • Sample app changes

What is the current behavior?

See #1946

What is the new behavior?

The developer can enable the native controls when available (see pull-to-refresh)

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Sample in sample app has been added / updated (for bug fixes / features)
  • 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)
  • Contains NO breaking changes

Other information

For this implementation to work, the sample app had to be updated to the latest Insider's SDK (Build 17125).

@Odonno
Copy link
Contributor

Odonno commented Apr 17, 2018

@kbrons Can you rename the title of this PR so everyone can understand what this is about in a glimpse?

@kbrons
Copy link
Contributor Author

kbrons commented Apr 17, 2018

Sure thing, I forgot to change it when I created the PR. Thanks for letting me know!

@kbrons kbrons changed the title Feature/1946 Deprecating PullToRefreshListView for controls available in next Windows release Apr 17, 2018
@@ -23,7 +23,7 @@


<UwpMetaPackageVersion>5.4.1</UwpMetaPackageVersion>
<DefaultTargetPlatformVersion>16299</DefaultTargetPlatformVersion>
<DefaultTargetPlatformVersion>17125</DefaultTargetPlatformVersion>
Copy link
Member

Choose a reason for hiding this comment

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

We should probably wait until the final SDK release and do this once, eh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I had to change it because the app would crash when trying to use the native control otherwise. Maybe we could remove the code to enable the feature and leave a PR with it to merge when the final SDK is released, along with this update. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

@nmetulev thoughts?

@@ -33,6 +34,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Controls
[TemplatePart(Name = PartIndicatorTransform, Type = typeof(CompositeTransform))]
[TemplatePart(Name = PartDefaultIndicatorContent, Type = typeof(TextBlock))]
[TemplatePart(Name = PullAndReleaseIndicatorContent, Type = typeof(ContentPresenter))]
[Obsolete("The PullToRefreshListView will be removed in a future major release. Please use the RefreshContainer control available in the 1803 version of Windows")]
Copy link
Member

Choose a reason for hiding this comment

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

Is the sample already marked deprecated as well?

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 just noticed I updated most of the documentation, but forgot to add the badge for the menu and the disclaimer on top of the page in the sample app. I'll add them as soon as I can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added the badge and the deprecated message to the menu, also the deprecation warning on the page.

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

I haven't tested this yet, just few things I noticed.

I don't think appveyor is setup to build with an insider sdk, so we are going to have to wait until 1803 is released to merge this

private ControlTemplate _previousTemplateUsed;
private RefreshContainer _refreshContainer;

private bool UsingRefreshContainer => IsRefreshContainerSupported && UseRefreshContainerWhenPossible;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should start with _ and lowercase

@@ -45,7 +48,42 @@ be raised and the *RefreshIntentCanceledCommand*, if any, will be executed.

[PullToRefreshListView Sample Page](https://github.com/Microsoft/UWPCommunityToolkit/tree/master/Microsoft.Toolkit.Uwp.SampleApp/SamplePages/PullToRefreshListView)

## Default Template
## <a name="refreshcontainer"></a> Moving to RefreshContainer
The (soon to be released) 1803 version of Windows now includes its own implementation of [pull-to-refresh](https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/pull-to-refresh) controls, having [RefreshContainer](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.refreshcontainer) as the main control.
Copy link
Contributor

Choose a reason for hiding this comment

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

1803 will be released when 3.0 is released, remove "soon to be released"

## <a name="refreshcontainer"></a> Moving to RefreshContainer
The (soon to be released) 1803 version of Windows now includes its own implementation of [pull-to-refresh](https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/pull-to-refresh) controls, having [RefreshContainer](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.refreshcontainer) as the main control.

The PullToRefreshListView and the RefreshContainer share the same concepts and provide mostly the same functionality, with the caveats that the RefreshContainer works only with a touch interface and it's not as customizable as the PullToRefreshListView.
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "it's not as customizable as the PullToRefreshListView"

Being a touch-only control, it's recommended that you also have a refresh button for users without a touch interface. You can trigger the RefreshRequested event by calling the RefreshContainer's RequestRefresh method.

### Making the transition even easier
Starting with v3 of the UWP Community Toolkit, the PullToRefreshListView provides a new property called **UseRefreshContainerWhenPossible**. Setting the value to true will force the PullToRefreshListView to use a template based on the RefreshContainer when running on the 1803 version of Windows and above, and the regular template otherwise.
Copy link
Contributor

Choose a reason for hiding this comment

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

should be v3.0

@nmetulev nmetulev changed the base branch from master to rel/3.0.0-preview April 24, 2018 19:36
@nmetulev nmetulev merged commit 4fd0046 into CommunityToolkit:rel/3.0.0-preview Apr 30, 2018
@windowstoolkitbot
Copy link

This PR is linked to unclosed issues. Please check if one of these issues should be closed: #1946

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants