Skip to content

feat(animation): support animating width/height properties #5147

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 6 commits into from
May 21, 2019

Conversation

vchimev
Copy link
Contributor

@vchimev vchimev commented Dec 7, 2017

Original PR #4917 by @justindujardin that implements #1764.

@vchimev
Copy link
Contributor Author

vchimev commented Mar 16, 2018

Hello All,

Great work on this pull-request!

Our main concerns with this implementation are related to performance. I am afraid that we have not had the time to measure and investigate it due to higher priority tasks. However, we are aware of its importance and wide demand, that is why we would like to invest further efforts and improve this feature as well as the animations at all.

This has not been scheduled for release yet. Currently, we are working on the 4.1 roadmap, so I hope we could get it there. Please follow the issue and this pull-request for any comments and updates.

@vchimev vchimev removed their assignment May 29, 2018
@MisterBrownRSA
Copy link

When are you planning on giving this Pull Request some daylight?

@farfromrefug
Copy link
Collaborator

@vchimev i have read through the PR and I dont really see anything that would impact performances if not using width or height in into animations. I understand you want to improve the whole animation system but in the meantime it would still be very useful to get that PR in.
And it would allow us not to use tricks like rxjs or tweenjs which might have worst performance consequences.

@Jonarod
Copy link

Jonarod commented Feb 21, 2019

Hey !!
What is to be done for this to be released ???
I can see some UI tests were not successful, however seems like the page hosting details is down... so it is not possible to check where it fails.

@vchimev could you bring some details back ?

Also, I agree with @farfromrefug here, as of today we use funky workarounds like Tween.js to animate width and height, for what I understand from @justindujardin 's code is that it should not be worse than todays hacks... Maybe you could release that PR with some beta warnings like to use with caution - still in beta or whatever: the community will surely help to stabilize it.

I can totally understand concerns (plus I can't see why tests fail...) but the PR is over 1 year old now 😅

Thanks !!

@bgrand-ch
Copy link

Hello! Thank you very much for your improvements to NativeScript. This feature is essential for many developers, could you implement it please? Would you have a planned date?

@ghost ghost added the in progress label May 7, 2019
@cla-bot cla-bot bot removed the in progress label May 8, 2019
@ghost ghost added the in progress label May 8, 2019
@NativeScript NativeScript deleted a comment from cla-bot bot May 8, 2019
@NativeScript NativeScript deleted a comment from cla-bot bot May 8, 2019
@NativeScript NativeScript deleted a comment from cla-bot bot May 8, 2019
@cla-bot cla-bot bot removed the in progress label May 9, 2019
@ghost ghost added the in progress label May 9, 2019
@NativeScript NativeScript deleted a comment from cla-bot bot May 9, 2019
@cla-bot cla-bot bot removed the in progress label May 9, 2019
@ghost ghost added the in progress label May 9, 2019
@NativeScript NativeScript deleted a comment from cla-bot bot May 9, 2019
@cla-bot cla-bot bot removed the in progress label May 21, 2019
@NativeScript NativeScript deleted a comment from cla-bot bot May 21, 2019
@MartoYankov MartoYankov added e2e test needed docs needed Additional documentation on this issue/PR is needed labels May 21, 2019
@SvetoslavTsenov SvetoslavTsenov merged commit e7c575e into master May 21, 2019
@SvetoslavTsenov SvetoslavTsenov deleted the animate-width-height branch May 21, 2019 14:00
@jbarnabe
Copy link

Thanks @MartoYankov and @SvetoslavTsenov for this improvement! Looking forward to use it.

@danielnitu
Copy link

Is this scheduled for future releases? I'm not able to use it in 5.4.3 and couldn't find more details elsewhere

@MartoYankov
Copy link
Contributor

@danielnitu Sorry for the late reply. This will be available in the 6.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs needed Additional documentation on this issue/PR is needed e2e test needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants