Skip to content

Conversation

@albyrock87
Copy link
Contributor

Description of Change

#26629 removed SetNeedsLayout propagation via inheritance, this was needed to fix #24996.
Unfortunately, this also removed the ability to intercept and stop propagation as needed.

A good option here would be to make IPlatformMeasureInvalidationController public and change void InvalidateMeasure(bool isPropagating = false); return type to bool, so that anyone could stop propagation in case of need.
This seems to me the best and clean option, but unless MAUI team makes an exception, we cannot touch PublicAPI in a Service Release.

For such reason, this PR brings back SetNeedsLayout propagation, but it also includes a way to tell if the SetNeedsLayout is being triggered by MAUI so we can avoid the issue described in #24996.

Issues Fixed

Fixes #28410

@albyrock87 albyrock87 requested a review from a team as a code owner March 16, 2025 13:41
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Mar 16, 2025
@dotnet-policy-service
Copy link
Contributor

Hey there @albyrock87! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@albyrock87 albyrock87 force-pushed the back-to-setneedslayout-inheritance-propagation branch from ec246ac to 80abb6e Compare March 16, 2025 14:52
@albyrock87 albyrock87 force-pushed the back-to-setneedslayout-inheritance-propagation branch from 80abb6e to d61577c Compare March 16, 2025 14:55
@jsuarezruiz
Copy link
Contributor

/azp run

@jsuarezruiz jsuarezruiz added platform/ios area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter labels Mar 17, 2025
@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. However, I think the previous changes have sense but just enabling a way to stop the propagation like InvalidateMeasure(bool isPropagating = false); So, let's create a new Spec for .NET 10 and you could work on it if want.

@albyrock87
Copy link
Contributor Author

albyrock87 commented Mar 17, 2025

@jsuarezruiz if creating an [Experimental] Public API with bool InvalidateMeasure(bool isPropagating = false); is not acceptable in .NET 9 then I can probably refactor this a bit more so that we can include this signature and let SetNeedsLayout invoke it so that creating the PR on .NET 10 will be a no-brainer.

This will also spare me some refactoring on #28225

Something like:

public override void SetNeedsLayout()
{
    if (IPlatformMeasureInvalidationController.IsInvalidating)
    {
        // TODO: NET10 move propagation loop back to iOS `ViewExtensions`
        var controller = (IPlatformMeasureInvalidationController)this;
        if (controller.InvalidateMeasure(IPlatformMeasureInvalidationController.IsPropagatingInvalidation))
        {
	    this.InvalidateParentMeasure();
        }
    } else {
        base.SetNeedsLayout();
    }
}

bool IPlatformMeasureInvalidationController.InvalidateMeasure(bool isPropagating)
{
    base.SetNeedsLayout();
    return true;
}

@PureWeen PureWeen added the do-not-merge Don't merge this PR label Mar 18, 2025
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still chatting with Alberto

@albyrock87
Copy link
Contributor Author

Not needed anymore. Let's try to evolve on the existing implementation.

@albyrock87 albyrock87 closed this Mar 23, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter community ✨ Community Contribution do-not-merge Don't merge this PR platform/ios

Projects

None yet

3 participants