-
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
Fillproperty toBoxViewthat accepts anyBrushtype - Implements proper event subscription and resource management for brush changes
- Updates the
IShapeView.Fillimplementation to prioritize the newFillproperty over the existingColorproperty
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 |
src/Controls/tests/TestCases.HostApp/FeatureMatrix/BoxView/BoxViewControlPage.xaml
Show resolved
Hide resolved
src/Controls/tests/TestCases.HostApp/FeatureMatrix/BoxView/BoxViewControlPage.xaml
Show resolved
Hide resolved
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jsuarezruiz
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.
@jsuarezruiz , I have added the pending snaps for android and mac platforms. There was a issue with windows platform , it does not have anything to do with this PR. it was related to shadow (#31821). Fill is working fine in windows also. attaching images for reference.
|
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jsuarezruiz
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.
@jsuarezruiz , yeah need to restrict this test case too. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| PathAspect IShapeView.Aspect => PathAspect.None; | ||
|
|
||
| Paint? IShapeView.Fill => Color?.AsPaint(); | ||
| Paint? IShapeView.Fill => Fill ?? Color?.AsPaint(); |
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 implementation allows both Color and Fill to be set simultaneously, with Fill taking priority. This requires updates in the documentation.
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.
Also, what happens when Fill is cleared?
<BoxView x:Name="box" Color="Red" Fill="{StaticResource MyGradient}" />
Later in code:
box.Fill = null;
Does Red color suddenly appear? Could we have a test?
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 , when fill is set to null , the color takes over. i have added test case to ensure this.
c84fd2d to
239f965
Compare







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
Fillproperty to theBoxViewcontrol, 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:
FillPropertyand correspondingFillgetter/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.Fillproperty to prioritize the newFillproperty over the existingColorproperty. (src/Controls/src/Core/BoxView/BoxView.cs)Event Handling and Resource Management:
WeakBrushChangedProxyhelper 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]Fillproperty changes, and cleaned up in theBoxViewdestructor. (src/Controls/src/Core/BoxView/BoxView.cs) [1] [2]Public API Updates:
Fillproperty and its accessors in the public API files for all supported platforms. [1] [2] [3] [4] [5] [6]Test and Sample Enhancements:
Fillproperty, 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.ComponentModelimport and relocated theWeakBrushChangedProxyclass fromShape.csto a more appropriate location. (src/Controls/src/Core/BoxView/BoxView.cs,src/Controls/src/Core/Shapes/Shape.cs) [1] [2]Issues Fixed
Fixes #2365