-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Sync DeveloperBalance sample changes from the MAUI Samples repo to the .NET MAUI repo #32148
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
|
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. |
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 synchronizes the DeveloperBalance template sample from the MAUI Samples repository to the .NET MAUI repository, bringing UI improvements, accessibility enhancements, and code refactoring changes.
Key Changes:
- Replaced deprecated layout options (
FillAndExpand,CenterAndExpand) with their modern equivalents - Refactored tag selection UI from
HorizontalStackLayoutwith template selector toCollectionViewwith visual states - Added platform-specific handler configurations for iOS, macOS, and Windows to improve keyboard accessibility
Reviewed Changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| AppStyles.xaml | Updated layout options to use non-deprecated values |
| Info.plist | Added MacCatalyst app category metadata |
| ProjectListPage.xaml.cs | Added binding context assignment for appearing behavior |
| ProjectListPage.xaml | Replaced BindableLayout with CollectionView for improved selection handling |
| ProjectDetailPage.xaml | Refactored tag selection from template selector to CollectionView with visual states |
| MainPage.xaml | Reordered Grid property attributes |
| TaskView.xaml | Restructured task item layout to improve accessibility |
| ProjectCardView.xaml | Changed tag container from HorizontalStackLayout to FlexLayout and removed deprecated property |
| ChartDataLabelConverter.cs | Created new converter to replace DataLabelValueConverter with updated implementation |
| CategoryChart.xaml | Updated to use new ChartDataLabelConverter and cleaned up duplicate x:DataType attributes |
| ProjectListPageModel.cs | Added selected project property and null check for navigation |
| ProjectDetailPageModel.cs | Added SelectionChanged command to handle CollectionView multi-selection |
| MauiProgram.cs | Added platform-specific handler configurations for improved keyboard accessibility |
| TagRepository.cs | Added duplicate association check to prevent redundant database operations |
| DataLabelValueConverter.cs | Removed obsolete converter class |
| AppShell.xaml.cs | Added semantic description for theme control on Android/Windows |
| AppShell.xaml | Added semantic descriptions to theme segment items |
| { | ||
| BindingContext = model; | ||
| InitializeComponent(); | ||
| AppearingBehavior.BindingContext = model; |
Copilot
AI
Oct 22, 2025
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.
Setting BindingContext on a behavior after InitializeComponent() is unusual, as behaviors typically inherit the binding context from their parent element. This could indicate a deeper issue with how the behavior's data context is resolved. Consider whether this explicit assignment is necessary, or if the XAML binding on line 17 of the .xaml file should be sufficient.
| AppearingBehavior.BindingContext = model; |
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.
BindingContext is already set in XAML, so there’s no need to set it here. I have removed it
| BindableLayout.ItemTemplateSelector="{StaticResource ChipDataTemplateSelector}"/> | ||
| </ScrollView> | ||
|
|
||
| <!-- TODO This is still a little jumpy when changing selection --> |
Copilot
AI
Oct 22, 2025
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.
This TODO comment indicates an unresolved UI issue with tag selection. Before merging, either resolve the jumpiness issue or create a tracked work item to address it.
| <!-- TODO This is still a little jumpy when changing selection --> | |
| <!-- UI jumpiness when changing tag selection. Tracked in issue #123. --> |
| IsChecked="{Binding IsCompleted, Mode=OneTime}" | ||
| VerticalOptions="Center" | ||
| CheckedChanged="CheckBox_CheckedChanged" | ||
| AutomationProperties.IsInAccessibleTree="True" |
Copilot
AI
Oct 22, 2025
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.
The default value of IsInAccessibleTree is already True. This explicit assignment is redundant and can be removed.
| AutomationProperties.IsInAccessibleTree="True" |
src/Templates/src/templates/maui-mobile/Pages/Controls/ChartDataLabelConverter.cs
Outdated
Show resolved
Hide resolved
| Task NavigateToProject(Project project) | ||
| => Shell.Current.GoToAsync($"project?id={project.ID}"); | ||
| Task? NavigateToProject(Project project) | ||
| => project is null ? null : Shell.Current.GoToAsync($"project?id={project.ID}"); |
Copilot
AI
Oct 22, 2025
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.
Returning null as a Task? instead of Task.CompletedTask is unconventional. Consider using Task.CompletedTask for consistency with async patterns, or change the return type to Task and throw an exception if the project is null.
| => project is null ? null : Shell.Current.GoToAsync($"project?id={project.ID}"); | |
| => project is null ? Task.CompletedTask : Shell.Current.GoToAsync($"project?id={project.ID}"); |
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.
Why at any point would we be passing a null value into NavigateToProject?
is this a bug further up?
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.
The Project was never null, and I updated the code based on the suggestion.
| await SaveItemAsync(item); | ||
|
|
Copilot
AI
Oct 22, 2025
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.
The IsAssociated method calls SaveItemAsync(item) which modifies the database state while checking if an association exists. This is a side effect that shouldn't occur in a method that appears to only query state. Remove line 230 to make this a pure query method.
| await SaveItemAsync(item); |
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.
Removed the changes.
| Microsoft.Maui.Handlers.ContentViewHandler.Mapper.AppendToMapping(nameof(Pages.Controls.CategoryChart), (handler, view) => | ||
| { | ||
| if (view is Pages.Controls.CategoryChart && handler.PlatformView is ContentPanel contentPanel) | ||
| { | ||
| contentPanel.IsTabStop = true; | ||
| } | ||
| }); |
Copilot
AI
Oct 22, 2025
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.
[nitpick] Using nameof(Pages.Controls.CategoryChart) as the mapping key is unusual since this is typically a simple string identifier. Consider using a descriptive string constant like \"CategoryChartAccessibility\" instead for clarity.
Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
| BindableLayout.ItemTemplateSelector="{StaticResource ChipDataTemplateSelector}"/> | ||
| </ScrollView> | ||
|
|
||
| <!-- TODO This is still a little jumpy when changing selection --> |
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.
Is tracked somewhere in an issue?
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.
@Vignesh-SF3580 please leave out any TODO comments on the templates
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.
@jsuarezruiz I have checked the MAUI Samples issue reports - no related issue report were found.
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.
Removed the TODO comments.
|
/azp run MAUI-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
| #if (IncludeSampleContent) | ||
| .UseMauiCommunityToolkit() | ||
| .ConfigureSyncfusionToolkit() | ||
| //-:cnd:noEmit |
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.
That's required?
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.
yea, It's needed for the #IF WINDOWS Calls
We might want to move these so it's more explicilit that's what they are for
|
/backport to release/10.0.1xx |
|
Started backporting to release/10.0.1xx: https://github.com/dotnet/maui/actions/runs/18754846384 |
…#32112) * Initial plan * Migrate chart data label binding fix to Developer Balance template Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Integrate all changes from PR #32148 into Developer Balance template Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Add template semantic markers for platform-specific directives and document template conventions Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Update MauiProgram.cs * Move template instructions to path-specific YAML file Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Resync with PR #32148 - fix implementation differences Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Fix path-specific custom instructions format per GitHub docs Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Revert all template changes - keep only copilot instructions Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Update .github/copilot-instructions/templates.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Improve copilot instructions structure and clarity per feedback Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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!
Issue Details
The DeveloperBalance template sample in the .NET MAUI repo is not up to date with the latest changes from the MAUI Samples repo.
Description of Change
Updated the MAUI mobile DeveloperBalance sample with the latest changes from the MAUI Samples repository.
Screenshots
MacDebug.mov
WindowsDebug.mp4
AndroidDebug.mov
iOSDebug.mov