Moon-0xff / gnome-mpris-label

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

unknown symbolic icons in label page settings ... #99

Closed dasnoopy closed 4 weeks ago

dasnoopy commented 2 months ago

[very minor issue) ...as per attached screenshot , in label page setting there are some unknown symbcolic icons (see filter segments containing and button placeholder settings) - [ extension version 30 - gnome 46 - default adwaita icon theme)

Screenshot from 2024-04-12 06-46-50

I'm not a developer but I find out that should be involved this part of code in pref.js file. I replaced the info-symbolic icon name with e.g. dialog-information-symbolic and a correct icon appear near the label..

function buildInfoButton(labeltooltip){
    let thisInfoButton = new Gtk.MenuButton({
        valign: Gtk.Align.CENTER,
        icon_name: 'info-symbolic',
        visible: true
    });

thanks Andrea

Moon-0xff commented 2 months ago

Thanks for reporting this!
I don't run 46 myself so I'll have to boot up a VM to test (it could be that your system is missing some files!).

Batwam commented 2 months ago

Yeah, I have already investigated this as I noticed a similar issue in Gnome nightly. Unfortunately, this icon is not part of Adwaita but of the gnome-control-center package.

See gitlab issue raised, hoping that they would consider adding it to Adwaita: https://gitlab.gnome.org/GNOME/adwaita-icon-theme/-/issues/283#note_2027550

The reason it didn't work on gnome nightly was because Gnome Settings was installed as some sort of flatpak so the icon was in the normal location. What OS are you using? Do you have gnome control center installed? Seems quite unlikely that a gnome user wouldn't have Gnome Settings.

When discussing the issue with extensions reviewers, I was advised that best practice was to package the icon as part of the extension.

Using dialog-information-symbolic is also an option but I was initially going for a design similar to Gnome Settings for consistency (with the i rather than a lightbulb) for familiarity. Using help-about-symbolic.svg was also a logical option as it does look like an i intended to be used as help button. However, it used to look like a star in previous gnome versions for some reason so that would look weird in older systems.

Moon-0xff commented 2 months ago

I was wondering where the icon was coming from.
I'm assuming gnome-control-center is or should be packaged alongside gnome.
Does gnome work well without gnome-control-center?

If not then I think this is the result of a broken package or misconfiguration.

If using gnome without gnome-control-center is a valid use case scenario, then I think we should test if the icon is available, if not, replace it with an icon that's explicitly packaged with gnome, or inside a package that gnome depends on (regardless of distro).

Batwam commented 2 months ago

I'd be surprised if anyone has Gnome without gnome-control-center, that's why I didn't think that the potential issue was worth fixing. I'd be interested to know how this is happening to @dasnoopy.

Using an existing icon was my preference but I wasn't able to find anything which matches the gnome-settings look and was consistent between older and newer gnome versions.

The recommended solution was to simply include the icon in the extension and link that directly, perhaps that could be a fallback (pretty sure there is a "fallback" method in St.Icon. Would that be a good solution?

Edit: here it is: https://gnome.pages.gitlab.gnome.org/gnome-shell/st/property.Icon.fallback-icon-name.html

As fallback, we could use a local copy of the current icon or use an Adwaita icon as fallback like dialog-information-symbolic, help-about-symbolic.svg or anything else from your /usr/share/icons/Adwaita folder.

Moon-0xff commented 2 months ago

perhaps that could be a fallback (pretty sure there is a "fallback" method in St.Icon. Would that be a good solution?

In that case it would be a simple one line diff:

diff --git a/prefs.js b/prefs.js
index 1ee7752..f8c735e 100644
--- a/prefs.js
+++ b/prefs.js
@@ -394,6 +394,7 @@ function buildInfoButton(labeltooltip){
        let thisInfoButton = new Gtk.MenuButton({
                valign: Gtk.Align.CENTER,
                icon_name: 'info-symbolic',
+               fallback_icon_name: 'dialog-information-symbolic',
                visible: true
        });
        thisInfoButton.add_css_class('flat');

but thisInfoButton is an instance of Gtk.MenuButton, not of St.Icon, it's worth to try the above diff just in case (I haven't).

Batwam commented 2 months ago

you are right, this fallback method doesn't work. I just tried and got: Error: No property fallback_icon_name on GtkMenuButton.

If we want to keep it simple and avoid shipping a custom icon within the extension, the easiest is probably to either:

It looks like this: Yaru (Ubuntu default) for which the icon interestingly still looks like an i image

Adwaita image

Moon-0xff commented 2 months ago

The commit above will solve this issue for 45/46, expecting that versions below 45 can always get the icon from gnome-control-center-data, this might not be the case.

I believe this is a good compromise, as the icon we had was better, but apparently problematic for GNOME 46.

Batwam commented 2 months ago

Agreed 👍

Batwam commented 2 months ago

Issue closed by 02cdae1

Moon-0xff commented 2 months ago

Issue closed by https://github.com/Moon-0xff/gnome-mpris-label/commit/02cdae19cf95649f5c2e6c687bf6848c821e2c0e

Not yet, a new version needs to be uploaded to EGO.

Moon-0xff commented 4 weeks ago

Version 32 of the extension for GNOME 45/46 includes a fix for this problem (commit https://github.com/Moon-0xff/gnome-mpris-label/commit/02cdae19cf95649f5c2e6c687bf6848c821e2c0e).