Initial work on adding remaining animations #1232
Conversation
| await animation.Animate(label); | ||
| stopwatch.Stop(); | ||
|
|
||
| stopwatch.ElapsedMilliseconds.Should().BeCloseTo(animation.Length, 50); |
There was a problem hiding this comment.
can we get this value from some constant in Animation from library code?
There was a problem hiding this comment.
Yep happy to stick that somewhere more central
There was a problem hiding this comment.
Since we have different default duration for each animation, should this value be the same as the animation duration we pass to BaseAnimation?
There was a problem hiding this comment.
This might highlight that the code doesn't read well. the Length property of the BaseAnimation is that value we pass into the constructor. The 50 is my attempt at verifying that the completed duration was within 50 milliseconds of the expected duration. Perhaps this could be a calculated percentage instead. I still need to find a reliable way of verifying this because running the tests locally don't yield positive test results for all yet.
ghost
added
stale
|
Will this PR resolve the animation doubled duration issue? #1231 |
|
Yes it will |
|
I believe I have already addressed that issue in this PR I'm just struggling to get the unit test to prove it |
| { | ||
| ArgumentNullException.ThrowIfNull(view); | ||
|
|
||
| var duration = (uint)(Length / Math.Ceiling(StartFactor / ReducingAmount)) / 2; |
There was a problem hiding this comment.
I must be missing something obvious but I can't work out why this fails when being tested. The current values mean there are 3 shakes crossing the x == 0 point twice. This should result in 6 awaits however the test results in a failure because the animation completes within roughly 200ms. Given the above I calculate with a Length == 300, duration should equal 50 and therefore result in a passing test. However it doesn't appear to work
|
As far as I can tell it might be down to our use of |
There was a problem hiding this comment.
Copilot reviewed 5 out of 16 changed files in this pull request and generated 2 suggestions.
Files not reviewed (11)
- samples/CommunityToolkit.Maui.Sample/Pages/Behaviors/AnimationBehaviorPage.xaml: Language not supported
- src/CommunityToolkit.Maui/CommunityToolkit.Maui.csproj: Language not supported
- src/CommunityToolkit.Maui/Animations/ShakeAnimation.shared.cs: Evaluated as low risk
- src/CommunityToolkit.Maui.UnitTests/Animations/FadeAnimationTests.cs: Evaluated as low risk
- src/CommunityToolkit.Maui.UnitTests/Animations/FlipHorizontalAnimationTests.cs: Evaluated as low risk
- src/CommunityToolkit.Maui.UnitTests/Animations/FlipVerticalAnimationTests.cs: Evaluated as low risk
- src/CommunityToolkit.Maui.UnitTests/Animations/RotateAnimationTests.cs: Evaluated as low risk
- src/CommunityToolkit.Maui.UnitTests/Animations/ScaleAnimationTests.cs: Evaluated as low risk
- src/CommunityToolkit.Maui.UnitTests/Animations/BaseAnimationTests.cs: Evaluated as low risk
- src/CommunityToolkit.Maui/Animations/BaseAnimation.shared.cs: Evaluated as low risk
- src/CommunityToolkit.Maui/Animations/FadeAnimation.shared.cs: Evaluated as low risk
Co-authored-by: Copilot <[email protected]>
Description of Change
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information