Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System tray #70

Merged
merged 54 commits into from
Sep 4, 2023
Merged

System tray #70

merged 54 commits into from
Sep 4, 2023

Conversation

kotontrion
Copy link
Contributor

@kotontrion kotontrion commented Aug 28, 2023

The system tray is working with only a few issues left (see list below).
Some applications behave different than expected, so maybe more testing is required.

introduces a dependency on @girs/dbusmenu-0.4
fixes #22

usage

Here is small snippet you can use in your config.

const systemtray = Box({
    connections: [[SystemTray, box => {
        const arr = SystemTray.items;
        box.children = arr.map(item => Button({
            //call the Activate function when icon is clicked
            //Note: if item.ItemIsMenu is true, left click should open menu
            onPrimaryClick: (_, event) => item.ActivateAsync(event.get_root_coords()[1], event.get_root_coords()[2]),
            //open menu on right click.
            //Note: if item.Menu is not set item.ContextMenuAsync(x, y) should be called.
            onSecondaryClick: (_, event) => item.AgsMenu.popup_at_pointer(event),
            //show icon
            child: SystemTray.get_icon(item),
            tooltipMarkup: SystemTray.get_tooltip_markup(item)
        }));
    }]],
});

Known issues:

  • some applications don't register an item, when they are started before the systemtray is ready (maybe search the dbus for items on start?)
  • Sometimes the menus height is too small and requires scrolling. Closing it and reopening fixes it. I'm not that familiar with GTK and don't know what causes this behavior.

@ferrreo
Copy link

ferrreo commented Aug 29, 2023

Found a few issues with this.

Steam icon has no tooltip so get_tooltip_markup blows up - needs a truthy check and early return.

get_pixbuf always blows up on icon change for discord when you get a message or if you read a pending message.

I don't seem to be able to control the size of the icons even setting font-size on various things. I ended up adding a size param to get_icon just so I could get it to work.

@ferrreo
Copy link

ferrreo commented Aug 29, 2023

I managed to fix the icon change by doing a _refresh_all_properties when NewIcon signal is received and changed the logic in getIcon to instead of checking status (as status here is "active") instead just use the attentionicon/pixmap if there is one otherwise use the regular.

@kotontrion
Copy link
Contributor Author

Steam icon has no tooltip so get_tooltip_markup blows up - needs a truthy check and early return.

fixed

get_pixbuf always blows up on icon change for discord when you get a message or if you read a pending message.

fixed

it is weird, that refreshing only the icon properties is not sufficient.
Thanks for testing and identifying the issue.

@ferrreo
Copy link

ferrreo commented Aug 29, 2023

One more minor thing, when an icon updates it's position in the output array is changing. So when you re-render icon goes from first to last for example. Might want to add some kind of stable sort to the getter for the items.

@kotontrion
Copy link
Contributor Author

Icons can't update their position. The array of the item is always sorted by time of registration. The order is already stable.

@ferrreo
Copy link

ferrreo commented Aug 29, 2023

Might have been discord doing something weird as it seems fine after restarting it.

@ferrreo
Copy link

ferrreo commented Aug 29, 2023

Getting this on one of my systems but not the other

image

Missing runtime dep maybe or weirdness with the official discord deb?

@kotontrion
Copy link
Contributor Author

The Error UnknownMethod is raised, when the called method is not implemented by the dbus interface. But the only method being called here is the Get method of the org.freedesktop.DBus.Properties interface, which should be always there, except maybe the item disappeared from the dbus before this was called.

I will look into this more closely in the coming days.

I guess more error checking is needed in general.

Note: maybe it would be better to open such issues over at my fork to keep this PR more clean and have a better overview which has to be done.

Sometimes the menus height is too small and requires scrolling. Closing it and reopening fixes it. I'm not that familiar with GTK and don't know what causes this behavior.

I rewrote the css in my config and this issue went away. So maybe this isn't an issue with the system tray implementation.

@Aylur
Copy link
Owner

Aylur commented Aug 29, 2023

some applications don't register an item, when they are started before the systemtray is ready (maybe search the dbus for items on start?9

If its possible to check if a dbus name corresponds to a SNI, then yeah, looping over them then registering them would be a solution, but I don't see this as much of an issue, because the end goal is to have a config that will run from start to finish throughout a session and there shouldn't be a need to restart ags

Sometimes the menus height is too small and requires scrolling. Closing it and reopening fixes it. I'm not that familiar with GTK and don't know what causes this behavior.

Most probably due to custom css.

I kind of don't like doing this

arr.map(item => Button({
  child: SystemTray.get_icon(item),
  tooltipMarkup: SystemTray.get_tooltip_markup(item)
}));

Making a wrapper class for item proxies would be better imo, so something like this is possible

SystemTray.items.map(item => Button({
  child: Icon(),
  connections: [[item, button => {
    button.child.icon = item.icon;
    button.tooltipMarkup = item.tooltipMarkup;
  }]],
});

item.icon could be either pixbuf or a string, Widget.Icon would need some modification to support pixbufs like this

Also having an added and removed signal on SystemTray would be a nice addition, then these buttons would be only constructed once and not on every update

implemented added/removed signals
@kotontrion
Copy link
Contributor Author

Making a wrapper class for item proxies would be better imo

done
this is a working config: (there are signals for items added/remove, but not used in this example)

const systemtray = Box({
  connections: [[SystemTray, box => {
    box.children = SystemTray.items.map(item => Button({
      child: Icon(),
      onPrimaryClick: (_, event) => item.activate(event),
      onSecondaryClick: (_, event) => item.openMenu(event),
      connections: [[item, button => {
        button.child.icon = item.icon;
        button.tooltipMarkup = item.tooltipMarkup;
      }]],
    }));
    }
  ]]
});

@aestheticjmack
Copy link

I'm a bit of a newbie to AGS, how do I implement this into my config?

@matt1432
Copy link
Contributor

I am trying to build this branch on NixOS but I keep getting this error when launching ags :

(ags:1863921): Gjs-CRITICAL **: 00:14:18.268: JS ERROR: Error: Requiring Dbusmenu, version none: Typelib file for namespace 'Dbusmenu' (any version) not found
require@resource:///org/gnome/gjs/modules/esm/gi.js:16:28
@gi://Dbusmenu:3:25


(ags:1863921): Gjs-CRITICAL **: 00:14:18.268: Module file:///etc/profiles/per-user/matt/bin/ags threw an exception

I tried adding libdbusmenu in buildInputs but it wouldn't change anything.

@cafetestrest
Copy link

Also there are more problems that I was able to find fiddling with copyq.

Right click behavior on copyq for ags system tray is not consistent.
Once I get it like on previous screenshots, other times I get it like so: https://imgur.com/a/trZmF3z

When I click on three dots (...) I get the following in my console:
(ags:3526): LIBDBUSMENU-GLIB-WARNING **: 17:50:56.657: Unable to replace properties on 0: Error getting properties for ID

@Aylur
Copy link
Owner

Aylur commented Sep 2, 2023

Right click behavior on copyq for ags system tray is not consistent.
Once I get it like on previous screenshots, other times I get it like so: https://imgur.com/a/trZmF3z

that is most probably due to custom css

@cafetestrest
Copy link

Yeah some of them are, but still got to experience the similar behavior with example config.

I figured it is changing only once I search for something under copyq (this is in turn narrowing down the results - can be achieved with the left click of the icon) thus it somehow keeps that state and displays in in the system tray.

@Aylur
Copy link
Owner

Aylur commented Sep 2, 2023

with raiobuttons and checkboxes, both are not implemented yet.

with libdbusmenu-gtk3 now they are, it simplified the code a lot too.

LIBDBUSMENU-GLIB-WARNING **: 17:50:56.657: Unable to replace properties on 0: Error getting properties for ID

I think this is an issue with copyq

@Aylur
Copy link
Owner

Aylur commented Sep 2, 2023

here are some examples you can

// will give you a warning about destroyed widgets if there are multiple items registering
// but you can ignore that
const SysTray = () => Box({
    connections: [[SystemTray, box => {
        box.children = SystemTray.items.map(item => Button({
            child: Icon(),
            onPrimaryClick: (_, event) => item.activate(event),
            onMiddleClick: (_, event) => item.secondaryActivate(event),
            onSecondaryClick: (_, event) => item.openMenu(event),
            connections: [[item, button => {
                button.child.icon = item.icon;
                button.tooltipMarkup = item.tooltipMarkup;
            }]],
        }));
    }]]
});
// this one uses a MenuBar and shouldn't throw that destroyed widget warning
const SysTrayItem = item => MenuItem({
    child: Icon({
        size: 24,
    }),
    submenu: item.menu,
    connections: [[item, btn => {
        btn.child.icon = item.icon;
        btn.tooltipMarkup = item.tooltipMarkup;
    }]]
});

export const SysTray = () => Widget({
    type: Gtk.MenuBar,
    className: 'systray',
    properties: [
        ['items', new Map()],
        ['onAdded', (box, id) => {
            const item = SystemTray.getItem(id);
            if (box._items.has(id) || !item)
                return;

            const widget = SysTrayItem(item);
            box._items.set(id, widget);
            box.add(widget);
            box.show_all();
        }],
        ['onRemoved', (box, id) => {
            if (!box._items.has(id))
                return;

            box._items.get(id).destroy();
            box._items.delete(id);
        }],
    ],
    connections: [
        [SystemTray, (box, id) => box._onAdded(box, id), 'added'],
        [SystemTray, (box, id) => box._onRemoved(box, id), 'removed'],
    ],
});

DbusmenuGtk3.Menu also seems to throw accel_group warning, which I assume is a layer-shell issue

@Aylur Aylur merged commit 665c46b into Aylur:main Sep 4, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System tray
6 participants