Revert Applying visibility change to child controls#28768
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR reverts a previous change that affected the propagation of visibility to child controls. The key changes include:
- Reverting assertions and behavior related to visibility propagation in both test and production code.
- Removing tests that validate the propagation of the IsVisible property.
- Simplifying the VisualElement implementation by eliminating custom propagation logic.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/tests/Xaml.UnitTests/Issues/Issue1549.cs | Added an assertion on InvertBoolenConverter.count to reflect the reverted behavior |
| src/Controls/tests/Core.UnitTests/VisualElementTests.cs | Removed tests validating visibility propagation to child controls |
| src/Controls/src/Core/VisualElement/VisualElement.cs | Removed property propagation calls and helper methods related to IsVisible |
Comments suppressed due to low confidence (3)
src/Controls/tests/Xaml.UnitTests/Issues/Issue1549.cs:192
- Please confirm that the assertion expecting InvertBoolenConverter.count to be 4 is stable given the revert; if this value is based on prior behavior, a brief code comment explaining the expected count would help avoid future confusion.
Assert.AreEqual(4, InvertBoolenConverter.count);
src/Controls/tests/Core.UnitTests/VisualElementTests.cs:317
- The tests for propagating visibility to child controls have been removed as part of the revert; please ensure that equivalent coverage is maintained in TestCases.HostApp and TestCases.Shared.Tests if this behavior is revisited in future releases.
[Fact] public void ShouldPropagateVisibilityToChildren() { ... }
src/Controls/src/Core/VisualElement/VisualElement.cs:1501
- The removal of the IsVisible property propagation call in OnIsVisibleChanged aligns with the revert; please double-check that this change does not disrupt any internal state updates expected by the existing test suite.
(this as IPropertyPropagationController)?.PropagatePropertyChanged(IsVisibleProperty.PropertyName);
Member
|
/rebase |
3036baf to
c3643ae
Compare
Member
|
/backport to release/9.0.1xx-sr5 |
Contributor
|
Started backporting to release/9.0.1xx-sr5: https://github.com/dotnet/maui/actions/runs/14342767821 |
Member
|
Contributor
|
@jfversluis Since the original PR was reverted, should the bugs it was meant to fix be reopened? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
Reverts #20154 since it caused unexpected behavior for a lot of folks.
Reverting now and then improve on this and try again at a later stage.
Fixes #28677
Fixes #28415
cc: @kubaflo