Feature/animation samples #1527

bijington · 2021-07-21T22:13:19Z

Description of Change

This is very much a work in progress but it is here to show some progress on the pre-built animations along with a samples section.

Screen.Recording.2021-07-21.at.21.34.56.mov

Bugs Fixed

Fixes [Enhancement] More complete set of animations #1447

API Changes

Behavioral Changes

PR Checklist

Has a linked Issue, and the Issue has been approved
Has tests (if omitted, state reason in description)
Has samples (if omitted, state reason in description)
Rebased on top of main at time of PR
Changes adhere to coding standard

Updated documentation

bijington · 2021-07-22T14:56:08Z

src/CommunityToolkit/Xamarin.CommunityToolkit/Animations/TadaAnimation.shared.cs


 namespace Xamarin.CommunityToolkit.Animations
 {
    public class TadaAnimation : AnimationBase


If happy with the approach in TadaAnimationType this entire class and folder can disappear

pictos

@bijington sorry for the delay

pictos · 2021-07-26T22:47:19Z

...Toolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationTypes/AnimationBase.shared.cs

+		protected virtual Animation CreateAnimation(params View[] views)
+		{
+			return new Animation();
+		}


shouldn't this be abstract?

Yes good spot!

pictos · 2021-07-26T22:53:16Z

...kit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationTypes/TadaAnimationType.shared.cs

+			return taskCompletionSource.Task;
+		}
+
+		protected override Animation CreateAnimation(params View[] views) => Create(RotationAngle, MinimumScale, MaximumScale, views);


Here we have a situation. If you do something like

public Test() { var tada = new TadaAnimationType(); tada.IsRepeated = true; tada.MaximumScale = 1; var lbl = new Label(); var entry = new Entry(); // This works with more than one view var animation = tada.CreateAnimation(16, null, null, lbl, entry); animation.Commit(); animation.Abort(); // This just works with one view tada.Animate(lbl); // Task }

This can lead to confusion about which method should I call? Just Animate or CreateAnimation?
Also, if I call Animate I can't Abort it.

So, if we want to use the BehaviorAnimation API, we need to make those animations simpler and drop the CreateAnimation, Commit, and Abort methods, which works for me.

If you see that those methods, at least Commit, and Abort, we can :

Try to add those methods in BehaviorAnimation API;

Create another API, to hold those animations.

Just to point out, that these animations are more complex than the animations that we have, so makes sense to be another implementation if the existing one doesn't fit well.

I completely agree on the confusion this creates. What do you think to keeping the BehaviorAnimation support but creating a second underlying type that BehaviorAnimation can use?

Currently we have AnimationBase, could this stay but then we create 2 abstract classes inheriting from this; SimpleAnimation and ComplexAnimation (or possibly AsyncAnimation and Animation). Then the BehaviorAnimation class could call them based on which is supplied.

Or an alternative could be to bend the complex to work in the existing approach by allowing a CancellationToken to be passed in for the aborting. This all feels a bit wrong considering non of the animations are actually async.

I guess there are good use cases for both async and non async still. I was going to suggest we could replace the async parts with implementing these directly in an on async manner: https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Core/ViewExtensions.cs but I am not sure that is a good idea now

# Conflicts: # src/CommunityToolkit/Xamarin.CommunityToolkit/Animations/AnimationBase.shared.cs # src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationTypes/TadaAnimationType.shared.cs

bijington · 2021-08-12T21:30:26Z

This PR now covers:

a new samples section in the XCT.Samples app for Animations that will allow a user to test out the animations
switched to only using a TPL based approach for invoking an animation (e.g. await new TadaAnimation().Animate(myView))
2 animations; Tada and Rubber band as per https://animate.style
xml documentation for the new animation parts

I need to think on how to test this stuff and I am also keen to write up the docs for this stuff which I think should be fairly safe now given the approach?

bijington · 2021-08-14T10:22:29Z

@pictos i have noticed that the other CI builds aren't running against this PR. I assume it is because it isn't going in to develop or main. Is this ok? I guess it will be run when we eventually merge to develop

pictos · 2021-08-14T18:34:04Z

@bijington you're correct. The CI will just run when we do a merge to main or develop

bijington · 2021-08-17T20:26:14Z

@pictos what do you think to adding a CancelCommand to the new PreBuildAnimation base class to allow for a developer to cancel a repeating animation? I can't decide if it covers any extra scenarios or is cleaner than just setting IsRepeating = false.

pictos · 2021-08-19T23:20:48Z

@pictos what do you think to adding a CancelCommand to the new PreBuildAnimation base class to allow for a developer to cancel a repeating animation? I can't decide if it covers any extra scenarios or is cleaner than just setting IsRepeating = false.

I would say that use the IsRepeating will be cleaner. Because that BP can be used in triggers, Bindings, converters, etc

bijington · 2021-09-14T17:17:19Z

@pictos is there anything you need from me in order to get this PR in? I need to write up some documentation but I assumed I could do this when we eventually merge in to develop?

pictos · 2021-09-15T00:17:14Z

@bijington thanks for you hard work, I'll update this branch and open the official PR ASAP.

bijington · 2021-09-15T11:49:11Z

@bijington thanks for you hard work, I'll update this branch and open the official PR ASAP.

Awesome. Thank you for this

* Initial attempt at an Animations sample section * Move all responsibility to AnimationTypes * Code tidy up * changed the null and empty verification to be more performant * Inverted `if` to make the code more readable * An attempt at async with cancellation * Remove the need for Task.Delay * Removed the Type suffix * Code tidy up and xml docs Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>

* Add tada animation (#1468) * Base class implementation for pre-built animations * TADA! Added the first animation * Max and Min scales for Tada * Added the RubberBand animation Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com> * Feature/animation samples (#1527) * Initial attempt at an Animations sample section * Move all responsibility to AnimationTypes * Code tidy up * changed the null and empty verification to be more performant * Inverted `if` to make the code more readable * An attempt at async with cancellation * Remove the need for Task.Delay * Removed the Type suffix * Code tidy up and xml docs Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com> * Consolidate AnimationViewModel, Remove Unnecessary Nullables * TaskExtensions tidy up * AnimationWrapper tidy up Remove default parameters * Rename to CompoundAnimationBase * Unit tests added around animations * Reversed the >= check to keep the constant on the right * Switched to TimeSpan * Disabled/removed inconsistent tests * No longer share a ticker instance * Clear PlatformServices after each animation test * Code tidy up * Remove the inconsistent asserts * Further test ticker changes - only use AsyncTicker when really needed. - extra protection around enabling - set and clear default ticker before/after each test * Add Missing Asserts * Update Unit Test NuGet Packages * Update for Nullable Co-authored-by: Shaun Lawrence <shaunrlawrence@gmail.com> Co-authored-by: Brandon Minnick <13558917+brminnick@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>

bijington added 3 commits July 21, 2021 23:10

Initial attempt at an Animations sample section

0e753fd

Move all responsibility to AnimationTypes

face88e

Code tidy up

edd5e65

bijington mentioned this pull request Jul 21, 2021

Feature/add tada animation #1526

Closed

6 tasks

bijington commented Jul 22, 2021

View reviewed changes

pictos added 2 commits July 26, 2021 19:40

changed the null and empty verification to be more performant

c1d363e

Inverted if to make the code more readable

557f836

pictos reviewed Jul 26, 2021

View reviewed changes

bijington added 5 commits July 28, 2021 20:45

An attempt at async with cancellation

6b0a529

Remove the need for Task.Delay

93fad83

Merge branch 'feature/one-async-base' into feature/animation-samples

9473519

# Conflicts: # src/CommunityToolkit/Xamarin.CommunityToolkit/Animations/AnimationBase.shared.cs # src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationTypes/TadaAnimationType.shared.cs

Removed the Type suffix

58b4e48

Code tidy up and xml docs

2b8b12e

bijington marked this pull request as ready for review August 12, 2021 21:23

pictos merged commit fbdaa90 into xamarin:pj/pre-built-animation Sep 15, 2021

Feature/animation samples #1527

Feature/animation samples #1527

Uh oh!

Conversation

bijington commented Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Bugs Fixed

API Changes

Behavioral Changes

PR Checklist

Uh oh!

bijington Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

pictos left a comment

Choose a reason for hiding this comment

Uh oh!

pictos Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

bijington Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

pictos Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

pictos Jul 26, 2021

Choose a reason for hiding this comment

Uh oh!

bijington Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

bijington Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

bijington commented Aug 12, 2021

Uh oh!

bijington commented Aug 14, 2021

Uh oh!

pictos commented Aug 14, 2021

Uh oh!

bijington commented Aug 17, 2021

Uh oh!

pictos commented Aug 19, 2021

Uh oh!

bijington commented Sep 14, 2021

Uh oh!

pictos commented Sep 15, 2021

Uh oh!

bijington commented Sep 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bijington commented Jul 21, 2021 •

edited

Loading