AnimationBehavior enhancements: #1180

bijington · 2021-04-07T22:06:32Z

Description of Change

Addition of AnimateCommand to the AnimationBehavior class which lets the animation be triggered from a read-only Command.

Bugs Fixed

Fixes [Enhancement] Provide ability to start Animations from a Command #1175

API Changes

Added:

ICommand AnimationBehavior.AnimateCommand { get; }

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 well develop this time
Changes adhere to coding standard

Updated documentation

- AnimateCommand now lets the animation be triggered from a Command

src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationBehavior.shared.cs

AndreiMisiukevich · 2021-04-07T22:34:22Z

src/CommunityToolkit/Xamarin.CommunityToolkit.UnitTests/Behaviors/AnimationBehavior_Tests.cs

+
+			behavior.AnimateCommand.Execute(null);
+
+			Assert.That(mockAnimation.HasAnimated, Is.True);


Assert.IsTrue(mockAnimation.HasAnimated)

?

Do you want me to resolve these now that I have actioned them?

AndreiMisiukevich · 2021-04-07T22:34:39Z

src/CommunityToolkit/Xamarin.CommunityToolkit.UnitTests/Behaviors/AnimationBehavior_Tests.cs

+			Assert.That(mockAnimation.HasAnimated, Is.True);
+		}
+
+		class MockAnimationType : AnimationBase


can you please move this class to Mocks folder?

AndreiMisiukevich · 2021-04-07T22:36:36Z

src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationBehavior.shared.cs

@@ -7,12 +10,25 @@ public class AnimationBehavior : EventToCommandBehavior
 		public static readonly BindableProperty AnimationTypeProperty =
 			BindableProperty.Create(nameof(AnimationType), typeof(AnimationBase), typeof(AnimationBehavior));

+		public static readonly BindablePropertyKey AnimateCommandPropertyKey =


should be internal

AndreiMisiukevich · 2021-04-07T22:37:09Z

src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationBehavior.shared.cs

+ 				nameof(AnimateCommand),
+ 				typeof(ICommand),
+ 				typeof(AnimationBehavior),
+ 				null,


null is fine, but also you may use "default" keyword

src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationBehavior.shared.cs

- tidy up based on code review comments

…t into develop

pictos · 2021-04-09T17:58:15Z

@bijington are you able to write the docs for this feature?

bijington · 2021-04-09T19:11:54Z

Yes of course. At first I thought there wasn't much at all for AnimationBehavior but I have found this which I should be able to update easily enough. https://github.com/MicrosoftDocs/xamarin-communitytoolkit/blob/master/docs/behaviors/animationbehavior.md

I will aim to fork and submit a PR to that repo over the weekend

AndreiMisiukevich · 2021-04-10T23:56:34Z

Could you please resolve conflicts?

bijington · 2021-04-11T13:29:16Z

@pictos the documentation PR is here: MicrosoftDocs/xamarin-communitytoolkit#115 I assume you want to wait for that to be merged before accepting this?

@AndreiMisiukevich merge conflicts have been resolved

pictos · 2021-04-11T17:22:14Z

@bijington can you please double-check your merge? Looks like something went wrong.

bijington · 2021-04-11T17:33:21Z

@bijington can you please double-check your merge? Looks like something went wrong.

Oh dear. I'm not sure what I've done. I thought I had just rebased off develop.

pictos · 2021-04-11T17:35:47Z

@bijington I'll see if I can fix it (:
No worries, git can be tricky sometimes

bijington · 2021-04-11T17:38:15Z

@pictos oh thank you! I'm happy to try and see if I can fix it first to not take up your time

pictos · 2021-04-11T17:44:34Z

@bijington try this, on your terminal type
git reset --soft HEAD~2, this will move your branch two commits backward. Unstage and delete the changes. Double check if your code is there, I believe they are just two commits AnimationBehavior: and AnimationBehavior enhancements:.

If the project builds and runs as expected, just do a force push

bijington · 2021-04-11T17:57:15Z

@pictos Great thanks. I will try that out in a little bit and tidy up my mess 🙂

bijington · 2021-04-11T19:46:14Z

@pictos I am a little confused now. I definitely want to keep the commit from Resolved merge conflicts as this brought in a unit test that was missed as part of the initial merge from develop. The bit that confuses me are the additional 102 file changes... these seem to map back to commit d9ef5f7 and PR #1185.

I am starting to think I should have branched off develop as it may have made merging in a little easier (at least given my limited experience with git).

pictos · 2021-04-11T19:55:15Z

@bijington Ooh, I see now, you didn't create another branch to do this fix... Yeah, this can cause some confusion... I would say to delete the Resolved merge conflicts because that test shouldn't be in your PR files changed.
SO my idea here is to move your branch to the point that you did the fix, and then do a merge or rebase against our develop branch, is that make sense for you?

bijington · 2021-04-11T20:11:48Z

@pictos thank you for the assistance, this latest set of changes looks much more like what you are expecting :).

Sorry about the mess I created. I will make sure to branch in future to simplify the merging

pictos · 2021-04-11T20:17:20Z

@pictos thank you for the assistance, this latest set of changes looks much more like what you are expecting :).

Sorry about the mess I created. I will make sure to branch in future to simplify the merging

No need to be sorry, as I said before git can be tricky sometimes. Glad that you were able to fix all :D
Anytime that you need help feel free to ping me ❤

pictos

@bijington thanks for this PR, I'm awaiting for the next ones❣

bijington · 2021-04-11T21:41:50Z

Thank you for accepting the change and all the help getting it merged! This one had a big benefit for me, I will try to see if I can find something else to help out on.

Keep up the great work! I have been surprised at how useful this toolkit has been for my project!

bijington and others added 2 commits April 7, 2021 23:03

AnimationBehavior enhancements:

9002529

- AnimateCommand now lets the animation be triggered from a Command

Merge branch 'develop' into develop

527c962

bijington mentioned this pull request Apr 7, 2021

AnimationBehavior enhancements #1179

Closed

6 tasks

bijington commented Apr 7, 2021

View reviewed changes

src/CommunityToolkit/Xamarin.CommunityToolkit/Behaviors/Animations/AnimationBehavior.shared.cs Outdated Show resolved Hide resolved

AndreiMisiukevich suggested changes Apr 7, 2021

View reviewed changes

bijington and others added 3 commits April 8, 2021 08:41

AnimationBehavior:

32e3a4f

- tidy up based on code review comments

Merge branch 'develop' of github.com:bijington/XamarinCommunityToolki…

bfad3cd

…t into develop

Merge branch 'develop' into develop

180a661

bijington requested a review from AndreiMisiukevich April 8, 2021 21:14

Merge branch 'develop' into develop

1591147

AndreiMisiukevich previously approved these changes Apr 8, 2021

View reviewed changes

pictos added a/animation a/behaviors This issue/PR is related to behaviors labels Apr 9, 2021

pictos added the needs-documentation This needs better documentation to be closed out. label Apr 9, 2021

bijington mentioned this pull request Apr 11, 2021

AnimateCommand detail added MicrosoftDocs/xamarin-communitytoolkit#115

Merged

4 tasks

bijington dismissed AndreiMisiukevich’s stale review via 8782336 April 11, 2021 13:27

pictos removed the needs-documentation This needs better documentation to be closed out. label Apr 11, 2021

Merge branch 'develop' into develop

9c35a7d

AndreiMisiukevich approved these changes Apr 11, 2021

View reviewed changes

pictos approved these changes Apr 11, 2021

View reviewed changes

pictos merged commit 7f41cfa into xamarin:develop Apr 11, 2021


		behavior.AnimateCommand.Execute(null);

		Assert.That(mockAnimation.HasAnimated, Is.True);

AnimationBehavior enhancements: #1180

AnimationBehavior enhancements: #1180

Uh oh!

Conversation

bijington commented Apr 7, 2021 • edited by pictos 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!

Uh oh!

AndreiMisiukevich Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

bijington Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

AndreiMisiukevich Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

AndreiMisiukevich Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

AndreiMisiukevich Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pictos commented Apr 9, 2021

Uh oh!

bijington commented Apr 9, 2021

Uh oh!

AndreiMisiukevich commented Apr 10, 2021

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

pictos commented Apr 11, 2021

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

pictos commented Apr 11, 2021

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

pictos commented Apr 11, 2021

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

pictos commented Apr 11, 2021

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

pictos commented Apr 11, 2021

Uh oh!

pictos left a comment

Choose a reason for hiding this comment

Uh oh!

bijington commented Apr 11, 2021

Uh oh!

Uh oh!

bijington commented Apr 7, 2021 •

edited by pictos

Loading