Add support for GNOME 43

[Moon-0xff]

It simply adds the removed code for AggregateLayout to widgets.js

This is a quick and simple fix for my own testing purposes, so don't expect much of it :P

[JasonLG1979]

Sorry I missed this. Must have got lost in the noise. I will test this out after work today and if all goes well merge it. Thanks.

[ChrisLauinger77]

thats how it looks like for me
the complete command to pack the extension was
"gnome-extensions pack --podir=../po --extra-source=dbus.js --extra-source=indicatorToolTip.js --extra-source=translations.js --extra-source=widgets.js --force"

[JasonLG1979]

That's what I was afraid of. We may be missing a style class or something? The goal ofc is to match the width of the menu on the far right (whatever they're calling it now?) as it did in previous releases.

[ChrisLauinger77]

its called quicksettings
https://gjs.guide/extensions/upgrading/gnome-shell-43.html#quick-settings
see also
https://gjs.guide/extensions/topics/quick-settings.html#example-usage

[ChrisLauinger77]

I just found they have QuickSettingsLayout as replacement.
https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/main/js/ui/quickSettings.js
I think I can make another proposal how to solve this tomrrow

[Moon-0xff]

Using QuickSettingsLayout it looks like this:

I just copied the entire thing to widgets.js, from line 381 to 579, added GLib, and replaced AggregateLayout to QuickSettingsLayout

This isn't a clean way of doing this but probably maintains compatibility with older versions of GNOME.

[JasonLG1979]

That doesn't look right either though.

[ChrisLauinger77]

https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/gnome-42/js/ui/panel.js
how about using the old AggregateLayout from panel.js inside the extension ?

[Moon-0xff]

That's the commit of this PR

[JasonLG1979]

I'm pretty sure it depends on a style class to set the width.

[Moon-0xff]

AggregateLayout gets the width from vfunc_get_preferred_width. You can set the height creating a function named vfunc_get_preferred_height too.

[Moon-0xff]

Both functions are present in QuickSettings, however they are a lot more complicated.

[JasonLG1979]

The layout doesn't set the width it just makes sure everything stays the proper size. The Aggregate menu style class sets the width. I'm looking for a replacement for that if it exists.

[JasonLG1979]

If a suitable replacement does not exist we can create our own. I'd rather not do that though. I'd rather it be a built-in style class so the extension matched whatever theme more.

[giinuu]

I might be totally missing something here, messing around locally I noticed that commenting out line #1427 fixes the width issue.

This part could be totally essential and I'm missing something, tried Rhythmbox, YT Music and Youtube - all seem to work well.

[doronbehar]

I tested this patch along with commenting out line 1427 as @giinuu suggested and it's working great for me. Thanks for everyone who tested and contributing :) Would be great for others if this got a bit of attention.

[petar-v]

Hi! Thanks for that extension, it's great! I have missed having it! :)

I just tested it on my end as well. Looks alright IMHO, and it does the job.

Moreover, this is tested on Gnome v44. It works perfectly as long as the metadata.json includes that version. :)

[Moon-0xff]

Commenting 1427 negates this PR, indeed is the only change the extension needs to run on 43/44

[Mershl]

Commenting 1427 negates this PR, indeed is the only change the extension needs to run on 43/44

Is it still working for you on 44 @Moon-0xff ? Fedora 38 throws an JS error for me:

JS ERROR: TypeError: Gtk.IconTheme.get_default is not a function

Location: https://github.com/JasonLG1979/gnome-shell-extension-mpris-indicator-button/blob/master/mprisindicatorbutton%40JasonLG1979.github.io/dbus.js#L1915

Someone know an alternative to Gtk.IconTheme.get_default() or is it just gone with Gtk4?
-> https://gjs.guide/extensions/upgrading/gnome-shell-44.html#gtk-icontheme

[Moon-0xff]

@Mershl I can't test right now but probably you are commenting the wrong line. If you are starting from JasonLG1979:master the line number is 1428. The line to comment is this one: this.menu.box.set_layout_manager(new AggregateLayout());

[Mershl]

@Mershl I can't test right now but probably you are commenting the wrong line. If you are starting from JasonLG1979:master the line number is 1428. The line to comment is this one: this.menu.box.set_layout_manager(new AggregateLayout());

I'm seeing a seperate issue on 44 regarding the default IconTheme, that is also mentioned in the porting guide of 44 (linked in my previous post). I was just curious why it was working for you on 44, but maybe this interface change was introduced late.

[Moon-0xff]

I tested this when 44 was still in beta so yeah, might be a change introduced after my tests. Have you tried to replace the Gtk.IconTheme statements to St.IconTheme?

[Mershl]

        const iconTheme = new St.IconTheme();
        return this.app_id && (iconTheme ? iconTheme.has_icon(iconName) : false) ? Gio.ThemedIcon.new(iconName) : null;

seems to work nicely on 44. I'm not sure when St12, the first version of this API supporting .IconTheme.has_icon was released though. This could likely break for 43 and lower.