-
Notifications
You must be signed in to change notification settings - Fork 39
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
Initial GraphPresenter prototype for #17 #18
Conversation
# Conflicts: # Microsoft.Toolkit.Graph/Providers/ProviderManager.cs # SampleTest/MainPage.xaml
Need to fix StyleCop issues:
|
Not fully tested as hit error with payload response see microsoftgraph/msgraph-sdk-dotnet#740
|
Update Calendar Example Test
TODO: Display HTML better or strip out tags.
(they need re-work see #55) Also updated Teams sample messages to remove HTML tags for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 😄
Just left a few suggestions and a couple questions!
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/QueryOption.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Sergio Pedri <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 🚀
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/QueryOption.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs
Show resolved
Hide resolved
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs
Show resolved
Hide resolved
Microsoft.Toolkit.Graph.Controls/Controls/GraphPresenter/GraphPresenter.cs
Show resolved
Hide resolved
@azchohfi thanks for the review, addressed your comments as well and opened #56 for tracking. If it looks good, please merge this. Also added #57 for updating to our new 7.0.0-preview2 package and the Target SDK bump. If you have a chance to do that update today, that'd be big help for UnoConf. Then we could update our preview package for this as well to align with Thanks! 🦙❤ |
Working on it!
|
Fixes #17
Fixes #27
Pushed some things needed here to the Toolkit in CommunityToolkit/WindowsCommunityToolkit#3033, will update this once those get merged in.
'Done':
How to deal with paging? Use IncrementalLoadingCollection?Opened Issue [Feature] Handle Paging of Graph Calls in GraphPresenter #54 to track for futureSample: What to do with empty thumbnails, should the converter just be a 'sample' one vs. part of the lib?Needs Some RequestBuilder interfaces don't implement commonIBaseRequestBuilder
microsoftgraph/MSGraph-SDK-Code-Generator#266 to be fixed to continue OneDrive sampleXAML Islands: Test in XAML Islands, maybe make it so XAML Islands UWP just uses MainPage from UWP sample???Doesn't need to be part of this workTODO/Questions: