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 629777 - make "margin" property more useful
make "margin" property more useful
Status: RESOLVED WONTFIX
Product: gtk+
Classification: Platform
Component: Widget: Other
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtk-bugs
gtk-bugs
: 651720 655522 (view as bug list)
Depends on:
Blocks: 651605
 
 
Reported: 2010-09-15 16:24 UTC by Christian Persch
Modified: 2016-03-06 02:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make "margin" property a boxed GtkBorder (8.16 KB, patch)
2010-09-15 16:24 UTC, Christian Persch
none Details | Review

Description Christian Persch 2010-09-15 16:24:04 UTC
IMHO it would be much more useful to have a property that one can use to get/set all margins at once, via a boxed GtkBorder, instead of making it set all to the same value, and get the weird maximum of the margins... Attached patch does that, and also adds public accessors for this.
Comment 1 Christian Persch 2010-09-15 16:24:22 UTC
Created attachment 170352 [details] [review]
Make "margin" property a boxed GtkBorder

Bug #629777.
Comment 2 Havoc Pennington 2010-09-16 00:59:33 UTC
Review of attachment 170352 [details] [review]:

::: gtk/gtkwidget.c
@@ +11649,3 @@
+  g_return_if_fail (GTK_IS_WIDGET (widget));
+
+  aux_info = _gtk_widget_get_aux_info (widget, TRUE);

this needs to use get_aux_info_or_default or whatever to avoid creating the aux info if it wasn't set already

@@ +11678,3 @@
+  if (margin == NULL)
+    margin = &null_margin;
+  if (memcmp (margin, &aux_info->margin, sizeof (GtkBorder)) == 0)

sizeof(margin) makes one less assumption

::: gtk/gtkwidget.h
@@ +763,3 @@
+                                       GtkBorder *margin);
+void     gtk_widget_set_margin        (GtkWidget *widget,
+                                       const GtkBorder *margin);

should probably egtk-format-protos these to align with the other margin funcs

::: tests/testadjustsize.c
@@ +377,3 @@
     "margin-right",
     "margin-top",
+    "margin-bottom"

would be nice to test the new property
Comment 3 Havoc Pennington 2010-09-16 01:02:02 UTC
Summarizing IRC, some of what you can do with this patch would be:

GtkBorder margin = { 1,2,3,4 };
widget = g_object_new(GTK_TYPE_BUTTON, "margin", &margin, NULL);

or

/* pre-existing GtkBorder kept around */
widget = g_object_new(GTK_TYPE_BUTTON, "margin", &settings->margin, NULL);

If you had dynamic border instead of a struct initializer,

GtkBorder margin;
margin.left = foo();
margin.right = bar + baz;
margin.top = 27;
margin.bottom = 42 + compute_stuff();
widget = g_object_new(GTK_TYPE_BUTTON, "margin", &margin, NULL);

In JavaScript,

margin = new Gtk.Border();
margin.left = 1;
margin.right = 2;
margin.top = 3;
margin.bottom = 4;
widget = new Gtk.Button({ margin: margin });

Or with some work we could support some of these (maybe we do already, not sure):

widget = new Gtk.Button({ margin: [ 1, 2, 3, 4 ] });
or
widget = new Gtk.Button({ margin: { left: 1, right: 2, top: 3, bottom: 4 })
or
widget = new Gtk.Button({ margin: 6 }); // set all sides using gvaluetransform

(Not sure if there's any precedent in GTK already for a transform like integer to GtkBorder, but maybe it would be a nice thing to include and dynamic languages could do it automatically.)


The current way is the "CSS way" and it looks like:

widget = g_object_new(GTK_TYPE_BUTTON, "margin", 6, NULL);

where if you want to do all four you have to use the specific property names:

widget = g_object_new(GTK_TYPE_BUTTON,
 "margin-left", 1,
 "margin-right", 2,
 "margin-top", 3,
 "margin-bottom", 4,
 NULL);

The getter on "margin" as just an integer is useless really (could just make the property write-only).

I don't know. I guess it comes down to how common is it to have a GtkBorder handy vs. how common to want to set the same size on all 4 sides.
Comment 4 Matthias Clasen 2010-09-17 14:39:46 UTC
It would certainly be cool if gobject would let you do things like

g_object_set (foo, "margin", &border, NULL);
g_object_set (foo, "margin.left", 4, NULL);
g_object_set (foo, "margin", 3, NULL);

(with the last one transforming 3 -> { 3, 3, 3, 3 } )

but maybe it is better to leave this kind of fancy convenience to JavaScript and other bindings.
Comment 5 Matthias Clasen 2011-06-03 13:25:32 UTC
*** Bug 651720 has been marked as a duplicate of this bug. ***
Comment 6 Javier Jardón (IRC: jjardon) 2011-12-30 14:03:39 UTC
*** Bug 655522 has been marked as a duplicate of this bug. ***