Skip to content

Conversation

pekspro
Copy link
Contributor

@pekspro pekspro commented Sep 27, 2016

When you swipe a SlidableListItem passed the ActivationWidth threshold the icon and text “jumps” close to the margin. This PR introduces a tiny animation to make this jump a bit smoother.

And some types in the comments is also fixed.

@msftclas
Copy link

Hi @pekspro, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

{
_leftCommandTransform = _leftCommandPanel.RenderTransform as CompositeTransform;

_leftCommandAnimation = new DoubleAnimation();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use our animation library ? like await _leftCommandAnimation.Offset...

Copy link
Contributor Author

@pekspro pekspro Sep 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea :-). From what I can see it is not possible to cancel an animation in the animation library. And that will be an issue for instance if user passes above the threshold, which starts the animation, and then immediately goes below the threshold again. Then the animation needs to be cancelled and text and icon show be set on a fixed position. This is covered in the current PR, not sure how to do that with the library.

I’m also working with another tiny PR with another animation that uses clipping which I hope gets accepted. And as I understand it this is not supported in the animation library. I think it’s a good idea all animation is created in the same way in this control.

That said, I’m willing to test this if you think this is a good idea :-).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @nmetulev

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Animation can be cancelled like this:
var anim = foo.offset(0, 5);

anim.Start();
...
anim.Stop()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding clipping...as this is not yet supported by the toolkit itself, I'm fine doing it using sb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the animations have been updated, and I like it :-). One notable thing is that the RenderTransform need to be reset every time Stop is called, since the AnimationSet changes this when the animation is created.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seriously, isn't that cool right now? :)
Love it!

@deltakosh deltakosh added this to the v1.1 milestone Sep 27, 2016
@deltakosh
Copy link
Contributor

can you please fix the conflicts? I'll merge then

@pekspro
Copy link
Contributor Author

pekspro commented Sep 29, 2016

Now all conflicts have been fixed.

@nmetulev
Copy link
Contributor

Hey @pekspro, before we merge this, we made a fix on AnimationSet.Stop that might break your usage, I'd wait to merge until that PR( #418 ) has been resolved and this tested.

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking the merge until #418 has merged

{
// This will cover extrem cases when previous state wasn't
// below threshold.
_leftCommandAnimationSet?.Stop();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to wait for #418 to merge and retest before merging

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just verified the change did not break this.

@deltakosh deltakosh merged commit 70454a8 into CommunityToolkit:dev Sep 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants