Skip to content

Conversation

@PeterHindes
Copy link
Contributor

@PeterHindes PeterHindes commented Oct 14, 2019

When playing a single background track, the expanded notification now also offers the ability to restart track, rather than only the ability to rewind

With a single song we have the option to restart the track.
Single Song
With A playlist we already have the option.
Single Song

app-debug.apk.zip

VishalNehra and others added 26 commits January 18, 2019 23:44
@PeterHindes PeterHindes changed the title [Feature Added] When Playing A Single Background Track, The Expanded Notification Offers The Ability To Restart Track, Rather Than Only The Ability To Rewind [Feature Added] When Playing A Single Background Track, The Expanded Notification Also Offers The Ability To Restart Track, Rather Than Only The Ability To Rewind Oct 14, 2019
@Stypox
Copy link
Member

Stypox commented Oct 15, 2019

Can you please provide a debug APK or some screenshots? Also, please edit make the issue title shorter and rather add something to the description. Thank you for the PR! :-)

@PeterHindes PeterHindes changed the title [Feature Added] When Playing A Single Background Track, The Expanded Notification Also Offers The Ability To Restart Track, Rather Than Only The Ability To Rewind [Feature Added] Restart Song From Start Oct 15, 2019
still need to find out why its null on the library
@Stypox
Copy link
Member

Stypox commented Oct 15, 2019

Buttons are too squished toghether on my phone. Could you please make sure buttons never overlap? As you can see in the second image the bounds of the "repeat" button overlap the bounds of the newly-added button. (I used the "Show layout bounds" option in Developer options)

@PeterHindes
Copy link
Contributor Author

Ok, Ill work on that

@PeterHindes
Copy link
Contributor Author

It looks like the timer on the video also overlaps buttons (including the regular rewind) on exceptionally small screens

@PeterHindes
Copy link
Contributor Author

On screens that are extremely small, the thumbnail should gray out and the notification should take the whole width (displaying over the thumbnail)

Currently

@PeterHindes
Copy link
Contributor Author

what code can I use to check if the elements are overlaping?

@Stypox
Copy link
Member

Stypox commented Oct 16, 2019

You can probably check whether the X coordinate of the left side of the newly added button is smaller than the X coordinate of the right side of the repeat button.

Also, I don't know if it is ok to hide thumbnail on small screens. @TobiGr what do you think?

@PeterHindes
Copy link
Contributor Author

I was thinking more of a grayed out thumbnail with the text on-top

@Stypox
Copy link
Member

Stypox commented Oct 17, 2019

That's ok, I think

@wb9688
Copy link
Contributor

wb9688 commented Apr 12, 2020

What's up with the 54(!) commits including those of others? Also imho we shouldn't touch notifications until #3178 has been merged.

@PeterHindes
Copy link
Contributor Author

I don't know how those got there lol, this commit is kinda dead tbh, I'm probably gonna close it since no one seems interested in the feature, and it will likely be default behavior with the new notification.

@PeterHindes
Copy link
Contributor Author

I am going to close this issue&pull request as it doesn't make as much sense as I had originally intended, and it is out of date. I better fix might be to have a tap&hold on the rewind button (which shows when only one song is qued) jump to the start of the song instead of a second button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants