Skip to content

Conversation

@moises2657
Copy link

New context option to close all windows of an app on the dock:

elementary/gala#2196

Copy link
Member

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, once again thank you!
I've added some comments how things could be made a bit cleaner.
In general I would move everything to the app class instead of having it in the launcher. Also instead of adding and removing the action and menuitem you can keep both around and set the action enabled depending on whether the app is running. Then you can add the attribute hidden-when with value action-disabled to your menuitem so that it will automatically be hidden when the action is disabled (see https://valadoc.org/gtk4/Gtk.PopoverMenu.html there's a list of supported attributes).

@leolost2605 leolost2605 requested a review from a team January 2, 2025 12:55
@moises2657 moises2657 requested a review from leolost2605 January 2, 2025 17:27
LauncherManager.get_default ().sync_pinned ();
});

var close_item = new GLib.MenuItem (_(CLOSE_WINDOWS_LABEL), ACTION_PREFIX + CLOSE_WINDOWS_ACTION);
Copy link
Member

@lenemter lenemter Mar 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't add Close All Windows as a constant, it's untranslatable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, one can if it is marked as N_,
So it has to be:

    private const string CLOSE_WINDOWS_LABEL = N_("Close All Windows");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I don't see the benefit of a constant here

@lenemter lenemter added this to OS 9 Mar 13, 2025
@lenemter lenemter moved this to Needs Review in OS 9 Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

4 participants