Added AnimationBuilder.Start(UIElement, Action) overload

[Sergio0694]

Addition to #3639

PR Type

What kind of change does this PR introduce?

• Feature

What is the current behavior?

There is no synchronous overload for AnimationBuilder.Start taking a completion callback.

What is the new behavior?

This PR adds a new AnimationBuilder.Start(UIElement, Action) overload, with an efficient implementation not using an async state machine to schedule and invoke the callback when the animation group(s) complete.

Example usage

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link:
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

[ghost]

Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[Rosuavio]

@Sergio0694 I think this is a really important addition to the API, but maybe some tests coverage would be nice on this. Maybe something simple that ensures that the callback is called after all animations it depends on. Just to make sure this does not break in the future.

[Sergio0694]

@RosarioPulella Yeah that's good advice, I've just tested this new API locally for now.
I'll see if I can add some tests for the AnimationBuilder.Start___ overloads in the UITests project 🙂

[Sergio0694]

@RosarioPulella Do you know if there's a proper way to handle asynchronous tests with the MUX test infrastructure?
I used the other sample test as a reference and made a simple test page for AnimationBuilder, but the test is failing and I'm not sure if that's due to the assertion not actually waiting for the animation to complete. I tried both Wait.ForIdle and Wait.ForSeconds(1), but had no luck with that. The debugger doesn't work either in the UWP test app, so I can't use that to verify that, but I'm not seeing why the test itself shouldn't be executing correctly. Is there some documentation for this new test infrastructure you could point me to? Or, ideas on what I'm doing wrong here? 🤔

Thanks! 🙂

[Rosuavio]

Is there some documentation for this new test infrastructure you could point me to?

I don't believe there is any documentation as this area new. I can take a look on whats going wrong though, maybe this experience will help me formulate some docs, or improve this test infra structure.

[michael-hawker]

@Sergio0694 you're talking about the Integration Tests (UITest) app vs. the UnitTest (and the VisualUITestBase), eh?

[Sergio0694]

@michael-hawker Correct. But now that you mention it, I double-checked the UnitTest project and remembered that we also have the ability to set a window content there to test things actually loaded in the tree (for some reason I was thinking of all the tests only using XamlLoader locally and thought we couldn't put couldn't put controls in the actual window from there, and that we had to use the UITest project for that, my bad 😅 ). I'll move my test to the UnitTest project tomorrow and just use the current infrastructure there, it's very possible that the issue I was having will be gone there, as we already have a whole bunch of other tests there for other controls that work without issues. Thanks, sorry for the confusion! 🙂

[Sergio0694]

Alright, removed that previous commit and added new unit tests in 2aae986.
Thanks @michael-hawker for pointing me to the right unit test project 😄
@RosarioPulella Let me know if those are alright!

[michael-hawker]

@Sergio0694 no problem. I was in the middle of writing docs for the tests before we shipped, so I should finish those.

The VisualUITestBase certainly should solve the majority of UI testing needs I believe. The integration tests I think will be more for E2E scenarios.

[Rosuavio]

Looks good!

[michael-hawker]

@Sergio0694 seems great, thanks! Thanks for adding detailed comments and tests!

Should other animation helpers have more unit tests?? 😉