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

Change iOS SetNeedsLayout propagation mechanism #26629

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

albyrock87
Copy link
Contributor

@albyrock87 albyrock87 commented Dec 14, 2024

Description of Change

I've been asked to split #25664 into separate PR.

This is the first piece which aims to change the way we propagate SetNeedsLayout by doing that only when MAUI commands.

As seen in the linked issue, there are situations where iOS triggers SetNeedsLayout natively and we don't want to propagate those.

This enables us to receive a SetNeedsLayout in TemplatedCell which will allow us to resize the cell without relaying on MeasureInvalidated cross platform event propagation.
There will be a follow-up PR to tackle this.

Issues Fixed

Fixes #24996

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 14, 2024
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.

@@ -147,7 +147,10 @@ await CreateHandlerAndAddToWindow<CarouselViewHandler>(carouselView, async (hand
Assert.False(data.IsCollectionChangedEventEmpty);
});

carouselView.Handler?.DisconnectHandler();
await InvokeOnMainThreadAsync(() =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this change could be a separate PR. I suppose that DisconnectHandler should always run on the main thread (or not?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a device test which randomly fails due to the fact it may run on a background thread. I came across it and it was a quick fix to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

So that's a great PR-stuff actually.

(In fact, if there is a way to detect it, it might probably fix a class of bugs.)

@albyrock87 albyrock87 force-pushed the ios-set-neesd-layout-propagation-mapper branch from 2ad0ab2 to 5b5e4b9 Compare December 14, 2024 11:54
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community ✨ Community Contribution
Projects
None yet
3 participants