Skip to content

Fix animated miss implemented for android #552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 27, 2023

Conversation

kefahB
Copy link
Contributor

@kefahB kefahB commented Nov 25, 2023

Hi @NathanWalker,

The animated property was miss implemented for android, the circle is animated even if we sat animated = false and make the animation restarted for each iteration because of the continuous update of the progress property, then the last one is animated but too lately.

@cla-bot cla-bot bot added the cla: yes label Nov 25, 2023
Copy link
Contributor

@CatchABus CatchABus left a comment

Choose a reason for hiding this comment

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

We'll also need a check inside initNativeView method.

Copy link
Contributor

@CatchABus CatchABus left a comment

Choose a reason for hiding this comment

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

With a few tweaks, Property declaration shall be good.

Remove old getter and setter
Copy link
Contributor

@CatchABus CatchABus left a comment

Choose a reason for hiding this comment

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

Types will help prevent possible errors in case of refactoring class in the future.

@CatchABus
Copy link
Contributor

@kefahB Let me also point out this line: https://github.com/NativeScript/plugins/blob/main/packages/animated-circle/index.android.ts#L42
You might have to check animated flag there too.

@NathanWalker
Copy link
Contributor

Thanks @CatchABus I'll merge and fix up last condition within initNativeView there, thanks @kefahB for effort here!

@NathanWalker NathanWalker merged commit 1fb7e70 into NativeScript:main Nov 27, 2023
@NathanWalker
Copy link
Contributor

Published in 1.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants