Merge ~ProgressBarValue classes

[Nirmal4G]

Fixes #4073

Meld BindableProgressBarValue into AdaptiveProgressBarValue. Although this is a Breaking Change, it doesn't change any functional behavior. It's refractored to be a drop-in replacement. So, just a global text replace is enough.

PR Type

What kind of change does this PR introduce?

• Feature
• Refactoring (NO functional changes, API Changes only)
• Documentation content changes
• Sample app changes

What is the current behavior?

It's confusing to have BindableProgressBarValue and AdaptiveProgressBarValue providing similar functionality but on different platforms with no common type, even though that's possible.

What is the new behavior?

Replaced the BindableProgressBarValue class with AdaptiveProgressBarValue class which wasn't available in previous versions.

PR Checklist

Please check if your PR fulfils the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link: TBA
• Sample in sample app has been added / updated (for bug fixes / features)
• Tests for the changes have been updated (for bug fixes / features)

Other information

• If you're editing this patch tree, please rebase on latest HEAD and then commit, without updating from the latest HEAD.
• When merging, please update the commit title to PR title instead of the default Merge pull request #xxxx from repo/branch, and commit message to either PR message or messages of individual commits. The auto-merge bot does this by default.

[ghost]

Thanks Nirmal4G 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 🙌

[michael-hawker]

@andrewleader thoughts on this? If we're going to make a breaking change though this will have to wait until an 8.0 release, which we currently don't have scheduled.

[andrewleader]

There were explicit reasons why there were two different classes for WinRT (C++) vs the .NET (C#) libraries. One of the classes supports dynamic type conversion, so developers can assign doubles or strings if using absolute values. WinRT didn't like that though, so for WinRT I created separate classes (WinRT is only used when consumed in a C++ app).

I haven't been able to thoroughly look through this, so I'm not sure if it's changed any functional behavior of being able to use the automatic type conversions or anything.

Additionally, someday in the future we're going to implement these toast notification APIs in Project Reunion (which will be a fully new API set). I'm in favor of holding off any breaking changes until when we bring these APIs into Project Reunion. Switching to the Project Reunion APIs will already be a breaking change, might as well have those changes bulked together.

This is great feedback to consider though when we bring these APIs into Project Reunion!

[Nirmal4G]

@andrewleader It's a drop-in replacement. It doesn't change any functional behavior.

Both implicit and explicit type conversions still work for non-WinRT targets.

[andrewleader]

Thanks @Nirmal4G, I took a slightly longer look at this, and while it does look like you're right that this will work for both WinRT (C++) and non-WinRT, I do have concerns about the breaking change. Developers that wrote AdaptiveProgressBarValue.Indeterminate will have to change to BindableProgressBarValue.Indeterminate.

I fully agree that the APIs should have been what you have here from the start. However, I'm not sure this small change is worth the breaking change.

As I explained earlier, we're going to be considering moving these APIs into the Windows App SDK, and that alone will be a breaking change and an opportunity to redo these classes how you designed them. How would you feel about waiting till these APIs move to the Windows App SDK? That probably won't happen till late 2022 or possibly 2023.

[michael-hawker]

Thanks for the info @andrewleader, we'll probably have another major version before that timeframe, so we can think about applying these changes at that time as well.

I don't know when an '8.0' will be on the horizon at the moment. I think we'll have a 7.2 in timeline for the .NET 6 release in November-ish. So, probably wouldn't be at the earliest until start of 2022.

[Nirmal4G]

I do have concerns about the breaking change. Developers that wrote AdaptiveProgressBarValue.Indeterminate will have to change to BindableProgressBarValue.Indeterminate.

It's actually other way around. But yes! They have to do a global find and replace for the type.

How would you feel about waiting till these APIs move to the Windows App SDK?

So, probably wouldn't be at the earliest until start of 2022.

If it takes more than 6 months, why not make a change here, gather feedback and properly ingest these APIs into the WAS (Windows App SDK)?

That's one of the purpose of this project, right?

[michael-hawker]

If it takes more than 6 months, why not make a change here, gather feedback and properly ingest these APIs into the WAS (Windows App SDK)?

That's one of the purpose of this project, right?

💯 but since it's a breaking change we hold those to our major releases. We don't have an 8.0 scheduled yet.

Moving this to draft, as that'll still probably occur before these move into the SDK; so we can pick this up when we get there.