Skip to content
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

VisualElement.OnMeasure currently routes to some inconsistent logic compared to OnMeasureOverride #23244

Closed
PureWeen opened this issue Jun 25, 2024 · 3 comments
Labels
area-controls-general General issues that span multiple controls, or common base classes such as View or Element s/triaged Issue has been reviewed t/bug Something isn't working
Milestone

Comments

@PureWeen
Copy link
Member

PureWeen commented Jun 25, 2024

Description

VE.OnMeasure follows this path

image

Which calls this

image

It should probably call this in both places of the OnMeasure override

image

The checks on the constraints of "0" also is going to cause inconsistent behavior. MAUI treats zero as zero.

We should review all the measure code inside VisualElement and just force everything to route to the MAUI paths.

Right now we hit a lot of inconsistent behavior because

button.Measure
and
(button as IView).Measure

Do different things.

we should make it so those both do the same thing and then if users need to switch back to old behavior they can via a switch.

@dotnet-policy-service dotnet-policy-service bot added the s/triaged Issue has been reviewed label Jun 25, 2024
@PureWeen PureWeen added this to the 9.0-preview7 milestone Jun 25, 2024
@PureWeen PureWeen added the area-controls-general General issues that span multiple controls, or common base classes such as View or Element label Jun 28, 2024
@samhouts samhouts removed the s/triaged Issue has been reviewed label Jul 3, 2024
@samhouts samhouts added the s/triaged Issue has been reviewed label Jul 10, 2024
@mattleibow
Copy link
Member

This is the new logic in OnMeasure after my PR #23725:

image

@mattleibow
Copy link
Member

Why do we have both, and why does the old Measure -> OnMeasure do a whole bunch of min/max checks where the new one just returns the size?

They seem to be very different. IView.Measure -> MeasureOveride is just the desired size, but the old one is trying to do more. Do we even use the old path?

@PureWeen PureWeen modified the milestones: 9.0-preview7, 9.0-rc1 Jul 26, 2024
@PureWeen PureWeen modified the milestones: 9.0-rc1, 9.0-preview7 Jul 31, 2024
@samhouts samhouts added the t/bug Something isn't working label Jul 31, 2024
@bradencohen
Copy link
Contributor

Can this also be fixed for Arrange?: #17417

@PureWeen PureWeen closed this as completed Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-general General issues that span multiple controls, or common base classes such as View or Element s/triaged Issue has been reviewed t/bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

4 participants