After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740104 - support GtkPopover
support GtkPopover
Status: RESOLVED FIXED
Product: glade
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Glade 3 Maintainers
Glade 3 Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-14 04:43 UTC by Matthias Clasen
Modified: 2015-12-13 05:11 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Matthias Clasen 2014-11-14 04:43:39 UTC
Initial support for GtkPopover as a container can be found on the popover branch.
Comment 1 Matthias Clasen 2014-11-20 02:14:44 UTC
I've pushed more work to that branch, including support for the 3.15 additions of GtkPopoverMenu and GtkModelButton. The branch relies on recent fixes on the GTK+ side.
Comment 2 Tristan Van Berkom 2015-01-18 07:07:56 UTC
Here are the initial problems I found in the popover branch:

 o GtkPopover, GtkPopoverMenu and GtkModelButton do not set the 'since' 
   attribute in the catalog indicating which stable GTK+ version they are 
   introduced in.

 o GtkPopover is visible by default, all widgets in Glade except for toplevels
   are visible by default, but this should be reversed for GtkPopover, as
   showing the popover is something usually you do conditionally, the result
   is that designing a UI and setting the popover to have a related widget
   causes the popover to just display itself when loading the UI (see this
   happen by previewing a UI created with a popover).

   Should be a simple matter of setting the visible default to 'False' in
   the catalog for popovers

 o GtkPopover and GtkPopoverMenu can be 'Added' to a widget in the project.

   Setting: toplevel="True" on the GtkPopover definition in the catalog fixes
   this partially, as 'clicking' on the GtkPopover results in adding one
   at the toplevel.

   But it still remains possible to drag and drop the popover onto a 
   placeholder.

   Fixing this will probably require an additional clause in 
   glade_gtk_container_add_verify().

 o GtkModelButton needs to have the 'icon' property disabled, currently trying 
   to set it brings up a 'Please choose a GIcon from this project' dialog - but 
   Glade still has no way of editing GIcon properties.

   I see how you made the create type GThemedIcon and it happens to do something
   useful, however I would prefer to leave it disabled for now - as Glade does
   not yet have an adaptor for GThemedIcon and all GIcon properties in Glade are
   still disabled, would be preferrable to add support for GThemedIcon in a 
   separate work and ensure that all 'gicon' widget properties start working.

 o It would be really great to have an icon for GtkModelButton, but I dont know 
   what it would look like.

 o Also GtkModelButton should not be at the end of the display widgets list, we 
   have grouped all the 'button' classes together in the hope that they are 
   easier to locate in the palette, it should probably be either the last 
   button type (after GtkLockButton and before GtkSwitch), or at least come 
   directly after the primative button types (after GtkRadioButton and before 
   GtkMenuButton).

 o Is there a reason to have disabled the GtkModelButton:role property ?

   If the model button is only useful with the action set and the 'role'
   is overridden by the action name, then I concur there is little use
   for exposing the property in Glade.

Looks like this branch will be easier than the other container related branches,
as the popover has only a single child and is pretty straight forward.
Comment 3 Matthias Clasen 2015-01-18 19:12:35 UTC
(In reply to comment #2)
> Here are the initial problems I found in the popover branch:

Thanks for getting back to this!
 
>  o GtkPopover, GtkPopoverMenu and GtkModelButton do not set the 'since' 
>    attribute in the catalog indicating which stable GTK+ version they are 
>    introduced in.

I didn't do that since these are 3.16 additions. if I set since="3.16", that will make testing this with gtk 3.15 difficult, right ?

>  o GtkPopover is visible by default, all widgets in Glade except for toplevels
>    are visible by default, but this should be reversed for GtkPopover, as
>    showing the popover is something usually you do conditionally, the result
>    is that designing a UI and setting the popover to have a related widget
>    causes the popover to just display itself when loading the UI (see this
>    happen by previewing a UI created with a popover).
> 
>    Should be a simple matter of setting the visible default to 'False' in
>    the catalog for popovers

Indeed, pushed that change to the branch.

>  o GtkPopover and GtkPopoverMenu can be 'Added' to a widget in the project.
> 
>    Setting: toplevel="True" on the GtkPopover definition in the catalog fixes
>    this partially, as 'clicking' on the GtkPopover results in adding one
>    at the toplevel.
> 
>    But it still remains possible to drag and drop the popover onto a 
>    placeholder.
> 
>    Fixing this will probably require an additional clause in 
>    glade_gtk_container_add_verify().

Fixed both of these.
 
>  o GtkModelButton needs to have the 'icon' property disabled, currently trying 
>    to set it brings up a 'Please choose a GIcon from this project' dialog - but 
>    Glade still has no way of editing GIcon properties.
> 
>    I see how you made the create type GThemedIcon and it happens to do
> something
>    useful, however I would prefer to leave it disabled for now - as Glade does
>    not yet have an adaptor for GThemedIcon and all GIcon properties in Glade
> are
>    still disabled, would be preferrable to add support for GThemedIcon in a 
>    separate work and ensure that all 'gicon' widget properties start working.
> 
>  o It would be really great to have an icon for GtkModelButton, but I dont know 
>    what it would look like.
> 
>  o Also GtkModelButton should not be at the end of the display widgets list, we 
>    have grouped all the 'button' classes together in the hope that they are 
>    easier to locate in the palette, it should probably be either the last 
>    button type (after GtkLockButton and before GtkSwitch), or at least come 
>    directly after the primative button types (after GtkRadioButton and before 
>    GtkMenuButton).

Added an icon and moved GtkModelButtons to the other buttons.
 
>  o Is there a reason to have disabled the GtkModelButton:role property ?
> 
>    If the model button is only useful with the action set and the 'role'
>    is overridden by the action name, then I concur there is little use
>    for exposing the property in Glade.

Yeah, when I made GtkModelButton 'fit for export', I made it so that you're only supposed to set the action-name, everything else follows from that.
Comment 4 Tristan Van Berkom 2015-01-19 05:11:22 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Here are the initial problems I found in the popover branch:
> 
> Thanks for getting back to this!
> 
> >  o GtkPopover, GtkPopoverMenu and GtkModelButton do not set the 'since' 
> >    attribute in the catalog indicating which stable GTK+ version they are 
> >    introduced in.
> 
> I didn't do that since these are 3.16 additions. if I set since="3.16", that
> will make testing this with gtk 3.15 difficult, right ?
> 

Not exactly, this has been 'a thing' for a very long time by now, basically
most of the checks are completely artificial and depend on the 'targettable'
versions in the toplevel <glade-catalog> attributes (if you rebase your
branch against master you will see I have already added 3.16 there so it
becomes a valid target).

There are some cases where it's worked around, for instance during transitional
periods we have conditional code which syncs the runtime with the actual 
property value depending on the version, like here:

https://git.gnome.org/browse/glade/tree/plugins/gtk+/glade-gtk-scale.c#n44

In those cases we always just add 1 to GTK+'s binary minor version, so if
a 3.15 dev version of GTK+ is installed, we still sync properties marked
as the inexistent '3.16'...

Anyway that's the long winded answer, short answer is no, it's not a problem
and should just be immediately marked as 3.16 while the current dev version
is 3.15 :)
Comment 5 Tristan Van Berkom 2015-02-02 05:25:23 UTC
Hi,

There only a couple issues remaining with GtkPopoverMenu.


I'm getting crashes from glade_gtk_popover_menu_visible_submenu_changed()

  This can be triggered as such:

  1.) Create a GtkWindow 
  2.) Drag a GtkPopoverMenu and drop it in the window
  3.) A popup correctly pops up and says you cant add a popover to a window
  4.) Click "OK" (no choices of course)

  Now crash occurs in the said function, I didnt investigate exactly why,
  only saw the stack trace.


I'm also getting crashes when setting the "submenu" packing property of a 
button that I added to the popover menu. This one is a bit more obscure, it 
seems to crash in gtk_widget_set_tooltip_text(), which is showing some tooltip 
text about the property...

This second part seems to be a GTK+ bug, maybe it's already fixed in GTK+
master ?
Comment 6 Tristan Van Berkom 2015-02-02 05:30:24 UTC
Note, I rebased your popover branch against master, which has the new GTK+
versions declared in the catalog.

I was only going to rebase locally, but as I dealt with a couple of conflicts
in gtk+.xml.in I thought I'd just push the freshly rebased branch so you wouldnt
need to deal with those.
Comment 7 Matthias Clasen 2015-03-02 04:12:54 UTC
Pushed a fix for the first crash to the branch.
Comment 8 Matthias Clasen 2015-11-18 15:42:23 UTC
Also pushed a fix for an issue with editing the submenu child property
Comment 9 Ben 2015-11-18 20:54:34 UTC
I have a branch rebased on master. I also stopped emission of button-press-event, button-release-event, and key-press-event, so e.g. pressing escape doesn't hide the popover in glade.
Comment 10 Tristan Van Berkom 2015-12-09 03:28:06 UTC
Reviewed "popover" branch and looks good.

(In reply to Ben from comment #9)
> I have a branch rebased on master. I also stopped emission of
> button-press-event, button-release-event, and key-press-event, so e.g.
> pressing escape doesn't hide the popover in glade.

Ben, I dont see this in your wip branch. This looks to be the last issue with the popover branch that I can see.


Matthias, please merge the popovers branch.

Let's open a new bug to deal with the ESC key causing popovers to disappear from the workspace.
Comment 11 Tristan Van Berkom 2015-12-09 03:39:15 UTC
Ben, regarding the keynav issue I see it now in your branch, it's just hidden below on of Matthias's commits.

Please squash it into a single commit so that it does not add any commented code.

I think there are issues with your stop_emission() approach so please do not merge (right now if I click on a popover with your branch and then try to keynav out of the popover, it both does not work, and I get an endless supply of warning messages in the console which say:

(glade:21407): Gtk-WARNING **: State 36 for GladeDesignLayout 0x22567c0 doesn't match state 192 set via gtk_style_context_set_state ()

As I said, we should probably treat this ESC issue as a separate one, and probably we should trap the key-release-event and conditionally return TRUE if the event->keyval is ESC so as to avoid breaking keynav completely.
Comment 12 Ben 2015-12-10 00:39:25 UTC
I took the stop_emission code from https://git.gnome.org/browse/glade/tree/plugins/gtk+/glade-gtk-dialog.c

Also, I think the warnings are unrelated to popovers.
Comment 13 Tristan Van Berkom 2015-12-13 05:11:42 UTC
(In reply to Tristan Van Berkom from comment #10)
[...]
> Let's open a new bug to deal with the ESC key causing popovers to disappear
> from the workspace.

Opened bug 759395 to track the disappearing popover issue.