-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Provided Fill property support for BoxView control #31789
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
base: net10.0
Are you sure you want to change the base?
Conversation
Hey there @@NirmalKumarYuvaraj! 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 adds support for the Fill
property to the BoxView
control, enabling it to be filled with any Brush
(including gradients) instead of just solid colors. The implementation includes proper event handling, resource management, and comprehensive test coverage.
- Adds a new
Fill
property toBoxView
that accepts anyBrush
type - Implements proper event subscription and resource management for brush changes
- Updates the
IShapeView.Fill
implementation to prioritize the newFill
property over the existingColor
property
Reviewed Changes
Copilot reviewed 13 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/Controls/src/Core/BoxView/BoxView.cs |
Core implementation of the Fill property with event handling and resource management |
src/Controls/src/Core/Internals/WeakEventProxy.cs |
Adds WeakBrushChangedProxy class for managing brush change subscriptions |
src/Controls/src/Core/Shapes/Shape.cs |
Removes duplicate WeakBrushChangedProxy class |
src/Controls/src/Core/PublicAPI/*/PublicAPI.Unshipped.txt |
Updates public API surface for all platforms |
src/Controls/tests/TestCases.HostApp/FeatureMatrix/BoxView/BoxViewViewModel.cs |
Adds view model support for Fill property with gradient brushes |
src/Controls/tests/TestCases.HostApp/FeatureMatrix/BoxView/BoxViewControlPage.xaml |
Updates UI test page with Fill property binding and controls |
src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/BoxViewFeatureTests.cs |
Adds new UI tests for Fill property functionality |
/// <summary>Bindable property for <see cref="CornerRadius"/>.</summary> | ||
public static readonly BindableProperty CornerRadiusProperty = CornerElement.CornerRadiusProperty; | ||
|
||
/// <summary>Bindable property for <see cref="Color"/>.</summary> |
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 XML documentation comment incorrectly refers to Color
instead of Fill
. It should read 'Bindable property for <see cref="Fill"/>.'
/// <summary>Bindable property for <see cref="Color"/>.</summary> | |
/// <summary>Bindable property for <see cref="Fill"/>.</summary> |
Copilot uses AI. Check for mistakes.
<StackLayout Grid.Column="0" | ||
Grid.Row="0" | ||
Padding="20" | ||
Spacing="10"> |
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 removal of Padding='20' from the left column StackLayout may affect the visual layout and spacing. This change appears unrelated to the Fill property implementation and could impact existing UI tests.
Spacing="10"> | |
Spacing="10" | |
Padding="20"> |
Copilot uses AI. Check for mistakes.
<StackLayout Grid.Column="1" | ||
Grid.Row="0" | ||
Padding="20" | ||
Spacing="1"> |
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 removal of Padding='20' from the right column StackLayout may affect the visual layout and spacing. This change appears unrelated to the Fill property implementation and could impact existing UI tests.
Spacing="1"> | |
Spacing="1" | |
Padding="20"> |
Copilot uses AI. Check for mistakes.
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
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 pull request adds support for the
Fill
property to theBoxView
control, allowing it to be filled with anyBrush
(including gradients), not just a solid color. It introduces the necessary property, binding, event handling, and updates to tests and sample code to demonstrate and validate the new functionality.The most important changes are:
BoxView Fill Property Implementation:
FillProperty
and correspondingFill
getter/setter toBoxView
, enabling the use of anyBrush
(such as gradients) to fill the box. The property includes logic to manage event subscriptions for brush changes and resource updates. (src/Controls/src/Core/BoxView/BoxView.cs
) [1] [2] [3] [4]IShapeView.Fill
property to prioritize the newFill
property over the existingColor
property. (src/Controls/src/Core/BoxView/BoxView.cs
)Event Handling and Resource Management:
WeakBrushChangedProxy
helper class to manage weak event subscriptions for brush property changes, ensuring proper resource cleanup and preventing memory leaks. (src/Controls/src/Core/Internals/WeakEventProxy.cs
, moved fromsrc/Controls/src/Core/Shapes/Shape.cs
) [1] [2]Fill
property changes, and cleaned up in theBoxView
destructor. (src/Controls/src/Core/BoxView/BoxView.cs
) [1] [2]Public API Updates:
Fill
property and its accessors in the public API files for all supported platforms. [1] [2] [3] [4] [5] [6]Test and Sample Enhancements:
Fill
property, including options for solid, linear, and radial gradient fills, and corresponding UI controls. (src/Controls/tests/TestCases.HostApp/FeatureMatrix/BoxView/BoxViewControlPage.xaml
,src/Controls/tests/TestCases.HostApp/FeatureMatrix/BoxView/BoxViewViewModel.cs
) [1] [2] [3] [4] [5]Code Cleanup:
System.ComponentModel
import and relocated theWeakBrushChangedProxy
class fromShape.cs
to a more appropriate location. (src/Controls/src/Core/BoxView/BoxView.cs
,src/Controls/src/Core/Shapes/Shape.cs
) [1] [2]Issues Fixed
Fixes #2365