elementary / videos

Video player and library app designed for elementary OS
https://elementary.io
GNU General Public License v3.0
59 stars 18 forks source link

LibraryItem: Use Gtk.Popover instead of Gtk.Menu #301

Closed danirabbit closed 2 years ago

danirabbit commented 2 years ago

In preparation for Gtk4

alice-mkh commented 2 years ago

This doesn't do much for preparing for GTK4 unfortunately. :)

  1. GtkModelButton is private, you have to use menu models
  2. Popover API in GTK4 is significantly different

I'd suggest to instead keep GtkMenu but create it from a menu model. Then it would be fairly easy to replace it with a popover in GTK4 (and also it won't introduce an unwanted arrow - GtkPopover:has-arrow is GTK4-only).

jeremypw commented 2 years ago

You can synthesise a rectangle at which to popup the menu from the button event that triggers it - presumably this can also be done in Gtk4. However, you may be right that there is not much to gain dealing with this menu in advance of the port.

alice-mkh commented 2 years ago

In gtk4 there's no need to do that because you do popover.set_offset (x, y);

But I mean:

In gtk4 you don't create a popover for a widget - the popover is instead a widget's child. Your size_allocate() must be aware of it and do popover.present(). etc. And implementation-wise it's a completely different widget really, if anything it behaves more like the current GtkMenu. :)

But you can't have GtkModelButtons, so pretty much all of your code will have to go away. You instantiate GtkPopoverMenu from a menu model - that's the only way to create menus, and in gtk3 you can create GtkMenus from the same models. But as is you're replacing one non-portable implementation with another - either way as is you'll have to replace most of it.

danirabbit commented 2 years ago

Yeah closing. Not sure where I was going with this