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

[Feature] Remove Behaviors reference? #67

Closed
michael-hawker opened this issue Jan 14, 2021 · 3 comments · Fixed by #76
Closed

[Feature] Remove Behaviors reference? #67

michael-hawker opened this issue Jan 14, 2021 · 3 comments · Fixed by #76
Assignees
Labels
Completed 🔥 enhancement ✨ New feature or request
Milestone

Comments

@michael-hawker
Copy link
Member

michael-hawker commented Jan 14, 2021

Describe the problem this feature would solve

We currently use the Behaviors package (via the Toolkit) to ensure all properties are loaded before we initialize the Graph via our XAML Provider helpers. This is a larger dependency (and causes more issues with our dependency issue from #66) for this small benefit. We should use an alternate solution to provide the same benefit without the overhead.

Describe the solution

We should add an attached property for Page (or maybe Control/FrameworkElement) which is named something like GraphProvider and accepts one of our Graph Providers. This would when the property is set, look at Loaded event for the attached FrameworkElement parent and at that point call our Initialize method on the set provider.

This would allow the Initialize method to still have access to all the property values we need on the providers to initialize the graph components, but without the need for the full Behaviors package which we utilize no other parts from.

Initialization Before

<Page
    ...
    xmlns:Interactivity="using:Microsoft.Xaml.Interactivity"
    xmlns:providers="using:Microsoft.Toolkit.Graph.Providers">
    <Interactivity:Interaction.Behaviors>
        <providers:InteractiveProviderBehavior ClientId="YOUR_CLIENT_ID_HERE" Scopes="User.Read,User.ReadBasic.All,People.Read"/>
    </Interactivity:Interaction.Behaviors>
</Page>

Initialization After

<Page
    ...
    xmlns:providers="using:Microsoft.Toolkit.Graph.Providers">
    <providers:Extensions.GraphProvider>
        <providers:InteractiveProvider ClientId="YOUR_CLIENT_ID_HERE" Scopes="User.Read,User.ReadBasic.All,People.Read"/>
    </providers:Extensions.GraphProvider>
</Page>

Other Considerations

We currently ship a WPF based package for our providers for use within XAML Islands. Not sure if we'd still want to ship a similar solution or not, we'd have to discuss. It was a bit easier before as we could leverage the same behavior code pretty easily between the two, not sure if this solution would be similar...

@ghost ghost added the needs triage 🔍 label Jan 14, 2021
@ghost
Copy link

ghost commented Jan 14, 2021

Hello michael-hawker, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker michael-hawker added enhancement ✨ New feature or request and removed needs triage 🔍 labels Jan 14, 2021
@michael-hawker
Copy link
Member Author

FYI @shweaver-MSFT let me know your thoughts!

@shweaver-MSFT
Copy link
Member

I think we settled on something like this:

    <Application.Resources>
        <!--
            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".
        -->
        <ResourceDictionary>
            <providers:Graph.Config>
                <!--<msal:MsalConfig ClientId="YOUR_CLIENT_ID_HERE" Scopes="User.Read,User.ReadBasic.All,People.Read,Calendars.Read,Mail.Read,Group.Read.All,ChannelMessage.Read.All" />-->
                <!--<uwp:WindowsConfig ClientId="YOUR_CLIENT_ID_HERE" Scopes="User.Read,User.ReadBasic.All,People.Read,Calendars.Read,Mail.Read,Group.Read.All,ChannelMessage.Read.All" />-->
                <providers:MockConfig />
            </providers:Graph.Config>
        </ResourceDictionary>
    </Application.Resources>

I'll submit a PR with the code to make this work.

@ghost ghost added the In-PR 🚀 label Mar 9, 2021
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Mar 24, 2021
@shweaver-MSFT shweaver-MSFT self-assigned this Apr 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 5, 2021
@shweaver-MSFT shweaver-MSFT added this to the 7.1.0 milestone Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Completed 🔥 enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants