Enhance maui-mobile developer sample with recent updates #31646
Conversation
|
Hey there @@Vignesh-SF3580! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
dotnet-policy-service
bot
added
community ✨
Tamilarasan-Paranthaman
added
the
area-templates
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the maui-mobile developer sample by updating the UI layout and fixing a null reference exception that occurred when clicking on projects after switching from ScrollView to CollectionView.
- Replaces VerticalStackLayout with Grid layout for better organization
- Switches from HorizontalStackLayout with BindableLayout to CollectionView for project display
- Adds null checking in the NavigateToProject command to prevent exceptions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Templates/src/templates/maui-mobile/Pages/MainPage.xaml | Refactors layout from VerticalStackLayout to Grid and replaces project display with CollectionView |
| src/Templates/src/templates/maui-mobile/PageModels/MainPageModel.cs | Adds SelectedProject property and null checking in NavigateToProject command |
| private Task? NavigateToProject(Project project) | ||
| => project is null ? null : Shell.Current.GoToAsync($"project?id={project.ID}"); |
There was a problem hiding this comment.
The method returns Task? which creates an inconsistent API design. Consider throwing ArgumentNullException for null projects or restructuring to always return a completed Task to maintain a consistent return type.
Copilot uses AI. Check for mistakes.
| SelectionChangedCommand="{Binding NavigateToProjectCommand, Source={RelativeSource AncestorType={x:Type pageModels:MainPageModel}}, x:DataType=pageModels:MainPageModel}" | ||
| SelectionChangedCommandParameter="{Binding SelectedProject}"> |
There was a problem hiding this comment.
The SelectionChangedCommandParameter binding to SelectedProject is redundant since the SelectedItem is already bound to SelectedProject. The command can access the selected project directly through the SelectedProject property, eliminating the need for the command parameter.
| SelectionChangedCommand="{Binding NavigateToProjectCommand, Source={RelativeSource AncestorType={x:Type pageModels:MainPageModel}}, x:DataType=pageModels:MainPageModel}" | |
| SelectionChangedCommandParameter="{Binding SelectedProject}"> | |
| SelectionChangedCommand="{Binding NavigateToProjectCommand, Source={RelativeSource AncestorType={x:Type pageModels:MainPageModel}}, x:DataType=pageModels:MainPageModel}"> |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Yeah, can simplify it because the command can access the selected project directly through the SelectedProject property. I'm ok with both options.
| private Task? NavigateToProject(Project project) | ||
| => project is null ? null : Shell.Current.GoToAsync($"project?id={project.ID}"); |
There was a problem hiding this comment.
The method should return a consistent Task type rather than nullable Task. Consider returning Task.CompletedTask for null projects instead of null to maintain a consistent async pattern.
| private Task? NavigateToProject(Project project) | |
| => project is null ? null : Shell.Current.GoToAsync($"project?id={project.ID}"); | |
| private Task NavigateToProject(Project project) | |
| => project is null ? Task.CompletedTask : Shell.Current.GoToAsync($"project?id={project.ID}"); |
Copilot uses AI. Check for mistakes.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
Updated the maui-mobile developer sample with the latest changes from maui-sample.
After changing ScrollView to CollectionView, a null exception occurred when clicking on projects. I resolved this issue using Javier's changes.
Associated Pull Requests
dotnet/maui-samples#662
Screenshots