[Refactor] Convert playback buttons to segmented button - #13669
[Refactor] Convert playback buttons to segmented button#13669Isira-Seneviratne wants to merge 7 commits into
Conversation
TobiGr
added
rewrite
|
Thank you, but why did you replace the background / headset icon with the play button? Why standard? |
That was a mistake on my part, will use the headset icon instead.
I felt that using just "Play all" might be a bit vague for indicating normal playback. |
|
I get this after pressing the popup button. However, I am prompted to approve the permissions for NewPipe and no Popup is created. |
Updated |
|
I'm pretty sure that we are not merging changes into refactor anymore and all refactors should now go into dev directly, no? https://github.com/TeamNewPipe/NewPipe/blob/HEAD/.github/CONTRIBUTING.md#creating-a-pull-request-pr |
|
Yes, but no because this is an improvement for something that has not been merged to dev yet. |
|
@Isira-Seneviratne Am I correct in understanding that from the screenshot, this swaps the background and play all buttons around? If so, why? |
Yes, it's because of how the player options are ordered in the code. |
AudricV
left a comment
AudricV
left a comment
There was a problem hiding this comment.
I think Player isn't the right string to use. Shouldn't we use something like Play in instead?
| import androidx.compose.material.icons.Icons | ||
| import androidx.compose.material.icons.automirrored.filled.PlaylistPlay | ||
| import androidx.compose.material.icons.filled.Headphones | ||
| import androidx.compose.material.icons.filled.PictureInPicture | ||
| import androidx.compose.ui.graphics.vector.ImageVector |
There was a problem hiding this comment.
In my opinion you shouldn't add Compose code in the non-UI player one. The player has already a lot of dependencies breaking separation concerns.
|
4 participants
What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
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. You can find more info and a video demonstration on this wiki page.
Due diligence