Add CancellationToken support to animation methods in ViewExtensions#33372
Add CancellationToken support to animation methods in ViewExtensions#33372StephaneDelcroix wants to merge 9 commits intonet11.0from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds CancellationToken support to all animation methods in ViewExtensions, allowing developers to cancel running animations programmatically. The changes replace existing animation method overloads with new versions that accept an optional CancellationToken parameter (defaulting to default). While this maintains source compatibility, it breaks binary compatibility as the old overloads without CancellationToken have been removed and marked with *REMOVED* in the PublicAPI files.
Key Changes
- Added
CancellationTokenparameter to 11 animation methods (FadeToAsync,LayoutToAsync,RelRotateToAsync, etc.) - Old overloads removed and marked as
*REMOVED*in PublicAPI tracking files - Removed duplicate
[Category]attributes from numerous UI test files for cleaner test organization - Multiple new test cases and test infrastructure improvements added
Reviewed changes
Copilot reviewed 284 out of 420 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/*.cs (100+ files) |
Removed duplicate [Category] attributes from UI tests (e.g., removed UITestCategories.Compatibility, UITestCategories.Navigation where tests already had more specific categories) |
src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/*.cs (40+ files) |
Refactored test classes to inherit from _GalleryUITest instead of UITest, added GalleryPageName property, removed manual FixtureSetup methods |
src/Controls/tests/TestCases.HostApp/Issues/*.cs/.xaml |
Added new test pages for issues (Issue28557, Issue28201, Issue22719, Issue13537, Issue33130, Issue30366) |
src/Controls/tests/TestCases.HostApp/MauiProgram.cs |
Added MacCatalyst-specific test startup logic to directly navigate to test pages via environment variable |
src/Controls/tests/TestCases.HostApp/CoreViews/CorePageView.cs |
Added TryToGetGalleryPage method to retrieve gallery pages by title |
src/Controls/tests/TestCases.HostApp/TestCases.cs |
Added TryToGetTestPage method and PageFactory property to support programmatic test page retrieval |
src/Controls/src/Xaml/SimplifyOnPlatformVisitor.cs |
Fixed OnPlatform with abstract types - marks nodes as IsOnPlatformDefaultValue when no platform matches |
src/Controls/src/SourceGen/Visitors/CreateValuesVisitor.cs |
Generates default(T) for OnPlatform nodes marked as IsOnPlatformDefaultValue instead of trying to instantiate abstract types |
src/Controls/tests/Core.UnitTests/ShellParameterPassingTests.cs |
Added tests verifying ApplyQueryAttributes is triggered during TabBar/FlyoutItem navigation (Issue #13537) |
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/*.cs |
Added tests for OnPlatform with abstract types, RelativeSource bindings, and RelayCommand support |
c344859 to
37affb5
Compare
| }.Commit(view, nameof(TranslateToAsync), 16, length, null, (f, a) => | ||
| { | ||
| registration.Dispose(); | ||
| tcs.SetResult(a); | ||
| }); |
There was a problem hiding this comment.
What happens to animations attached to visuals which are removed from the UI tree? Do they get finished/cancelled?
Newbie question: It just feels weird to attach lifetime to animation finishing. Also from a naming perspective, for example I would not be surprised if finish action didn't fire for cancelled animation (I know it does).
|
@copilot can you resolve the conflicts on this branch rebase on net11.0 ? |
|
/rebase |
ffe722b to
ba20da9
Compare
|
Just asking (I don't know our rules in MAUI around this): If there's a NuGet with animations code, this will effectively make it impossible to use that NuGet on .NET 11 - and it will force the NuGet to implement multi-targeting as there's no single binary which can be used on .NET 10 and .NET 11 at the same time. Personally, I don't see why we can't keep the existing overloads and let them just call the new ones without cancellation token. Yes, it makes the API bigger, but the above risk doesn't feel worth it. If we want to, we can mark the old APIs Obsolete and remove them once .NET 10 goes out of support. Additionally, this would allow us to ship this in .NET 10 SR and not have to wait for .NET 11. |
1f0432f to
ba277d2
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33372Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33372" |
ba277d2 to
80e022f
Compare
cd0c84f to
18147b5
Compare
|
|
- Replace existing *ToAsync animation methods with new overloads that accept CancellationToken - All parameters remain optional (length=250, easing=null, cancellationToken=default) - When cancelled, animations are properly aborted via AbortAnimation() - Remove old overloads without CancellationToken (breaking binary change) Affected methods: - FadeToAsync, LayoutToAsync, RelRotateToAsync, RelScaleToAsync - RotateToAsync, RotateXToAsync, RotateYToAsync - ScaleToAsync, ScaleXToAsync, ScaleYToAsync - TranslateToAsync BREAKING CHANGE: Binary compatibility is broken for existing code compiled against the old method signatures. Source compatibility is maintained due to optional default parameter.
- Test pre-canceled tokens return immediately for all animation methods - Test animations complete normally without cancellation - Cover FadeToAsync, RotateToAsync, RotateXToAsync, RotateYToAsync - Cover ScaleToAsync, ScaleXToAsync, ScaleYToAsync - Cover TranslateToAsync, RelRotateToAsync, RelScaleToAsync
Preserve original method signatures as forwarding overloads to maintain binary compatibility. Add new overloads with required CancellationToken parameter. Remove *REMOVED* entries from PublicAPI files since original signatures are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The entries with 'CancellationToken cancellationToken = default(...)' don't match any actual method signature. The real methods have CancellationToken as a required parameter without a default value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Wrap CancellationToken.Register callbacks in DispatchIfRequired to ensure AbortAnimation is called on the UI thread in both AnimateToAsync and TranslateToAsync, preventing cross-thread access to the animation system - Change tcs.SetResult to tcs.TrySetResult in the finished callback to prevent InvalidOperationException when CT fires concurrently with completion - Add TOCTOU guard after Commit() to handle tokens canceled between the Register and Commit calls - Add FadeToAsyncCanceledMidFlightReturnsCanceled test exercising in-flight cancellation with AsyncTicker Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use single-arg Prepare overload (matching all other tests in file). The prior two-arg form had handler/box variable names swapped, causing a compile error when using box as VisualElement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…TOCTOU guards - Replace fixed Task.Delay(64) in FadeToAsyncCanceledMidFlightReturnsCanceled with a polling loop (up to 5s) that waits until opacity actually decreases, eliminating timing-dependent flakiness on loaded CI machines - Wrap v.AbortAnimation() calls in TOCTOU guards (AnimateToAsync and TranslateToAsync) with DispatchIfRequired(), consistent with the cancellation callback path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7f90bac to
2edcb0f
Compare
…tionToken The rebase conflict resolution accidentally preserved a phantom entry with 'cancellationToken = default(CancellationToken)' that doesn't match any actual symbol, causing RS0017 warnings that fail the build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rameter The single-arg Prepare<T>(T view) method was hardcoded to create AnimationReadyHandler() (BlockingTicker) instead of using the TTicker generic parameter from the enclosing class. This caused FadeToAsyncCanceledMidFlightReturnsCanceled to fail on CI because the animation completed synchronously before the test could observe mid-flight state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run maui-pr |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
This PR adds
CancellationTokensupport to all animation methods inViewExtensions, allowing developers to cancel running animations programmatically.Changes
*ToAsyncanimation methods with new overloads that accept an optionalCancellationTokenparameterlength = 250,easing = null,cancellationToken = defaultCancellationTokenis cancelled, animations are properly aborted viaAbortAnimation()CancellationTokenAffected Methods
FadeToAsyncLayoutToAsyncRelRotateToAsyncRelScaleToAsyncRotateToAsyncRotateXToAsyncRotateYToAsyncScaleToAsyncScaleXToAsyncScaleYToAsyncTranslateToAsyncBinary compatibility is broken for existing code compiled against the old method signatures. The old overloads (without
CancellationToken) have been removed and are marked with*REMOVED*in the PublicAPI files.Source compatibility is maintained - existing source code will continue to compile without changes due to the optional default parameter value (
CancellationToken cancellationToken = default).Migration
No source code changes required. However, projects compiled against older versions of MAUI will need to be recompiled.
Usage Example
Testing