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 737661 - allow automatic property binding for template children
allow automatic property binding for template children
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
1.42.x
Other Linux
: Normal enhancement
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2014-09-30 14:05 UTC by Emmanuele Bassi (:ebassi)
Modified: 2014-10-31 18:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: Add test for automatic binding of template children (2.27 KB, patch)
2014-10-30 12:22 UTC, Bastien Nocera
committed Details | Review
Gtk: automatically bind declared template children (2.35 KB, patch)
2014-10-30 21:20 UTC, Giovanni Campagna
committed Details | Review

Description Emmanuele Bassi (:ebassi) 2014-09-30 14:05:01 UTC
a nice property of the GtkWidget template API is the ability to automatically bind a template child to a field in the instance (or instance private) data structure. this allows avoiding to call get_template_child() inside the instance initialization function.

now that GJS supports automatic templates for GtkWidget subclasses, it should be possible to automatically add properties to the `this` instance and bind them to the template children.

the default behaviour in GTK+ is to use the same field name as the child name, e.g.:

  typedef struct { GtkWidget *child_name; } FooWidgetPrivate;

  gtk_widget_class_bind_template_child_private (widget_class,
                                                FooWidget,
                                                child_name);

we could do the same by using a sanitized child name, e.g. replacing all '-' with '_':

  Children: [ 'child-name', ],

would add a `this.child_name` property to the instance.

alternatively, we could use an associative array for Children and InternalChildren, e.g.:

  Children: { 'child-name': '_childName', },

that would add a `this._childName` property to the instance.

my personal preference is for the first option; it's easy enough to start naming widgets in Glade using language-specific coding style rules. we could also conceivably check if `Children` and `InternalChildren` are objects or pure arrays, and allow both behaviours, but the code would probably suffer a bit.
Comment 1 Giovanni Campagna 2014-09-30 15:58:42 UTC
I agree on the first option (replacing '-' with '_' and treating the name as the property name), maybe with an added leading underscore for internal children (because they are supposed to be private).
I don't think it is worth the complication of specifying the name twice to get proper coding style.
Comment 2 Emmanuele Bassi (:ebassi) 2014-09-30 16:02:36 UTC
(In reply to comment #1)
> I agree on the first option (replacing '-' with '_' and treating the name as
> the property name), maybe with an added leading underscore for internal
> children (because they are supposed to be private).

good point on the internal children, and yes: I agree.
Comment 3 Bastien Nocera 2014-10-30 12:21:03 UTC
I don't see where we can implement this in the GTK+ overrides.

At the end of the GtkWidgetClass _init() function, we don't have access to an instance of the object, just the class, so we can't run get_template_child() on it.

Inside params._instance_init(), I can't figure out how to get access to the "class this".
Comment 4 Bastien Nocera 2014-10-30 12:22:16 UTC
Created attachment 289648 [details] [review]
tests: Add test for automatic binding of template children
Comment 5 Giovanni Campagna 2014-10-30 21:20:41 UTC
Created attachment 289696 [details] [review]
Gtk: automatically bind declared template children

Wrap Gtk.Widget.prototype._init() so that custom subclasses of
Gtk.Widget that are using the automatic template system will
also get automatic children properties added.

Note that like the rest of the widget template system, this
only supports a flat JS inheritance, ie, you cannot inherit
from a JS class that has a GtkWidget template - only the
most derived JS class can define a template.
Comment 6 Bastien Nocera 2014-10-30 22:01:03 UTC
Review of attachment 289696 [details] [review]:

Looks good otherwise. Feel free to merge in the test I made into your patch and commit that.

Thanks!

::: modules/overrides/Gtk.js
@@ +37,2 @@
         let children = params.Children;
+        delete params.Children;

Are those typos that should be fixed separately?
Comment 7 Giovanni Campagna 2014-10-31 18:33:13 UTC
Pushed after splitting out the typo fix.

Attachment 289696 [details] pushed as 40ff788 - Gtk: automatically bind declared template children