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

Swap seekbar position with player buttons in Queue screen #6824

Merged
merged 2 commits into from
Oct 2, 2021

Conversation

phigjm
Copy link
Contributor

@phigjm phigjm commented Aug 1, 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

  • reorder the Controll elements of the Play queue (putting the Title down)

Before/After Screenshots/Screen Record

Before After

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@blindm0use
Copy link

Great job fixing my issue!
It is best to place timecode popup dialog while seeking either in the middle of the screen or slightly above the seek bar. Otherwise vision gets obstructed by the finger while seeking. Thanks!

@phigjm
Copy link
Contributor Author

phigjm commented Aug 2, 2021

Great job fixing my issue!
It is best to place timecode popup dialog while seeking either in the middle of the screen or slightly above the seek bar. Otherwise vision gets obstructed by the finger while seeking. Thanks!

I'll give it a try.

@triallax triallax added the GUI Issue is related to the graphical user interface label Aug 2, 2021
@phigjm
Copy link
Contributor Author

phigjm commented Aug 2, 2021

Here you are

@SameenAhnaf
Copy link
Collaborator

SameenAhnaf commented Aug 3, 2021

@phigjm Thanks for opening a PR. I don't have a device with the bug. So, rectify me if I am wrong.

But I think, seek bar should be placed in the middle and preview should be shown within the title bar as it used to do. Google Podcasts works this way. Could you please give it a go?

@Stypox
Copy link
Member

Stypox commented Aug 4, 2021

@SameenAhnaf I agree with you, we should follow that ordering. Also, @phigjm try to make it look like as if there is the same spacing between seekbar-title and seekbar-buttons.

@phigjm
Copy link
Contributor Author

phigjm commented Aug 4, 2021

This is how it looks now:

at the bottom, there is just enough space to fully represent the play button effects. These buttons seem to only trigger at release. So there should be no problem with the gesture navigation.
I also decided (as you can see) to move the Time above the title as it is transparent and therefore doesn't look that grate over the title.

@phigjm
Copy link
Contributor Author

phigjm commented Aug 4, 2021

And here a screenshot from a simulated pixel a3:

@Stypox
Copy link
Member

Stypox commented Aug 5, 2021

The last two screenshots look really good to me. Thank you! I've edited all of your comments to use <img src="" height="300px"/> for screenshots so that they aren't so big.

@phigjm
Copy link
Contributor Author

phigjm commented Aug 5, 2021

The last two screenshots look really good to me. Thank you! I've edited all of your comments to use <img src="" height="300px"/> for screenshots so that they aren't so big.

Thank you

@phigjm phigjm requested a review from Stypox August 9, 2021 11:57
phigjm and others added 2 commits August 9, 2021 20:11
from bottom to top: Playback_controls, progress bar, center, (seek display)
@Stypox Stypox force-pushed the player_queue_control_rearrangement branch from 21037ea to 4d50a66 Compare August 9, 2021 18:12
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

I rebased and squashed; I un-nested some views (unrelated to your work). Thank you!

@litetex
Copy link
Member

litetex commented Sep 25, 2021

@Stypox
Is this ready for merging?

@opusforlife2
Copy link
Collaborator

@litetex If he's approved it and not asked anyone else to review, then it must mean he considers it ready. Is that right, @Stypox?

@opusforlife2 opusforlife2 changed the title Rearrange the control elements Swap seekbar position with player buttons in Queue screen Oct 2, 2021
@TobiGr TobiGr merged commit 5b31370 into TeamNewPipe:dev Oct 2, 2021
This was referenced Oct 2, 2021
ShareASmile added a commit to ShareASmile/NewPipeZing that referenced this pull request Jun 16, 2024
reorder the Control Elements of the background player (putting the Title down)
TeamNewPipe#6824
Fixes; TeamNewPipe#6788
TeamNewPipe#2931

Co-Authored-By: phigjm <39876427+phigjm@users.noreply.github.com>
ShareASmile added a commit to ShareASmile/NewPipe-Legacy-Revo that referenced this pull request Jun 16, 2024
reorder the Control Elements of the background player (putting the Title down) TeamNewPipe/NewPipe#6824
Fixes: TeamNewPipe/NewPipe#6788
TeamNewPipe/NewPipe#2931

new icons in background player by avently commit

made image view from playQueue visible by avently in commit
TeamNewPipe/NewPipe@dd726fa#diff-EFCAA05775EEBAE5D5E7D3DD8FB04196
from TeamNewPipe/NewPipe#4360

Co-Authored-By: Stanislav Dmitrenko <7953703+avently@users.noreply.github.com>
Co-Authored-By: wb9688 <46277131+wb9688@users.noreply.github.com>
Co-Authored-By: phigjm <39876427+phigjm@users.noreply.github.com>
ShareASmile added a commit to ShareASmile/FoxPipe that referenced this pull request Jun 16, 2024
Reorder the Control Elements of the background player (putting the Title down) TeamNewPipe/NewPipe#6824
Fixes: TeamNewPipe/NewPipe#6788
TeamNewPipe/NewPipe#2931

new icons in background player by avently commit in v0.20.0

made image view from playQueue visible by avently in commit
TeamNewPipe/NewPipe@dd726fa#diff-EFCAA05775EEBAE5D5E7D3DD8FB04196
from TeamNewPipe/NewPipe#4360

Co-Authored-By: phigjm <39876427+phigjm@users.noreply.github.com>
Co-Authored-By: Stanislav Dmitrenko <7953703+avently@users.noreply.github.com>
ShareASmile added a commit to ShareASmile/NewPipe-Legacy-Revo that referenced this pull request Jul 14, 2024
reorder the Control Elements of the background player (putting the Title down) TeamNewPipe/NewPipe#6824
Fixes: TeamNewPipe/NewPipe#6788
TeamNewPipe/NewPipe#2931

new icons in background player by avently commit

made image view from playQueue visible by avently in commit
TeamNewPipe/NewPipe@dd726fa#diff-EFCAA05775EEBAE5D5E7D3DD8FB04196
from TeamNewPipe/NewPipe#4360

Co-Authored-By: Stanislav Dmitrenko <7953703+avently@users.noreply.github.com>
Co-Authored-By: wb9688 <46277131+wb9688@users.noreply.github.com>
Co-Authored-By: phigjm <39876427+phigjm@users.noreply.github.com>
ShareASmile added a commit to ShareASmile/NewPipe-Legacy-Revo that referenced this pull request Jul 14, 2024
reorder the Control Elements of the background player (putting the Title down) TeamNewPipe/NewPipe#6824
Fixes: TeamNewPipe/NewPipe#6788
TeamNewPipe/NewPipe#2931

new icons in background player by avently commit

made image view from playQueue visible by avently in commit
TeamNewPipe/NewPipe@dd726fa#diff-EFCAA05775EEBAE5D5E7D3DD8FB04196
from TeamNewPipe/NewPipe#4360

Co-Authored-By: Stanislav Dmitrenko <7953703+avently@users.noreply.github.com>
Co-Authored-By: wb9688 <46277131+wb9688@users.noreply.github.com>
Co-Authored-By: phigjm <39876427+phigjm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Issue is related to the graphical user interface
Projects
None yet
8 participants