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

Fix HeadlessWindowImpl.Position does not persist new value #17103

Merged
merged 6 commits into from
Sep 28, 2024

Conversation

loyvsc
Copy link
Contributor

@loyvsc loyvsc commented Sep 23, 2024

What does the pull request do?

Fixes #17071.

What is the current behavior?

When I assert the Position of the after I have set it, it always returns (0, 0).

What is the updated/expected behavior with this PR?

Now Position of the Window persist new value.

Breaking changes

Implementation of the HeadlessWindowImpl.Move method.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0052032-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@cla-avalonia
Copy link
Collaborator

cla-avalonia commented Sep 23, 2024

  • All contributors have signed the CLA.

@maxkatz6
Copy link
Member

Thanks!
Can you add a simple unit test for this?
For existing examples, see https://github.com/AvaloniaUI/Avalonia/tree/master/tests/Avalonia.Headless.UnitTests
These test source files are reused for two projects https://github.com/AvaloniaUI/Avalonia/tree/master/tests/Avalonia.Headless.NUnit.UnitTests and https://github.com/AvaloniaUI/Avalonia/tree/master/tests/Avalonia.Headless.XUnit.UnitTests

@loyvsc
Copy link
Contributor Author

loyvsc commented Sep 23, 2024

@cla-avalonia agree

Copy link
Member

@maxkatz6 maxkatz6 left a comment

Choose a reason for hiding this comment

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

Thank you!

@maxkatz6
Copy link
Member

Ah, approved before checking CI.
It seems like Should_Click_Button_On_Window test is failing.
Will restart, but should be double checked.

@MrJul
Copy link
Member

MrJul commented Sep 25, 2024

The test failures are real.

The new test needs a Dispatcher.UIThread.RunJobs() at the end to run all jobs queued by Show() since it doesn't use any of the input methods that normally do this implicitly.

@maxkatz6 shouldn't we call RunJobs() automatically at the end of each test in the headless infrastructure, to make sure there's no pending jobs? I can PR that.

@maxkatz6
Copy link
Member

@MrJul it would make sense, yes.

@MrJul
Copy link
Member

MrJul commented Sep 26, 2024

Done for NUnit in #17140.
XUnit was already calling Dispatcher.RunJobs().

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0052209-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 merged commit 981f47f into AvaloniaUI:master Sep 28, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Headless window implementation does not persist Position property
5 participants