Implement Menu for player selection

[Batwam]

as per item 4 in Development Roadmap #4 , this is a working version to implement a menu selection for the active player.

Todo List:

• [x] generate menu with list of items
• [x] include link to Settings
• [x] make menu dynamically update based on actual sources
• [x] update menu item formatting to highlight selected/active item Vs other sources (See screenshots below)
• [x] grey out on sources when in Manual mode (instead of italic) to indicate it's info only.
• [x] link grey colour used for source to theme style for non sensitive menu items (or equivalent)
• [x] link item selection event to turn AUTO_SWITCH_TO_MOST_RECENT on and off
• [x] link item selection event to actual selection of source (i.e. Clicking Spotify switches the source)
• [ ] move Build_Menu code into separate menu.js file
• [ ] check code for edge cases and missing inputs which may make it generate errors/fail (e.g. source disappears while menu is open)
• [x] check compatibility (e.g. on GNOME 3.38, the menu works but the dot doesn't appear)
• [ ] check for impacts/improvements to existing code (e.g. button click to refresh extension position no longer works, new shortname item could be used in filter section to remove the address re-formatting lines)

known issue:

• [x] Menu is refreshed on each iteration. If an item is highlighted and the mouse left in position, the refresh will be visible as the item will blink as it gets refreshed. This is not visible on other extensions using menus. Need to compare implementation to see how the menu could be updated only on click. Edit: fixed by linking menu generation to mouse click instead of each refresh cycle.

Auto Selection Mode: tick for Auto, dot to indicate active source. sources in italic to indicate that they are listed for info only Manual Selection Mode: tick mark for selected source

[Batwam]

See current version and roadmap above. Feel free to provide any comments/pointer. I have now managed to get the menu to indicate the playing source. Next step is to work on the actions connected to each item. (Edit, I have now also implemented the source switching upon item selection...)

For example, I can read this.settings.get_boolean('auto-switch-to-most-recent') but what's the easiest way to change the value to true/false?

also, I'm implemented a bunch of functions into players.js which we could probably do without but for some reason, I can't really get this to work without it... Feel free to review/clean if you see a better way of implementing this.

[Moon-0xff]

I pushed my changes, i didn't really changed the implementation, just switched how the values are accessed. Inspect the commits for more details. I also removed a lot of the comments, for brevity.

I also connected the auto-switch button to the setting, the auto-switch prevents changing the source manually.

Regarding compatibility, the dot appears now on 3.38, along with the italics and the check.

[Batwam]

That's excellent, thanks!

I have also updated the todo list at the top with some design decisions we might need to make (e.g. saving selected player) feel free to review/add/remove as you see fit or comment here.

[Batwam]

Ah, I was working on this but my github personal token expired! At least it shows we are on the same page. I had some other changes too so I'll try to push that once I've figured out how to update my credentials!

[Moon-0xff]

It should only be a matter of deleting the key from your github account, deleting the expired keys from your .ssh folder, and generating a new key pair

[Batwam]

It should only be a matter of deleting the key from your github account, deleting the expired keys from your .ssh folder, and generating a new key pair

yeah, I had the new token but still needed to re-register it with git remote set-url origin https://user:key@...

[Batwam]

alright, see commit above. I have added grey colour to the source names when in Auto mode. This still allow selection for direct switch to Manual mode without having to disable Auto_Switch first. (Decided to only change colour to grey to allow direct selection rather than set as settingsMenuItem.sensitive = false). I was tempted to make them non-sensitive but selecting a specific source would have required to first untick Switch Automatically and then go back to select the source which I felt was a bit cumbersome.

The colour is hard coded grey and I would have preferred system colour but I checked with Light mode and it looks fine.

[Moon-0xff]

It looks good but i wonder if we can get the color from the theme, as if someone has a custom theme installed it might blend with the background.

Also, line 138 to 142 are repeated statements.

[Batwam]

Thanks for that, there were some merge conflicts as I merged my changes into yours since they overlapped and it looks like I forgot to clean that part out.

Yeah, there is probably a neater way to call the colour actually recommended by the theme, I'll add this to the todo list. I had a look on this page and just tried settingsMenuItem.set_style('style_class:popup-inactive-menu-item') but that didn't work.

[Moon-0xff]

I had a look on this page And insensitive_fg_color might be the one to use. I don't know how to add it to the style though.

Custom styles should have the same name i suppose.

[Batwam]

Yeah, custom themes should have consistent naming. If they are missing there is hopefully some sort of built-in fallback mechanism. Now the challenge is to find how to make use of either this colour, or the style it is included in.

I think that referencing a style might be preferable in case a theme used a different colour than the standard one but let's see what we can get to work and we can take it from there...

[Moon-0xff]

I did some detective work on the gnome-shell source code and learned some things. In a nutshell:

• We can get a color declared in the inherited "theme node file" using lookup_color
• Our widget doesn't inherit insensitive_fg_color

We can, however, get the foreground and background color and mix them in our own, so I did just that.

note: I pushed commits with wrong user.name and email (twice) so i had to force-push 😅

[Batwam]

Nice one, i also did an excessive amount of research on this and did notice yesterday that the colour was actually made of a mix of bg and fg colour $insensitive_fg_color: mix($fg_color, $bg_color, 50%); from the colors.scss file so even if we are not pulling insensitive_fg_color directly, your method would be consistent with the standard method.

Seems like this is as good as it's going to get!

[Batwam]

ok, I had a look at 2 of the 3 outstanding actions and don't think that they need to be addressed:

• moving Build_Menucode into separate menu.js file: it would probably be neater but not strictly necessary, the added code isn't really that long and, more importantly, I gave it a quick try and it turns out I don't really know how to do it with import so that's going to annoy me... I'd say no action unless you know how tackle without wasting too much time (and feel it's necessary).
• use shortname in other parts of the code if it simplifies it: i checked the filter mechanism where it could have been relevant but the filtering happens very early on as it's based on the dBusList which only contains the addresses so these sources get filtered out before they even get a change to get a shortname assigned. I'd say no action there.

This basically leaves us with the last step which it to let things settle down for a while and see if we encounter any issues.

I haven't had any so far. In fact, it works quite nicely. I was playing around with the filters to test what it would do and since we are refreshing everything on each iteration as soon as the filter is removed, next time I click, the sources re-appears in the greyed out source list without any need to close the Settings window or reload the extension.

[Batwam]

quick one. I know that this is out of scope but is there any chance I could slip in some code to remove the "(feat. ...)" and other "Featuring ...)" texts in the label which take too much room and drive me mad? I'd add this as an extra option under the one for Remaster..

Moved to #24

[Moon-0xff]

Sure, but submit a new PR for that, as this one is almost ready to be merged into main

Regarding the leftover actions, i agree with your thoughts.

[Batwam]

No worries, I'll park this for now.

[Moon-0xff]

I tried to connect the new method to an event, but it looks like there isn't an event for "color scheme changes", I also learned that the foreground and background colors changes depending of where or when in the code we get the theme node.

I would say we are done now, so if you agree i will merge it.

[Batwam]

look fine to me. I just had to remove a leftover log statement.

I just pulled the latest change and it's working. Did you re-test on gnome 3.38 considering the recent colour hack?

[Moon-0xff]

I develop the extension on GNOME 3.38 so the question should be about 3.36

[Moon-0xff]

Well, we are done here, regarding 3.36 let's test it when we are ready for release.

[Batwam]

I just fired up the old laptop with Ubuntu 20.04 (Gnome 3.36.8) and everything is working as expected 👍

By the way, the vlc icon shows up too.

[Batwam]

One last thing, will you be taking care of the changelog update?

Once uploaded, you should probably also update the screenshot (or add a new screenshot) to show the menu and maybe also the settings menu if EGO allows multiple screenshots?

[Moon-0xff]

Updating the changelog is a trivial change, regarding the screenshot, i would like to change it for a GIF showcasing the extension, but on my end that would be quite tedious to do.

[Batwam]

I can give it a go. I'll revert back to the normal Gnome layout with panel at the top for it. What do you want to show?

• [ ] just the menu opened and closed?
• [ ] start in Auto mode and select another source to show the icon/title change? I'll load up a few sources sources to show the various players supported.
• [ ] Settings overview as well? I could open the settings from the menu and click each of the 3 tabs
• [ ] Some of the options in action as well (Remaster, icon location, widget location...)?
• [ ] Full screen grab or just a zoomed in view of the panel area? I gave it a quick try as shown below. The default screencast app resolution seems to be a bit low so it might be best to go fullscreen so it looks decent and gives a general overview of the desktop with the extension?

PS: for some reason, the screengrab doesn't show the mouse. This seems to be a reported bug.... I guess it's still ok, even without it.

[Moon-0xff]

I would say it's important that the mouse pointer is shown, also that the resolution is high so the text doesn't look strangely multicolor.

I want to show the extension changing between 3 different sources, showcasing that:

1. The extension supports multiple players
2. You can select between players running simultaneously
3. The icon support
4. Missing/empty fields are simply not displayed

If you leave the controllers of a player in shot you might be able to showcase something else, but the idea is to keep it short.

[Batwam]

Ok, I'll see what I can do. I'm not sure what's going on with the pointer. I tried on X11 and Wayland and it's the same. Others seem to be having similar issues... i might need to try another app than the default one.

For the resolution, I'll check what I get if I do a video of the entire screen. If I zoom in on 10% of my screen, it makes sense that the resolution would be low as it would be 10% of my screen res and I don't have a high DPI display. By the way, is there a resolution limit for the EGO screenshot/screencast?

[Moon-0xff]

There's definitely a size limit (in mb)