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

Notification Improvements #3178

Merged
merged 29 commits into from Sep 27, 2020
Merged

Conversation

cool-student
Copy link
Contributor

@cool-student cool-student commented Mar 3, 2020

  • add MediaStyle notifications for Background and Popup playback
  • reduce excessive notification updating ( / recreating of Notification.Builder object) when playing background / popup media
  • add new buffering state indicator (can be disabled)
  • upscale close icon / downscale replay icon
  • add notification slot settings
  • move notification settings to appearance
  • fix Metadata (song title, artist and album art) sometimes not being set correctly
  • other misc notification fixes

debug apk (@Stypox 22/09/2020 b4e073c): app-debug.zip

closes #690 closes #1660 closes #2845 closes #1104 closes #1207 closes #3149 closes #3170
closes #1527 closes #1242 closes #1568 closes #1574 closes #1596 closes #2007
closes #3436 closes #3071 closes #4031 closes #4033 closes #2976 closes #2397 closes #228
closes #4286

@opusforlife2
Copy link
Collaborator

Uploads to Github are convenient to share the link with a download manager, if one doesn't want to rely on the browser. Firefox Send is like Mega in this respect. It forces the use of the browser for the download.

@wb9688
Copy link
Contributor

wb9688 commented Mar 3, 2020

Nice work! I'll take a look at it later (but I'm not going to do an extensive code review).

@opusforlife2: Firefox Send does that as that's the only way to have end-to-end encryption. However that's kinda off-topic here.

@opusforlife2
Copy link
Collaborator

Ah. Good to know.

@opusforlife2
Copy link
Collaborator

opusforlife2 commented Mar 3, 2020

The compact view item selector should be a list of checkboxes, with a toast or an alert showing up if the user tries to select more than 3.

Edit: No deleteIntent yet? If you do implement it, make sure that it is disabled if one of the buttons is Close.

Is there no buffering animated icon for the play button position?

@Stypox
Copy link
Member

Stypox commented Mar 5, 2020

Would this also solve #1574 and all of its duplicates? By the way, I improved the markdown in the pr description and added a "Closes" string before every issue so that github knows it should close each of these issues. Also, there is no need to upload apk files to an external service like Firefox Send, you can instead zip them and upload them directly here on github. @cool-student thank you :-D

@njmdietrich
Copy link

I have been using the debug apk for a few days and I can report that this PR fixes issue #3071 - although the "Unknown" notification will still come up when you press play on connected headphones, but it can be closed properly now. #2976 is probably a duplicate of the other issue so would be fixed too.
The only (minor) issue I've found with the PR is playing live streams on YouTube - the notification updates every few seconds like before, but every time it happens the play indicator jumps to the beginning for a second or so.

@vkhomenk
Copy link

Link has expired

@Stypox
Copy link
Member

Stypox commented Mar 15, 2020

@cool-student you can upload the apk directly on GitHub by zipping it ;-)

@opusforlife2
Copy link
Collaborator

Bug: #3201 (comment)

@wb9688
Copy link
Contributor

wb9688 commented Mar 25, 2020

Could you use XML drawables instead of PNG drawables?

@wb9688 wb9688 added this to the 0.20.0 milestone Apr 11, 2020
@Stypox Stypox linked an issue Apr 15, 2020 that may be closed by this pull request
@Stypox Stypox linked an issue May 2, 2020 that may be closed by this pull request
2 tasks
@Stypox Stypox linked an issue May 9, 2020 that may be closed by this pull request
@B0pol B0pol linked an issue May 9, 2020 that may be closed by this pull request
@Rorschach1010
Copy link

Download link expired, can someone please share it or even better, upload it to git itself?

@opusforlife2
Copy link
Collaborator

@Rorschach1010
Copy link

thanks a bunch, will test it tomorrow.

@Rorschach1010
Copy link

@Rorschach1010 app-debug-notifications-v8-8.zip

I was able to test this 2 days now and tbh it didn't solve the issue. Not sure if this is an important detail but instead of the normal newpipe notification icon, in this version it was replaced with the android standard download icon (arrow pointing down with a line at the bottom) in the notification bar.
Other then that, nothing changed and newpipe still started when I start my car.

@opusforlife2
Copy link
Collaborator

@Rorschach1010 Are you talking about this issue? #3071 (comment)

@Rorschach1010
Copy link

@Rorschach1010 Are you talking about this issue? #3071 (comment)

I was lead here from #1596 but it sounds like its the same issue. I didn't see your mentioned issue but now that I read it, what you posted there pretty much sums it up

If Newpipe was your last played audio app, then events such as connecting a headphone can automatically try to start such apps. If you have the ability to change tracks using the volume button, that can have this effect as well.

@wb9688
Copy link
Contributor

wb9688 commented May 14, 2020

I rebased this PR and fixed the Checkstyle errors. Here is the new APK:
app-debug.apk.zip.

Edit: I also replaced the new drawables with vector drawables, but I didn't test it yet on Android 4.4 and that's not in the above APK.

I'll test and take a look at the code soon, and maybe fix it.

PS: pre-rebase branch could be found at https://github.com/wb9688/NewPipe/tree/old-notification-improvements.

@vkay94

This comment has been minimized.

@MD77MD

This comment has been minimized.

@Stypox

This comment has been minimized.

@MD77MD
Copy link

MD77MD commented Sep 22, 2020

oh boy... no i cant access the play queue using apk from #3178 (comment) ... although i did everything you asked

however and just to clarify, i uninstalled it and reinstalled the apk from #3178 (comment) ....the notification on Lock screen shows fine but once i pause it, it disappears and cant resume playback unless i unlock the device and go back to newPipe like what i said in #3178 (comment)

@TheGinGear

This comment has been minimized.

@avently

This comment has been minimized.

@Stypox
Copy link
Member

Stypox commented Sep 23, 2020

@MD77MD do other buttons work fine? And does the notification disappear completely when you tap pause, or does it only disappear from the lock screen while still being accessible from the drag-down menu in the unlocked phone?

@TheGinGear

This comment has been minimized.

@vkay94

This comment has been minimized.

@TheGinGear

This comment has been minimized.

@vkay94

This comment has been minimized.

@Stypox
Copy link
Member

Stypox commented Sep 23, 2020

@TheGinGear that's a known issue, but won't be fixed here

@MD77MD
Copy link

MD77MD commented Sep 23, 2020

@MD77MD do other buttons work fine? And does the notification disappear completely when you tap pause, or does it only disappear from the lock screen while still being accessible from the drag-down menu in the unlocked phone?

@avently all buttons work fine... the notification only disappears from lock screen but still available at the notification center (if that what you mean by the drag-down menu)

p.s: just to clarify because i can attach screen shot... what i mean by notification background is the background on the lock screen with the opend video's thumbnail and laid on top of it the 3 player buttons.

edit: sorry... I was supposed to tag @Stypox not avently

@TobiGr
Copy link
Member

TobiGr commented Sep 27, 2020

@MD77MD We have a problem. @Stypox and I cannot reproduce your bug, because KitKat emulators suck. We agreed on merging this PR, because there are no problems with it except for KitKat users.
@Stypox might get a hand on a KitKat device and fix your problems. If he does not, we try to reach out to other devs with KitKat devices to help us.
edit: @mqus Can you help us here?

@TobiGr TobiGr merged commit fc9c073 into TeamNewPipe:dev Sep 27, 2020
@TobiGr
Copy link
Member

TobiGr commented Sep 27, 2020

thanks to everyone involved!

@opusforlife2
Copy link
Collaborator

HALLELUJAH!

@B0pol
Copy link
Member

B0pol commented Sep 27, 2020

21 issues fixed! congrats

This was referenced Sep 27, 2020
@MD77MD
Copy link

MD77MD commented Sep 27, 2020

@MD77MD We have a problem. @Stypox and I cannot reproduce your bug, because KitKat emulators suck. We agreed on merging this PR, because there are no problems with it except for KitKat users.
@Stypox might get a hand on a KitKat device and fix your problems. If he does not, we try to reach out to other devs with KitKat devices to help us.
edit: @mqus Can you help us here?

its ok..i don't think it's worth blocking the merger. we can always open a new issue if any of the users complain about it.

@opusforlife2
Copy link
Collaborator

Since the RC is now available, locking this thread because too many people get spammed with notifications.

@TeamNewPipe TeamNewPipe locked as resolved and limited conversation to collaborators Sep 27, 2020
@TobiGr
Copy link
Member

TobiGr commented Sep 27, 2020

@MD77MD Can you open a new issue with all the details about the problem and ping mqus?

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