Restructuring package layout, removing behaviors

[shweaver-MSFT]

• Removed behaviors from controls package
• Renamed StateChangedEventArgs to ProviderStateChangedEventArgs
• Shuffled packages around

Fixes #60
Fixes #67
Fixes #75
Fixes #78

PR Type

What kind of change does this PR introduce?

• Feature

What is the current behavior?

Currently, Providers can be declared in XAML leveraging Behaviors SDK.

<!--
    Initialize Graph Provider On Page Load
        
    Register Client Id: https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-register-app
        
    After finishing the initial registration page, you will also need to add an additional redirect URI.
    Click on "Add a Redirect URI" and check the https://login.microsoftonline.com/common/oauth2/nativeclient checkbox on that page. Then click "Save".
-->
<Interactivity:Interaction.Behaviors>
    <!--<providers:InteractiveProviderBehavior ClientId="YOUR_CLIENT_ID_HERE" Scopes="User.Read,User.ReadBasic.All,People.Read,Calendars.Read,Mail.Read,Group.Read.All,ChannelMessage.Read.All"/>-->
    <providers:MockProviderBehavior />
</Interactivity:Interaction.Behaviors>

What is the new behavior?

In this PR I've removed the behaviors references from the Controls package which includes how to declare Graph config from XAML. This means that the GlobalProvider can no longer be configured from XAML.

The new way is to declare in the project's root App.xaml.cs and initialize the GlobalProvider on app launch.

private enum ProviderType
{
    Mock,
    Msal
}

private void InitializeGlobalProvider()
{
    if (ProviderManager.Instance.GlobalProvider != null)
    {
        return;
    }

    // Which provider should be used?
    ProviderType providerType = ProviderType.Mock;

    // Provider config
    string clientId = "YOUR_CLIENT_ID_HERE";
    string[] scopes = { "User.Read", "User.ReadBasic.All", "People.Read", "Calendars.Read", "Mail.Read", "Group.Read.All", "ChannelMessage.Read.All" };

    switch(providerType)
    {
        // Mock provider
        case ProviderType.Mock:
            ProviderManager.Instance.GlobalProvider = new MockProvider(signedIn: true);
            break;

        //Msal provider
        case ProviderType.Msal:
            ProviderManager.Instance.GlobalProvider = new MsalProvider(clientId: clientId, scopes: scopes);
            break;
    }
}

/// <summary>
/// Invoked when the application is launched normally by the end user.  Other entry points
/// will be used such as when the application is launched to open a specific file.
/// </summary>
/// <param name="e">Details about the launch request and process.</param>
protected override void OnLaunched(LaunchActivatedEventArgs e)
{
    Task.Run(InitializeGlobalProvider);
    ...

Package shuffle

I've adjusted the package structure to meet a few of our project goals:

• Remove Graph SDK consumption from the main auth/providers package.
• Decouple authentication from graph interaction.

The package structure looks like this now:

CommunityToolkit.Net.Authentication

Core constructs for authentication: No Graph SDK, NetStandard

• NEW BaseProvider: Basic plumbing for responding to changes in the GlobalProvider. Extends IProvider.
• IProvider
• MockProvider: Now extends BaseProvider
• ProviderManager
• ProviderManagerChangedState
• ProviderState
• ProviderStateChangedEventArgs
• ProviderUpdatedEventArgs

CommunityToolkit.Net.Authentication.Msal

Authentication via Microsoft.Identity.Client.Extensions.Msal package: No Graph SDK, NetStandard

• MsalProvider: Now extends BaseProvider

CommunityToolkit.Net.Graph

Graph SDK interaction: Has Beta Graph SDK, NetStandard

• GraphExtensions
• NEW ProviderExtensions: Enables Graph client access from any IProvider.
• Helpers

CommunityToolkit.Uwp.Graph.Controls

Graph enabled controls and helpers: Has Beta Graph SDK, UWP

• GraphPresenter
• LoginButton
• PeoplePicker
• PersonView

Rename

I've renamed StateChangedEventArgs to ProviderStateChangedEventArgs for the sake of consistency.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• 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)
• Contains NO breaking changes

Other information

[ghost]

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

[shweaver-MSFT]

@michael-hawker @nmetulev I've made some more changes to the package structure based on our convo the other day. I think this setup works nice, but I'd love your feedback :)

[nmetulev]

I love this direction and simplifying things - but I think it can be simplified even more. I also like that we are changing the package names already - it's the right time to do it.

Few thoughts and comments

1. I'd move MsalProvider to it's own package (CommunityToolkit.Auth.MsalProvider).
   1. All packages reference Microsoft.Graph.Auth package - I'd expect this to only be referenced by the MsalProvider
   2. Related but not important for this PR, MsalProvider should not be built on top of Microsoft.Graph.Auth - but that should be a separate issue and work - created MsalProvider should be built on top of Msal.NET directly instead of Microsoft.Graph.Auth #78.
2. Consider combining CommunityToolkit.Auth and CommunityToolkit.Auth.Uwp in a single package, leveraging multi-targeting .netstandard and uwp - the benefit here being:
   1. You can combine ProviderManager and GlobalProvider into a single class - this simplifies the structure and makes it easier to understand
   2. You can keep the provider config alongside the provider class - simplifying things once again
3. Consider renaming the controls package to CommunityToolkit.Graph.Uwp.Controls since it will only contain controls

cc/ @michael-hawker for his thoughts

[shweaver-MSFT]

@nmetulev, Love the suggestions! I'll jump on those next and send an update.

On 3 though, I was thinking about putting helpers (e.g. RoamingSettings) in that package too, which is why I ended it in Uwp. Does that seem right the right place for them, and does the name change still make sense in that case? I knod of like it as *.Uwp imo

[shweaver-MSFT]

@nmetulev @michael-hawker I think I've made all of the major changes we discussed. Let me know if I missed anything!

[michael-hawker]

Added some initial comments, but I think the biggest thing to tackle is how we unravel the dependency on the Graph SDK for basic authentication scenarios. Added a comment in the WindowsProvider PR to give more context: https://github.com/windows-toolkit/Graph-Controls/pull/69/files#r596451587

[michael-hawker]

Ah, so we're still relying on this package... hmmm. This is what's pulling in MSAL/ (So if you only want the Windows provider, you're still pulling in MSAL?)

Also, this is the part that's getting rolled into the new Graph SDK which includes everything together... @nmetulev thoughts on if we should try and isolate that for the future vs. knowing eventually we'll pull in the whole Graph SDK... This seems like it's going to be a big problem for consumption of their v2 as we had mentioned.

What pieces are we still referencing from here (besides MSAL)? Like should we have an explicit CommunityToolkit.Net.Authentication.Msal or something?

I wonder if we investigate the source only style package, so rather than having another DLL for just the single MSAL Provider, it effectively just includes the CS file and it builds as part of the consuming app... 🤔 @Sergio0694 thoughts here? Is this the article you've been looking at for this 'feature'?

[shweaver-MSFT]

I've moved MsalProvider to it's own package, CommunityToolkit.Net.Authentication.Msal, and it's now based on the official Microsoft.Identity.Client.Extensions.Msal library.

I looked into the source code only package, but it has an unfortunate side effect of not being able to use project references anymore. Which means I can't reference the msal package from the sample app, because it emits a nuget from build, not a DLL. I think it's probably fine to leave it as a typical, dll-emitting package imo.

[michael-hawker]

Weird build issue:

"D:\a\1\s\Windows-Toolkit-Graph-Controls.sln" (Pack target) (1) ->
       "D:\a\1\s\CommunityToolkit.Uwp.Graph.Controls\CommunityToolkit.Uwp.Graph.Controls.csproj" (Pack target) (2) ->
       "D:\a\1\s\CommunityToolkit.Uwp.Graph.Controls\CommunityToolkit.Uwp.Graph.Controls.csproj" (_GetTfmSpecificContentForPackage target) (2:4) ->
       "D:\a\1\s\CommunityToolkit.Net.Graph\CommunityToolkit.Net.Graph.csproj" (GetPackagingOutputs target) (4:17) ->
       (ResolvePackageAssets target) -> 
         C:\Program Files\dotnet\sdk\5.0.201\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(241,5): error NETSDK1005: Assets file 'D:\a\1\s\CommunityToolkit.Net.Graph\obj\project.assets.json' doesn't have a target for 'uap10.0.17763'. Ensure that restore has run and that you have included 'uap10.0.17763' in the TargetFrameworks for your project. [D:\a\1\s\CommunityToolkit.Net.Graph\CommunityToolkit.Net.Graph.csproj]


       "D:\a\1\s\Windows-Toolkit-Graph-Controls.sln" (Pack target) (1) ->
       "D:\a\1\s\CommunityToolkit.Uwp.Graph.Controls\CommunityToolkit.Uwp.Graph.Controls.csproj" (Pack target) (2) ->
       "D:\a\1\s\CommunityToolkit.Uwp.Graph.Controls\CommunityToolkit.Uwp.Graph.Controls.csproj" (_GetTfmSpecificContentForPackage target) (2:4) ->
       "D:\a\1\s\CommunityToolkit.Net.Authentication\CommunityToolkit.Net.Authentication.csproj" (GetPackagingOutputs target) (3:31) ->
         C:\Program Files\dotnet\sdk\5.0.201\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(241,5): error NETSDK1005: Assets file 'D:\a\1\s\CommunityToolkit.Net.Authentication\obj\project.assets.json' doesn't have a target for 'uap10.0.17763'. Ensure that restore has run and that you have included 'uap10.0.17763' in the TargetFrameworks for your project. [D:\a\1\s\CommunityToolkit.Net.Authentication\CommunityToolkit.Net.Authentication.csproj]

Looks like target SDK mismatch somewhere?

[shweaver-MSFT]

No idea on the weird build error. I'll have to look into it more. It started after I added the msal package.

[shweaver-MSFT]

Hooray! I fixed the build issue :)

[michael-hawker]

Ah, it's because the packages were no longer multi-targeted but the UWP workaround was still included...

@jeromelaban was multi-targeting the .NET Standard packages something done for Uno? Can we just use .NET Standard 2.0 things directly now even in WASM?

[jeromelaban]

@jeromelaban was multi-targeting the .NET Standard packages something done for Uno? Can we just use .NET Standard 2.0 things directly now even in WASM?

Yes netstandard2.0 can be used in .NET for WebAssembly as well. If there are specifics that use uap10.xxx, then those can be multi-targeted later on.

[nmetulev]

I really like this. Few questions and comments, but overall, really good

[nmetulev]

Just a thought - should we have two packages (one pulling in the beta sdk, and one the v1 sdk)? The source can be all the same, just this csproj will be different.

[shweaver-MSFT]

That sounds pretty slick. Do you think there is any potential for type mismatch if the SDKs deviate much?

Michael and I had talked about splitting on Beta vs v1, this could be a great way to do it. Either way, I'll save that for another PR - #52

[nmetulev]

Awesome. Yeah, I think it's worth trying.