-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Standardize view animation duration #3038
Comments
Hi @Stypox , |
I vote for 420 ms 🤣 If you want to find all magic numbers in our code that aren't a constant, you could uncomment https://github.com/TeamNewPipe/NewPipe/blob/dev/checkstyle.xml#L148, see https://checkstyle.org/config_coding.html#MagicNumber. We should reduce the amount of magic numbers in general. |
250 seems good to me :-D |
I also vote for 250ms :D, probably have the issue fixed this weekend. |
Hello, we are 7 students majoring in Computer Science from IMT Atlantique in France. We are willing to do some contributions to this project. We work together in this fork, MohammedAymane/NewPipe. Because the pull request of @ludugz were closed, could we take this issue? Looking forward to your reponse :) |
@Novmbrain Thank your for taking a look at this issue. Are your contributions part of a seminar or course? If yes, please add some info on how quickly you need feedback to the PR description :) |
Hello, Yes! Our contributions are part of a course and also the main point of this course. We can give you this first feedback 02/11. |
Hello @TobiGr ! We are back Secondly, wee found that ludugz defined two constants in AnimationsUtils
Due a long time has passed since 16/04/2020, the source code is different that before. It seems to us that the class "AnimationsUtils" has been removed. We also want to defines two constants to replace all magic numbers, however magics numbers exist in plusieurs Classes such as LocalPlaylistFragment, LocalPlaylistStreamItemHolder etc. So we don't know Where we should define these two constants. Thanks :) |
@Novmbrain all of the functions from |
I can see that the previous PR has been closed without merge. If this issue is still active and accepting contributions, I would love to work on it. |
@ChristoJobyAntony the UI is being rewritten and the View animation code is not going to be used anymore, so it may be better if you work on something else, e.g. #11603 |
If you check the usages of
animateView
you will see that the typical range will be 0 all the way to 500, with most in the 100-250 range.It should've been done with a default value in the first place. It is pretty much arbitrary right now, it was whatever the person felt like at the time of writing.
I'll leave this task to another PR, which the goal will be to standardize the used duration of animations.
_Originally posted by @mauriciocolli in #2309 (comment)
The text was updated successfully, but these errors were encountered: