Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
51 stars 10 forks source link

tap to bring to front? #44

Closed vixalien closed 1 year ago

vixalien commented 1 year ago

I don't know how sp-tray does it, but when you click on the indicator, it jumps to the spotify app and when you click it again, it jumps back to where you were.

Moon-0xff commented 1 year ago

Since yesterday's update you can press right-click (default binding) in the label to raise the window, right-clicking again does nothing. So we are already halfway there.

This was a mere oversight, and shouldn't be a difficult change to add (i hope).
Answering the question of how sp-tray does it i think all comes down to this function:

        // many thanks to mheine's implementation
        showSpotify() {
            if (this._spotiWin && this._spotiWin.has_focus()) {
                // spotify is up and focused
                if (this._notSpotify) {
                    // hide spotify and pull up the last active window if possible
                    Main.activateWindow(this._notSpotify);
                }
            } else {
                // spotify is unfocused or the tray icon has never been clicked before
                this._spotiWin = this._notSpotify = null;
                let wins = global.get_window_actors(); // get all open windows
                for (let win of wins) {
                    if (typeof win.get_meta_window === "function") {
                        if (win.get_meta_window().get_wm_class() === "Spotify") {
                            this._spotiWin = win.get_meta_window(); // mark the spotify window
                        } else if (win.get_meta_window().has_focus()) {
                            this._notSpotify = win.get_meta_window(); // mark the window that was active when the button was pressed
                        }

                        if (this._spotiWin && this._notSpotify) {
                            break;
                        }
                    }
                }
                Main.activateWindow(this._spotiWin); // pull up the spotify window
            }
        }
vixalien commented 1 year ago

oh that's good to hear. let me re-login to checkout the new features

Batwam commented 1 year ago

@Moon-0xff Note that the current implementation in #29 opens the player but does not put the previously focussed window back on with second click.

I had the plan of hiding the window on second click (actually had a TODO note in the code at some point) as it felt like a natural extension of the action until I realised that we don't really know what the user is doing between the first and second click. For waht we know multiple hours could have passed so we could be showing the wrong app. Our implementation is also a little bit different than sp-tray as sp-tray only works with Spotify while mpris-label works with every music player known to man. This means that a user could click once to open Spotify, close it, start Rhythmbox and expect the Open App click to open Rhythmbox.

Basically it felt like to much work for something the user can achieve by typing alt+tab.

Now,if we really felt that it was needed, there would probably be a way. Perhaps save the name of the previously focussed window and mpris app window, reset the whole lot if the active player changes. If the focussed window still matches the active player, then we revert (or find a way to perform alt+tab from gjs).

vixalien commented 1 year ago

my tiny brains thought that the "recent windows" could be got by an API or something. I mean the list of windows a user see when they tap Alt+Tab. I thought we could then switch to the next app on that list (provided it isn't the current player)

Moon-0xff commented 1 year ago

Yes, we can trigger an alt+tab by interfacing with the AppSwitcherPopup on Main. That's my theory at least.

And we can guess what's the player window(s) by name like sp-tray does to find the spotify window:

  let wins = global.get_window_actors(); // get all open windows
                for (let win of wins) {
                    if (typeof win.get_meta_window === "function") {
                        if (win.get_meta_window().get_wm_class() === "Spotify") {
                            this._spotiWin = win.get_meta_window(); // mark the spotify window

edit: AppSwitcherPopup isn't on Main

vixalien commented 1 year ago

hmm this is interesting

Batwam commented 1 year ago

first pass code implementation in the PR above

Batwam commented 1 year ago

thanks @Moon-0xff for the rapid review and insightful suggestions always. As this is now merged, should we push this to the general public so everyone can enjoy the benefits of @vixalien's suggestion as a new release? It feels like merging the controls and filters might delay this a bit too long.

Note that while we are at it, it might be worth including #45 too as it's a pretty simple one.

Moon-0xff commented 1 year ago

I share the feeling that #31 and #39 are far from being merged, but this is a rather small feature to solely justify an update.

Moon-0xff commented 1 year ago

New update includes #46 Thanks @vixalien for raising this issue, and of course, @Batwam for the PR.