Applying visibility change to child controls#20154
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. |
jsuarezruiz
left a comment
There was a problem hiding this comment.
@tj-devel709 Could we include a device or ui tests here?
rmarinho
left a comment
There was a problem hiding this comment.
This will need more description about the change and maybe some discussion first. I m not sure we want to add this to our public API.
Also this would need device and unit tests for the new api
|
@rmarinho there was a short discussion in this issue #19139 I will write tests once there is a decision on whether this change makes sense or not. |
|
I added this PR here to address specifically with the next entry capability. I don't think we will need this PR (in regards to the next entry issue), but it could possibly be helpful for a different scenario! |
|
This PR seems sensible, but both WPF and UWP chose not to propagate this value. So I am asking some teams why they decided this: dotnet/wpf#8774 |
mattleibow
left a comment
There was a problem hiding this comment.
Looking good, we just need a bunch more tests for a good few combinations so we can ensure we don't break somerthing later on and all things stay working as expected.
|
Can you rebase? |
90d5d80 to
1b85ac4
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Now the question is... Is this a breaking change for a SR that means it should wait for net9.0? Or is it OK and people need to be aware of the change? |
b4370b4 to
549cd74
Compare
|
@kubaflo Could you rebase and fix the conflicts?. Thanks. |
549cd74 to
5de6439
Compare
|
@jsuarezruiz done :) |
|
/azp run |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
98aeecd to
60d1f1f
Compare
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
|
60d1f1f to
51728af
Compare
|
@jfversluis Because of how propagation works I had to modify the test |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@kubaflo |
|
I think, added |
Can you provide a sample so that I can try to reproduce it? Thanks! |
@mattleibow @kubaflo I think this is causing me issues in 9.0.50, we have some layout logic that depends on knowing the actually Set value of IsVisible, like the original behavior. I don't see how I can get the _isVisibleExplicit value even after the .net 10 changes |
Description of Change
Applying visibility change to child controls
Issues Fixed
Fixes #19139
Fixes #23573
Fixes #28012
Screen.Recording.2024-01-25.at.13.28.42.mov