Option to close player

[ghost]

Thanks for this great extension, I could only wish this was a default in Gnome as it removes the need for tray icons!

Would it be possible to add an option to close the player from the menu? I've added a circle where a close button could be: http://imgur.com/a/HbPE3

[JasonLG1979]

I'm really apprehensive to add a close button there. As clicking anywhere on the menu title opens and closes the menu everywhere else in the shell

[ghost]

Would there be a better way to do this? My main reason for asking this is that applications such as Rhythmbox continue to run in the background. This is of course not a fault of your extension, just the way the application is designed. Currently I have to open Rhythmbox and select quit from the toolbar or kill it via task manager.

I understand if you don't want to add this feature. The extension is great with or without it!

[icasdri]

Could make it available as a button in the expanded player drop-down (i.e. only clickable once expanded). And off by default (available in options).

[JasonLG1979]

I could but then the question is where?

[ghost]

@JasonLG1979 How about to the left of the app-title: http://imgur.com/a/8hVuj

This could be hidden when the player is minimized and appear when expanded. You would avoid accidental clicks since it's on the opposite side of the expand-icon.

[lexruee]

@JasonLG1979 @icasdri @nooryani84 I hacked a proof-of-concept together that implements the close button feature as proposed by @nooryani84.

I'm not sure how reliable and bug-free it is. It required some extra work to implement because the default behaviour of some gnome-shell widgets needed to be overridden. Overall, I think this feature would introduce more trouble than it's worth :smile:.

The proof-of-concept can be found in the branch close-button on my repo: https://github.com/lexruee/gnome-shell-extensions-mediaplayer/tree/close-button

Cheers

[JasonLG1979]

@lexruee The problem with what you got is that it assumes that the player has a UI:

    close: function() {
         if (this.app) {
             let windows = this.app.get_windows();
             for (let i = 0; i < windows.length; i++)
                 windows[i].delete(global.get_current_time());
         }
     },

That may not be the case as is with MPD for example. The MPRIS spec has the CanQuit prop you can use to tell if an application is capable of closing/quitting via MPRIS. If a player CanQuit you can just use it's MPRIS Quit Method.

[JasonLG1979]

@lexruee https://github.com/JasonLG1979/gnome-shell-extensions-mediaplayer/commit/054e6f2974eff17b734e300b6ac04851424b5417 adds the required plumbing.

[JasonLG1979]

You'd want to test to see that a player canQuit to even decide to show the button at all.

[JasonLG1979]

I would also prefer for the sake of appearance that the close button be in between the player's name and the playstatus icon. That way everything lines up better.

[lexruee]

@JasonLG1979 Thanks for the feedback. Now, I know why spotify's MPRIS interface is suboptimal. Without the ugly hack below (8ee3ce4) I can't quit spotify :-/. I think this close button feature could be a bug-hell :thinking: . Of course we could only show the close button for players that support canQuit but in this case this feature would appear to me incomplete.

 quit: function() {
      if (this.canQuit) {
        this._mediaServer.QuitRemote();
      } else if (this.app) {
            let windows = this.app.get_windows();
            for (let i = 0; i < windows.length; i++)
                windows[i].delete(global.get_current_time());
      }
    }

Regarding the UI, I changed it as you suggested (998fde2):

Currently, I still face some UX related bugs. For example, closing some players causes the popupMenu to be closed too. In my opinion, this makes only sense when there is no mpris-compliant player running anymore.

Edit: Overall, I think the main question is if we want to support an incomplete or no "close button feature" at all as only some players can be closed via MPRIS.

[JasonLG1979]

Just use the BROKEN_PLAYERS blacklist and don't even expose the functionality with Spotify. Spotify's MPRIS implementation is absolute shit...

[JasonLG1979]

As much as I hate blacklists I hate hacky workarounds more. That could cause problems with other apps.

[JasonLG1979]

I think this close button feature could be a bug-hell .

I agree. I'm in no hurry to implement it. We need to make sure it's well tested before/if it's implemented.

[JasonLG1979]

@lexruee I Wrote another MPRIS Extension, a much simpler alternative to this one. More of a direct replacement for the Ubuntu SoundMenu. Where this extension is ofc crazy feature packed, https://github.com/JasonLG1979/gnome-shell-extensions-mpris-indicator-button is very plain Jane with no user options. Anyway I added a quit button to it that's only visible on hover if player.CanQuit is true. You might take a look at this commit and you may be able to adapt it to this extension.

https://github.com/JasonLG1979/gnome-shell-extensions-mpris-indicator-button/commit/5cbb5399e8983ee135ddc50cb5d1702d793a85e5

[lexruee]

@JasonLG1979 Looks nice the new extension. I like the KISS principle behind it :smiley:. I'll have a look at the code.

[JasonLG1979]

@JasonLG1979 Looks nice the new extension. I like the KISS principle behind it . I'll have a look at the code.

Thanks, that's what I was going for. The new extension is less than 600 lines of js. I don't plan on adding support for the trackList or playList interfaces. I wanted it to be a bit of a middle of the road between the default media controls and this extension. IMHO the default media controls have a few glaring flaws both in their intended behavior/functionality and under the hood code wise. I did my best to address those in the new extension. I'm sure there are users out there that don't like how the default media controls work or that they're in the notification area (the biggest flaw IMHO) but also don't care for the layout and complexity of this extension. The new Extension gives them another choice, and more choices for the user is always a good thing.

I've tested it with a few players including the notoriously awful Spotify and it seems to work pretty well so far.

[JasonLG1979]

@lexruee see https://github.com/JasonLG1979/gnome-shell-extensions-mpris-indicator-button/commit/0cbb7fa5c2aa7ed2ca1725c74e3fba6c12736bef

Shell.App.request_quit() Works pretty well with Spotify.

[JasonLG1979]

https://github.com/JasonLG1979/gnome-shell-extensions-mediaplayer/blob/master/README.md