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

Mini player should also get closed if notification player is closed #6227

Closed
4 tasks done
SameenAhnaf opened this issue May 3, 2021 · 16 comments
Closed
4 tasks done
Labels
bug Issue is related to a bug

Comments

@SameenAhnaf
Copy link
Collaborator

Checklist

Steps to reproduce the bug

  1. Start any video on any player
  2. Close it from notification player
  3. Notification player gets closed
  4. Mini player still persists on the app inside even if I am using another app.

Actual behaviour

It seems like that "Pause" and "Close" action button has the exact same purpose for now. 'Close' button just ditches the notification as well.

Expected behavior

"Close" button should close both the notification and media completely.

Screenshots/Screen recordings

Logs

Device info

  • Android version/Custom ROM version: MIUI 12.0.7
  • Device model: Xiaomi Redmi Note 8 Pro
@SameenAhnaf SameenAhnaf added the bug Issue is related to a bug label May 3, 2021
@Stypox
Copy link
Member

Stypox commented Jul 28, 2021

This is the expected behaviour

@Stypox Stypox closed this as completed Jul 28, 2021
@SameenAhnaf
Copy link
Collaborator Author

@Stypox #4335 is already requested. So, there should be an option to clear queue easily as well.

@Stypox
Copy link
Member

Stypox commented Jul 29, 2021

The X button clears the queue, doesn't it? How is this related to this issue? And how is #4335 related to this issue?

@SameenAhnaf
Copy link
Collaborator Author

SameenAhnaf commented Jul 29, 2021

@Stypox Nope. Clearing notification was always the case once unified player was introduced.

Even if keeping the last media was the expected behavior, playback manager is not accessible and other videos on the queue are also remaining.

Screenrecorder-2021-07-29-16-26-39-113_540x960.mp4

And how is #4335 related to this issue?

#4335 asks for an alternative for clearing notification. So, "X" button should both clear queue and close player. Also, it's logical to have same behavior in both mini player and notification player for the same button.

@Stypox
Copy link
Member

Stypox commented Jul 31, 2021

Oh ok, I see what you mean. Why is that video an issue?

@SameenAhnaf
Copy link
Collaborator Author

@Stypox You forgot to reopen this issue.

Why is that video an issue?

I don't follow you. But all of my devices have the same bug. So, maybe, it's something with settings.

@Stypox
Copy link
Member

Stypox commented Aug 5, 2021

Ok, I'm reopening this. But I still don't get why it is a problem that the mini player (along with its queue) is not closed.

@Stypox Stypox reopened this Aug 5, 2021
@SameenAhnaf
Copy link
Collaborator Author

SameenAhnaf commented Aug 5, 2021

Thanks. Also, sorry for the inconveniences. The expected behavior of 'X' button is clearing notification as of now.

Even if I assume this is the expected behavior, a bug is remaining. Playback manager can't be accessed from notification player by short press.

You will notice at the end of screen recording.

Screen Recording
127477635-da221014-4b2c-4d87-af9a-919437987e9b.mp4

This is what I am calling playback manager.

Screenshot

As far as I know, you don't like inconsistent behavior. This is the reason why you wanted to ditch 'Enqueue' feature completely 🙃.

X button in the media player clears queue. That's why, I think the same button in mini player should behave the same.

@Stypox
Copy link
Member

Stypox commented Aug 6, 2021

Playback manager can't be accessed from notification player by short press.

That's because it can be accessed through the main player.

This is the reason why you wanted to ditch 'Enqueue' feature completely :upside_down_face.

I never wanted to ditch 'Enqueue'

X button in the media player clears queue. That's why, I think the same button in mini player should behave the same.

Ok, I get this, but what are the downsides of keeping the backstack of opened videos?

@SameenAhnaf
Copy link
Collaborator Author

SameenAhnaf commented Aug 6, 2021

That's because it can be accessed through the main player.

I didn't notice the behavior in the app. But why should it switch to main player? I didn't ask it to do so. There is 'Switch to main' option in playback manager.

I never wanted to ditch 'Enqueue'

See: #5850 (comment).

Maybe, I misinterpreted the things though. Please rectify me if I am wrong.

what are the downsides of keeping the backstack of opened videos?

There's no easy way to clear background or popup queue. These players are meant for multitasking.

  1. Pull down notification panel
  2. Short press on notification player
  3. Access playback manager
  4. Press 'Back'
  5. Swipe down queue manager
  6. Get back to the last app

It's still a hassle when accessed from recent apps.

  1. Go to recent apps
  2. Choose 'Newpipe'
  3. Swipe down video info (if any video info is open)
  4. Press 'Back'
  5. Swipe down mini player
  6. Get back to last app

Why can't I clear queue by just pulling down notification panel and tapping the X button?

Also, why should same button behave differently in notification and mini player? All other buttons like pause, next, previous etc behave the same all over the UI.

Plus, an alternative of clearing notification is already suggested. Clearing queue is a basic task and it should be easily accessible.

@Stypox
Copy link
Member

Stypox commented Aug 6, 2021

See: #5850 (comment).

I was saying that having multiple Enqueues that could change the player type was bad UX, not that the enqueue option should be ditched completely... The current "Enqueue" option is perfect.

Why can't I clear queue by just pulling down notification panel and tapping the X button?

That already happens!! If you close the notification with the X and then start another player the old videos will not be present! That's because X also clears the queue! The only thing that X doesn't currently do is clear the backstack of VideoDetailFragments, and I think that's what you are asking for. But even then, why would you clear the backstack of opened videos when pressing the X in the notification? How is that related?

There's no easy way to clear background or popup queue.

Mmmh, let's see... Clicking the X in the notification does exactly that?

Plus, an alternative of clearing notification is already suggested. Clearing queue is a basic task and it should be easily accessible.

Clearing the queue is (and should be) equivalent to closing the player, as a player cannot exist without a queue with at least one item. So why make them two different options, just to confuse all users?

@Stypox
Copy link
Member

Stypox commented Aug 6, 2021

Here is a demonstration that X clears the queue:

  • start playing video 1 in any player type
  • clear the notification with the X
  • start playing video 2 in any player type
  • tap on the notification to view the queue or access the queue from within the main player
  • the queue only contains video 2 (and possibly an auto-enqueued one, but that's unrelated)

@SameenAhnaf
Copy link
Collaborator Author

The situation isn't highly problematic for now. What if multiple queues are introduced? Avently himself opened an issue on this. See: #5940.

@Stypox
Copy link
Member

Stypox commented Aug 6, 2021

Yes, but multiple queues has nothing to do with this, hasn't it?

@SameenAhnaf
Copy link
Collaborator Author

First of all, as there's a mini player for main player, it's expected that users may want to keep main queue as well. Plus, Will it be logical to turn background queue into main queue although I only asked only to clear the notification? It's normal that I won't like to mix up queues.

@Stypox
Copy link
Member

Stypox commented Aug 10, 2021

Oh, btw, this was already discussed: #4131

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug
Projects
None yet
Development

No branches or pull requests

2 participants