Skip to content

Standardize view animation duration #3038#3458

Closed
ludugz wants to merge 2 commits intoTeamNewPipe:devfrom
ludugz:standardize-view-animation
Closed

Standardize view animation duration #3038#3458
ludugz wants to merge 2 commits intoTeamNewPipe:devfrom
ludugz:standardize-view-animation

Conversation

@ludugz
Copy link

@ludugz ludugz commented Apr 19, 2020 •
edited
Loading

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Standardize view animation for 3 methods: animateView, showControls, hideControls
  • Replace VideoPlayer/DEFAULT_CONTROLS_DURATION with AnimationUtils/DEFAULT_SHORT_ANIM_DURATION ( For 0-500 ms in previous version)
  • Create constant DEFAULT_LONG_ANIM_DURATION (for 800ms & 1500ms in previous version)
  • Refactor code format some places

Fixes the following issue(s)

Testing apk

app-debug.zip

Agreement

@B0pol
Copy link
Member

B0pol commented Apr 19, 2020 •
edited
Loading

Pausing in main player feels so slow now. I don't think it should be that slow.
And there is no animation for popup player.

wb9688
wb9688 requested changes Apr 19, 2020
View reviewed changes
Copy link
Member

@wb9688 wb9688 left a comment

Choose a reason for hiding this comment

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

Please don't do refactoring unrelated to your PR.

app/src/main/java/org/schabi/newpipe/streams/Mp4FromDashWriter.java Outdated
Comment on lines 714 to 716
throw new RuntimeException("bad track matrix length (expected 36) in track n°" + i);
throw new RuntimeException("bad track matrix length (expected 36) in"
+ " track n°" + i);
Copy link
Member

@wb9688 wb9688 Apr 19, 2020

Choose a reason for hiding this comment

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

Why split it into 2 lines? That line isn't even too long…

Copy link
Collaborator

@XiangRongLin XiangRongLin Apr 19, 2020

Choose a reason for hiding this comment

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

My guess is that his build also fails like mine, because his system also sees it as 101 characters in a single line (because of the °, being recognized as 2 characters).

Copy link
Member

@wb9688 wb9688 Apr 19, 2020

Choose a reason for hiding this comment

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

@XiangRongLin: Then you have weird encoding settings and should fix that, not this line. That also still doesn't explain the other changes in this file.

Copy link
Author

@ludugz ludugz Apr 20, 2020

Choose a reason for hiding this comment

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

@XiangRongLin nailed it. Maybe something wrong with my Android Studio, I will return it back soon.

Copy link
Member

@wb9688 wb9688 Apr 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@ludugz: Could you also try whether #3460 fixes this? It looks like this is a stupid default setting for Windows with certain languages, so that PR overrides it for Gradle.

Copy link
Author

@ludugz ludugz Apr 20, 2020

Choose a reason for hiding this comment

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

@wb9688 It worked. Thank you!

ludugz added 2 commits April 20, 2020 19:34
@ludugz
- Add default value duration for 3 methods : animateView, showControl…
3972dbf 
…s, hideControls

- Move VideoPlayer/DEFAULT_CONTROLS_DURATION --> AnimationUtils/DEFAULT_ANIM_DURATION
- Refactor some places
@ludugz
Return back Mp4FromDashWriter.java because of checkStyle build error
dd7c206
@ludugz ludugz force-pushed the standardize-view-animation branch from cd0dec0 to dd7c206 Compare April 20, 2020 10:35
@TobiGr TobiGr force-pushed the dev branch 2 times, most recently from 679bc75 to 2aeccc0 Compare March 16, 2021 08:24
@litetex litetex marked this pull request as draft October 1, 2021 16:59
@litetex
Copy link
Member

litetex commented Oct 1, 2021

There is no progress since >1,5 years, a ton of merge conflicts and no GitHub actions build.

Closing this PR.
Feel free to reopen it when there is again some progress happening.

@litetex litetex closed this Oct 1, 2021
@litetex litetex mentioned this pull request Oct 10, 2021
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@XiangRongLin XiangRongLin XiangRongLin left review comments

@wb9688 wb9688 wb9688 requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ludugz @B0pol @litetex @XiangRongLin @wb9688