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

fix: hide tray when there are no non-passive icons #3833

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Bahnschrift
Copy link

Currently, the tray module automatically hides when there are no tray icons. However, this behavior is not consistent with the "show-passive-items" config property. If there are passive tray items (e.g. Spotify) and "show-passive-items" is false (as it is by default), the tray will still be visible.

This PR fixes this by only hiding the tray if there are no visible tray icons.

@Bahnschrift
Copy link
Author

Marking as draft for now because creating a new tray icon does not un-hide an already hidden tray.

@Bahnschrift
Copy link
Author

This should fix it, but I don't think this is the best solution.

I'm not particularly familiar with Gtk development, so my workaround here is to record the config option "show-passive-items" in the tray widget, then check if the passive class has been added to the style of any icon in the tray.

@Bahnschrift Bahnschrift marked this pull request as ready for review December 19, 2024 01:34
@RobertMueller2
Copy link
Contributor

This should fix it, but I don't think this is the best solution.

I'm not particularly familiar with Gtk development, so my workaround here is to record the config option "show-passive-items" in the tray widget, then check if the passive class has been added to the style of any icon in the tray.

I'm not sure I fully understand this. But I think this is happening:
1st commit:

  • if show-passive-item is false, Item is initialized with set_visible(false) in its constructor
  • at this point, tray is updated, but get_visible is still false
  • then, Item::setStatus sets visibility and class
  • but tray isn't updated after that right away

after 2nd commit:

  • if show-passive-item is false, Item is initialized with set_visible(false) in its constructor
  • at this point, tray is updated, and as the new item doesn't have passive class yet, tray is shown, regardless of whether icon will be passive or not
  • then, Item::setStatus sets visibility and class
  • tray isn't updated after that right away

If I'm correct, the tray would be activated by a new icon that ends up as passive. Am I though? I guess you've gathered some insight about what's happening in detail. I can't promise I can come up with a suggestion, but maybe you can elaborate on your findings, please? :)

@Bahnschrift
Copy link
Author

I'm not quite sure how the whole update loop works, since I'm new to the codebase. From what I can gather, if an Item (icon) is created as passive, it has the CSS class "passive" added to it, and so we can just check if any child widgets of the tray have that class.

However, I think you are right that if the status of an Item changes while the icon exists, it may not update correctly. I'm not sure if Tray::update() is called if an Item's status changes (and I guess it wouldn't really make sense for that to be the case), and I'm not even sure if that's something that can happen. I imagine all this would be part of the FreeDesktop specification, but I can't seem to find the relevant documentation.

I've been using these changes for a few days now though and I confirm that it at least seems to work assuming that the status of Items doesn't change after they are created (e.g. opening Spotify with show-passive-items as false no longer shows a visible, but empty tray.

I think an ideal solution to this would be to have the Tray and it's Items have some way of communicating with each other that is more than just looking at the styles of the Items widgets, but I'm not really familiar enough with the code yet to implement that, though I'm happy to give it a go.

@RobertMueller2
Copy link
Contributor

I think an ideal solution to this would be to have the Tray and it's Items have some way of communicating with each other that is more than just looking at the styles of the Items widgets, but I'm not really familiar enough with the code yet to implement that, though I'm happy to give it a go.

I agree. Status looks like the only thing to me that should affect the tray, but according to https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/ it appears possible that the Status changes. In that case, the tray should be updated. It already has Tray::update, so that just needs to find its way into Item::SetStatus.

I tried this real quick in https://github.com/RobertMueller2/Waybar/tree/pr_3833_m based on your first commit. I've tested spotify and other apps closing and opening etc and it looks ok. Feel free to make use of this, but: my understanding of all this is limited as well, I have not paid much attention to the naming, and I've not tested it very thoroughly.

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.

2 participants