-
Notifications
You must be signed in to change notification settings - Fork 465
[Popup] Add ComplexPopup to CommunityToolkit.Maui.Sample
#2771
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
Conversation
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.
Pull Request Overview
This PR introduces a new ComplexPopup sample to demonstrate constructor-based DI with PopupService, handling the Opened event, binding to PopupOptions, and returning a result.
- Adds
ComplexPopupview and its XAML, showcasing a delayed description update on open. - Implements
ComplexPopupViewModelandComplexPopupOptionsViewModelfor DI, option binding, and return-value handling. - Hooks up a “Complex Popup” button in
PopupsPagewith dynamic overlay color changes and registers the popup in DI.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| samples/CommunityToolkit.Maui.Sample/Views/Popups/ComplexPopup.xaml.cs | New ComplexPopup class with DI constructor, event handler |
| samples/CommunityToolkit.Maui.Sample/Views/Popups/ComplexPopup.xaml | Layout for the complex popup |
| samples/CommunityToolkit.Maui.Sample/ViewModels/Views/Popup/ComplexPopupViewModel.cs | ViewModel injecting IPopupService, return command logic |
| samples/CommunityToolkit.Maui.Sample/ViewModels/Views/Popup/ComplexPopupOptionsViewModel.cs | ViewModel for popup options (overlay color) |
| samples/CommunityToolkit.Maui.Sample/Pages/Views/Popup/PopupsPage.xaml.cs | Added HandleComplexPopupClicked with dynamic overlay update |
| samples/CommunityToolkit.Maui.Sample/Pages/Views/Popup/PopupsPage.xaml | Added “Complex Popup” button |
| samples/CommunityToolkit.Maui.Sample/MauiProgram.cs | Registered ComplexPopup and ComplexPopupViewModel in DI |
Comments suppressed due to low confidence (3)
samples/CommunityToolkit.Maui.Sample/ViewModels/Views/Popup/ComplexPopupViewModel.cs:4
- [nitpick] The namespace omits the
.Popupsegment despite living in aPopupfolder, which may lead to confusion. Consider updating it toCommunityToolkit.Maui.Sample.ViewModels.Views.Popupfor consistency.
namespace CommunityToolkit.Maui.Sample.ViewModels.Views;
samples/CommunityToolkit.Maui.Sample/ViewModels/Views/Popup/ComplexPopupOptionsViewModel.cs:3
- [nitpick] The namespace omits the
.Popupsegment despite the file residing in aPopupfolder. Rename it toCommunityToolkit.Maui.Sample.ViewModels.Views.Popupto match the folder structure.
namespace CommunityToolkit.Maui.Sample.ViewModels.Views;
samples/CommunityToolkit.Maui.Sample/Pages/Views/Popup/PopupsPage.xaml.cs:150
- The
popupServicevariable is not defined in this class, causing a compilation error. Consider using thethis.ShowPopupAsync<ComplexPopup, string>(complexPopupOptions)extension method or injectIPopupServiceinto this page.
var popupResultTask = popupService.ShowPopupAsync<ComplexPopup, string>(Navigation, complexPopupOptions);
samples/CommunityToolkit.Maui.Sample/Views/Popups/ComplexPopup.xaml.cs
Outdated
Show resolved
Hide resolved
bijington
left a comment
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.
Thanks for this @TheCodeTraveler i left the one comment around the example for PopupOptions binding but otherwise it looks good
samples/CommunityToolkit.Maui.Sample/Pages/Views/Popup/PopupsPage.xaml.cs
Outdated
Show resolved
Hide resolved
pictos
left a comment
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.
LGTM, I left a small comment if you want to address or adda comment for reference
samples/CommunityToolkit.Maui.Sample/ViewModels/Views/Popup/ComplexPopupViewModel.cs
Show resolved
Hide resolved
bijington
left a comment
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.
LGTM from me! Just need to check the comment from @pictos regarding the Window being used
|
@TheCodeTraveler I have added this docs PR: MicrosoftDocs/CommunityToolkit#572 it may need some more text around it but I can get to that soon. We should be good to merge and release 👍 |
Description of Change
This PR adds a more complex example of using a Popup that demonstrates the following:
PopupServicePopup.Openedevent to trigger an action once the Popup appearsPopupOptionsPR Checklist
mainat time of PRAdditional information
Let's add this sample to the Popup docs @bijington