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

Use clutter for the background menu #1769

Closed
wants to merge 34 commits into from
Closed

Conversation

leolost2605
Copy link
Member

@leolost2605 leolost2605 commented Sep 29, 2023

The current way of handling window and desktop menus doesn't work under wayland because the daemon can't position the menus and AFAIK they don't work properly without a parent (see @Marukesu's comment here)

With this PR we add a Gala.Menu Clutter Actor that is then used to create the background menu. In a follow up the same can be used (with some additional effort) for the window menu.

The appearing and disappearing of the menu is currently slightly animated not sure if we want to keep that (I just took it from the WindowSwitcher).
The original Gtk.Menu's add some additional space to the end of the menu items. When porting to GTK4 we don't do that when replacing Menus with popovers, and popovermenus don't do it by themselves so I didn't do it here.
Fixes #1695 and sets the foundation for a follow up to f*x #1696

@leolost2605 leolost2605 marked this pull request as ready for review October 1, 2023 13:50
@leolost2605 leolost2605 requested a review from a team October 1, 2023 13:50
@leolost2605 leolost2605 changed the title Use clutter for desktop menu Use clutter for the background menu Oct 1, 2023
@lenemter
Copy link
Member

lenemter commented Oct 1, 2023

Seems like the menus don't get properly destroyed and still take input after being closed.

2023-10-02.00-21-58.mp4

Second, they can go off screen when clicked close to the screen edge.

image


Third, seems like there is no black outline in dark mode unlike other menus. I briefly looked into the stylesheet code and couldn't find why it's broken.

And the last, the menu is shorter for some reason. Menus' min-width is clearly 150px in stylesheet, but in reality it's 205?

@danirabbit
Copy link
Member

I’m a little concerned about like needing to create and maintain half a toolkit here to support everything that Gtk does with a11y and localization etc. maybe @tintou has thoughts/suggestions.

I wonder if it would be better to just have a full screen transparent window so that it could position its own popover?

@tintou
Copy link
Member

tintou commented Oct 1, 2023

Yes I think that the background menu is an exact usecase of Meta.WaylandClient as it is tightly tied to Gala

@lenemter
Copy link
Member

lenemter commented Oct 7, 2023

@danirabbit @tintou Gala still needs a Gala.Menu for window menus. So why don't just use Gala.Menu for everything?

And talking about gala's toolkit, it has many a11y issues e.g. #1301, but that doesn't mean we need to replace say window switcher with Gtk implementation. We should put more effort into developing gala instead of searching hacky solutions with Gtk.

@leolost2605
Copy link
Member Author

As @lenemter said the window menu was my primary concern too. I think we would have to go very hacky to get that working with only GTK...?
Because the main issue I had was being able to close the menu by clicking outside of it.
I tried doing a push modal with the WindowActor for a daemon Gtk.Window which didn't work though but would that be possible in theory and I just did something wrong? Or maybe there is a way to get all mouse button clicks in gala before they get consumed by a child?

Copy link
Member

@lenemter lenemter left a comment

Choose a reason for hiding this comment

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

Oops I forgot to click submit button


menuitem.leave_event.connect (() => {
selected = null;
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return false;
return Clutter.EVENT_PROPAGATE;


menuitem.enter_event.connect (() => {
selected = menuitem;
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return false;
return Clutter.EVENT_PROPAGATE;

@leolost2605
Copy link
Member Author

See #1844

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.

No context menu on wallpaper in Wayland
4 participants