[Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

Pavel Grunt pgrunt at redhat.com
Mon Feb 20 12:53:11 UTC 2017


Hi,

On Mon, 2017-02-20 at 12:44 +0100, Victor Toso wrote:
> Hi,
> 
> On Fri, Feb 17, 2017 at 11:24:55AM +0100, Pavel Grunt wrote:
> > GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> > deprecated. Just use the GtkGrid and GtkContainer api.
> 
> Sure
> 
> > ---
> > GtkContainer is an abstract class, so let's use the Grid with the
> > vertical orientation
> 
> At first I thought this was a ABI break but it was questioned in the
> public IRC and the SpiceUsbDeviceWidget should not be subclassed, so
> this should be fine. The chat is below for reference.
> 
> pgrunt | i have this patch https://paste.fedoraproject.org/.. is it
> abi
>          break ? SpiceUsbDeviceWidget is publically a GtkWidget
>          https://www.spice-space.org/api/spice-gtk/spice-gtk-SpiceUs
> bDeviceWidget.html,
>          but internally I am changing it from GtkBox to
> GtkContainer, so
>          binary is different api is same
> teuf   | yeah, depends if we consider it a valid/likely use case to
>          derive from SpiceUsbDeviceWidget or not
> teuf   | (ie whether G_DECLARE_FINAL_TYPE could be used in the
> header or
>          not)
> pgrunt | hm, it should be final
> teuf   | note that it's glib 2.44+ only
> pgrunt | well i can keep the type GtkBox and use the gtk_container
> api
>          anyway
> 
> > ---
> >  src/usb-device-widget.c | 25 ++++++++++++++-----------
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
> > index b394499..a7bbe8f 100644
> > --- a/src/usb-device-widget.c
> > +++ b/src/usb-device-widget.c
> > @@ -40,14 +40,14 @@
> >  
> >  struct _SpiceUsbDeviceWidget
> >  {
> > -    GtkVBox parent;
> > +    GtkGrid parent;
> >  
> >      SpiceUsbDeviceWidgetPrivate *priv;
> >  };
> >  
> >  struct _SpiceUsbDeviceWidgetClass
> >  {
> > -    GtkVBoxClass parent_class;
> > +    GtkGridClass parent_class;
> >  
> >      /* signals */
> >      void (*connect_failed) (SpiceUsbDeviceWidget *widget,
> > @@ -94,7 +94,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
> >  
> >  static guint signals[LAST_SIGNAL] = { 0, };
> >  
> > -G_DEFINE_TYPE(SpiceUsbDeviceWidget, spice_usb_device_widget,
> > GTK_TYPE_BOX);
> > +G_DEFINE_TYPE(SpiceUsbDeviceWidget, spice_usb_device_widget,
> > GTK_TYPE_GRID);
> >  
> >  static void
> > spice_usb_device_widget_get_property(GObject     *gobject,
> >                                                   guint        pro
> > p_id,
> > @@ -163,20 +163,22 @@
> > spice_usb_device_widget_show_info_bar(SpiceUsbDeviceWidget *self,
> >      gtk_info_bar_set_message_type(GTK_INFO_BAR(info_bar),
> > message_type);
> >  
> >      content_area =
> > gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
> > -    hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 12);
> > +    hbox = gtk_grid_new();
> > +    gtk_grid_set_column_spacing(GTK_GRID(hbox), 12);

this hbox widget is a child of SpiceUsbDeviceWidget (GtkGrid). The
hbox contains an icon and label.

The g_object_new below is related to the SpiceUsbDeviceWidget itself
which is vertically oriented -> row spacing.

> 
> It is horizontal grid but you are set column space. I guess it
> should be
> _row_spacing() - although I can't see any problem in my test.
> 
> >      gtk_container_add(GTK_CONTAINER(content_area), hbox);
> >  
> >      widget = gtk_image_new_from_icon_name(stock_icon_id,
> >                                            GTK_ICON_SIZE_SMALL_TOO
> > LBAR);
> > -    gtk_box_pack_start(GTK_BOX(hbox), widget, FALSE, FALSE, 0);
> > +    gtk_container_add(GTK_CONTAINER(hbox), widget);
> >  
> >      widget = gtk_label_new(message);
> > -    gtk_box_pack_start(GTK_BOX(hbox), widget, TRUE, TRUE, 0);
> > +    g_object_set(G_OBJECT(widget), "expand", TRUE, NULL);
> > +    gtk_container_add(GTK_CONTAINER(hbox), widget);
> >  
> >      priv->info_bar = gtk_alignment_new(0.0, 0.0, 1.0, 0.0);
> >      gtk_alignment_set_padding(GTK_ALIGNMENT(priv->info_bar), 0,
> > 0, 12, 0);
> >      gtk_container_add(GTK_CONTAINER(priv->info_bar), info_bar);
> > -    gtk_box_pack_start(GTK_BOX(self), priv->info_bar, FALSE,
> > FALSE, 0);
> > +    gtk_container_add(GTK_CONTAINER(self), priv->info_bar);
> >      gtk_widget_show_all(priv->info_bar);
> >  }
> >  
> > @@ -203,12 +205,14 @@ static GObject
> > *spice_usb_device_widget_constructor(
> >      if (!priv->session)
> >          g_error("SpiceUsbDeviceWidget constructed without a
> > session");
> >  
> > +    gtk_orientable_set_orientation(GTK_ORIENTABLE(self),
> > GTK_ORIENTATION_VERTICAL);
> > +
> >      priv->label = gtk_label_new(NULL);
> >      str = g_strdup_printf("<b>%s</b>", _("Select USB devices to
> > redirect"));
> >      gtk_label_set_markup(GTK_LABEL (priv->label), str);
> >      g_free(str);
> >      gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
> > -    gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE,
> > 0);
> > +    gtk_container_add(GTK_CONTAINER(self), priv->label);
> >  
> >      priv->manager = spice_usb_device_manager_get(priv->session,
> > &err);
> >      if (err) {
> > @@ -348,10 +352,9 @@ GtkWidget
> > *spice_usb_device_widget_new(SpiceSession    *session,
> >                                         const
> > gchar     *device_format_string)
> >  {
> >      return g_object_new(SPICE_TYPE_USB_DEVICE_WIDGET,
> > -                        "orientation", GTK_ORIENTATION_VERTICAL,
> >                          "session", session,
> >                          "device-format-string",
> > device_format_string,
> > -                        "spacing", 6,
> > +                        "row-spacing", 6,
> 
> Likely I don't see any issue because of row-spacing here
> 
> Other than that, looks good
> Acked-by: Victor Toso <victortoso at redhat.com>
> 
> >                          NULL);
> >  }
> >  
> > @@ -572,7 +575,7 @@ static void
> > device_added_cb(SpiceUsbDeviceManager *manager,
> >      align = gtk_alignment_new(0, 0, 0, 0);
> >      gtk_alignment_set_padding(GTK_ALIGNMENT(align), 0, 0, 12, 0);
> >      gtk_container_add(GTK_CONTAINER(align), check);
> > -    gtk_box_pack_end(GTK_BOX(self), align, FALSE, FALSE, 0);
> > +    gtk_container_add(GTK_CONTAINER(self), align);
> >      spice_usb_device_widget_update_status(self);
> >      gtk_widget_show_all(align);
> >  }
> > -- 
> > 2.11.1
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list