Skip to content
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

Add queue time #6023

Merged
merged 3 commits into from
Apr 8, 2021
Merged

Add queue time #6023

merged 3 commits into from
Apr 8, 2021

Conversation

Douile
Copy link
Contributor

@Douile Douile commented Apr 7, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Adds a text view to top of queue that displays the time viewed through the queue

screenshot of feature

Fixes the following issue(s)

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

@AudricV AudricV added feature request Issue is related to a feature in the app player Issues related to any player (main, popup and background) labels Apr 7, 2021
@Redirion
Copy link
Member

Redirion commented Apr 8, 2021

Code is looking well structured. Thank you.

Could you maybe add a bit of padding/margin so that the time has the same spacing to the shuffle button like the shuffle button has to the repeat button? Or try to center it between the shuffle and the "X" button. I would prefer the latter. Without padding or margin even very long times should fit fine even on small displays.

But maybe that's just me. In general this is good as it is.

@Douile
Copy link
Contributor Author

Douile commented Apr 8, 2021

Thanks. Ye, I do think centered will look better.

Copy link
Member

@Redirion Redirion left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks

@Redirion Redirion merged commit 3fcd580 into TeamNewPipe:dev Apr 8, 2021
This was referenced Apr 11, 2021
tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Playlist/Queue Total Time
3 participants