[Net10.0] Toolbar item - IconColor property #30826
Conversation
|
Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| } | ||
| else | ||
| { | ||
| Image = result?.Value.ImageWithRenderingMode(UIImageRenderingMode.AlwaysTemplate); |
There was a problem hiding this comment.
What should happen when result?.Value is null here?
181abb8 to
8870fca
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new IconColor bindable property for ToolbarItem that allows developers to customize the color of toolbar icons across platforms. The feature enables setting toolbar icon colors directly in XAML or code-behind, providing better visual customization for cross-platform applications.
Key changes:
- Added
IconColorbindable property toToolbarItemclass with proper change notifications - Implemented platform-specific icon color handling for Android and iOS
- Created comprehensive UI tests to validate the new functionality
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Controls/src/Core/Toolbar/ToolbarItem.cs |
Adds the new IconColor bindable property and update mechanism |
src/Controls/src/Core/Platform/Android/Extensions/ToolbarExtensions.cs |
Implements Android-specific icon color application logic |
src/Controls/src/Core/Compatibility/iOS/Extensions/ToolbarItemExtensions.cs |
Implements iOS-specific icon color handling with proper rendering modes |
src/Controls/tests/TestCases.HostApp/Issues/Issue30818.xaml |
Creates UI test page demonstrating toolbar items with different icon colors |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30818.cs |
Adds automated UI test for the new IconColor functionality |
Multiple PublicAPI.Unshipped.txt files |
Updates public API surface to include the new property |
Comments suppressed due to low confidence (2)
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30818.cs:1
- The test is excluded on Windows platform, but according to the coding guidelines, all code changes should have relevant test cases that can execute properly across platforms. Consider implementing Windows support or providing a platform-specific test that validates the expected behavior (even if it's a no-op on Windows).
#if TEST_FAILS_ON_WINDOWS // It is not implemented on Windows yet
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30818.cs:19
- The test only verifies a screenshot without any specific assertions about the IconColor property behavior. Consider adding more comprehensive tests that validate the actual color application, such as checking element properties or adding multiple test cases for different color scenarios.
VerifyScreenshot();
| @@ -17,6 +18,9 @@ public class ToolbarItem : MenuItem | |||
|
|
|||
| static readonly BindableProperty PriorityProperty = BindableProperty.Create(nameof(Priority), typeof(int), typeof(ToolbarItem), 0); | |||
|
|
|||
| public static readonly BindableProperty IconColorProperty = BindableProperty.Create(nameof(IconColor), typeof(Color), typeof(ToolbarItem), null, | |||
There was a problem hiding this comment.
This change introduces a new public API property 'IconColor' to the ToolbarItem class. This is a potentially breaking change that should not be included in minor versions or service releases. Please ensure this is targeted for a major version release.
Copilot uses AI. Check for mistakes.
|
|
||
| if (item.IconColor is Color iconColor) | ||
| { | ||
| if (item.IconImageSource is FontImageSource fontImageSource && fontImageSource.Color is null) |
There was a problem hiding this comment.
[nitpick] The logic for handling FontImageSource vs other image sources is complex and nested. Consider extracting this icon color application logic into a separate method to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
|
How does this work with the Shell Foreground color ? |
The Foreground doesn't affect the toolbar icon color |
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
This PR introduces a new
IconColorbindable property forToolbarItemthat allows developers to customize the color of toolbar icons directly in XAML or code, bringing more flexibility and platform consistency to the appearance of toolbars.Issues Fixed
Fixes #30818
Screen.Recording.2025-07-25.at.02.09.50.mov