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

Reduce the complexity of the AttachAndRun code #19352

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Conversation

mattleibow
Copy link
Member

@mattleibow mattleibow commented Dec 12, 2023

Description of Change

Sometimes the Windows tests fail with no apparent reason, hopefully the slightly reduced complexit and more linear flow makes remote debugging easier. A specific change to first remove the window content before removing the view from the container might help.

Issues Fixed

Fixes #19015

Sometimes the Windows tests fail with no apparent reason, hopefully the
slightly reduced complexit and more linear flow makes remote debugging
easier. A specific change to first remove the window content before
removing the view from the container might help.
@mattleibow mattleibow requested a review from a team as a code owner December 12, 2023 00:35
if (window.Content is not null)
throw new Exception("The window retrieved from the service is already attached to existing content");

window.Content = new Grid
Copy link
Member

@PureWeen PureWeen Dec 12, 2023

Choose a reason for hiding this comment

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

Do we want this to be a Grid? Would it make sense for this to be a ContentPanel or a a custom panel? That way we can call into the IView.Measure and IView.Arrange calls?

Which should let us remove calls like this
https://github.com/dotnet/maui/blob/main/src/Controls/tests/DeviceTests/Elements/Frame/FrameTests.cs#L438-L440

Which are now happening outside the natural layout cycle

I kicked off a lazy version of this idea here #19353 to see if it helps anything.

Copy link
Member Author

@mattleibow mattleibow Dec 12, 2023

Choose a reason for hiding this comment

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

I think I am using a double Grid because the first grid is the root, and then the second one keeps the view at the size it wants and centers. Maybe the inner grid should be the ContentPanel? I'll have a look when I wake up because all your points sound very valid but my brain says ZZZZZZ

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow mattleibow mentioned this pull request Dec 12, 2023
@rmarinho rmarinho merged commit 6d4395e into main Dec 12, 2023
47 checks passed
@rmarinho rmarinho deleted the dev/win-frame-tests branch December 12, 2023 12:18
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tests] The windows Frame device tests fail with : No installed components were detected.
4 participants