GNOME Bugzilla – Bug 541009
Get rid of separate subclasses for horizontal and vertical orientation
Last modified: 2011-06-08 01:45:54 UTC
Having GtkHBox, GtkVBox, or GtkHScale and GtkVScale etc. is evil in some ways: - it makes subclassing a pain (always need two subclasses) - it makes GUIs which change orientation very hard since widgets need to be recreated and reparented (see bug #442042) - it duplicates almost-identical code which is bad for maintainability - it's probably evil in other ways ;)
Created attachment 113741 [details] [review] Add an "orientation" property to GtkBox Patch which reduces GtkVBox and GtkHBox to mere types without any implementation. Example code in testgtk->flipping included. The functions become a bit longer but on the other hand it feels much more maintainable if everything is in one place.
(In reply to comment #1) > Created an attachment (id=113741) [edit] > Add an "orientation" property to GtkBox > > Patch which reduces GtkVBox and GtkHBox to mere types without any > implementation. Example code in testgtk->flipping included. > > The functions become a bit longer but on the other hand it feels > much more maintainable if everything is in one place. I like the idea very much. > static void > gtk_vbox_init (GtkVBox *vbox) > { > + g_object_set (vbox, "orientation", GTK_ORIENTATION_VERTICAL, NULL); > } Does this suffice for Glade to honor the right default orientation?
> static void > gtk_vbox_init (GtkVBox *vbox) > { > + g_object_set (vbox, "orientation", GTK_ORIENTATION_VERTICAL, NULL); > } Wouldn't it be more elegant to just g_object_new (GTK_TYPE_VBOX, "orientation", GTK_ORIENTATION_VERTICAL, NULL); ?
No, a GtkHVox needs to have a vertical orientation no matter if created by gt_vbox_new(), or by g_object_new (GTK_TYPE_VBOX, ...); (The default value in GtkBox is HORIZONTAL, so you would end up with a horitontal vbox)
Maybe my comment was too terse. Of course, I meant to do return g_object_new (GTK_TYPE_VBOX, "orientation", GTK_ORIENTATION_VERTICAL, NULL); in gtk_vbox_new instead of the current return g_object_new (GTK_TYPE_VBOX, NULL);
Yes I understand. What I meant is that the property needs to be set in init() so it is *always* set, even if user code says vbox = g_object_new (GTK_TYPE_VBOX, NULL);
ah, good point.
One way to overcome that would be to change the default value of orientation in GtkVBox and GtkHBox, but gobject makes that hard.
Marking patch here as obsolete because I will file individual bugs for each set of widgets (box, ruler, separator etc). to make review and tracking easier. Will make this one depend on all of them.
All these new "orientation" APIs added in the bug linked from here almost cry for an interface. If we had a GtkFlippable interface I could remove all these new setters and getters again, and apps could easily recursively flip their UI by recursively flipping their children if GTK_IS_FLIPPABLE(child) is TRUE. Perhaps flippable is the wrong word, but it sounds better than GtkRotatable (does it?). Opinions please :)
Created attachment 119591 [details] [review] Patch implementing the GtkOrientable interface
The GtkOrientable implementation looks fine to me (though I'll never internalize the rationale for using base_init vs. class_init).
Me neither ;) I'll ask the godfather of gobject when he is back. Committed this version in the meantime: 2008-09-30 Michael Natterer <mitch@imendio.com> Bug 541009 – Get rid of separate subclasses for horizontal and vertical orientation: * gtk/Makefile.am * gtk/gtk.symbols * gtk/gtk.h * gtk/gtkorientable.[ch]: add new interface GtkOrientable which will be implemented by everything that can switch orientation.
Since GtkToolbar is now orientable and _get_orientation and _set_orientation are deprecated, please don't forget to apply the same change to GtkToolShell.
There's a number of things that need to happen, and adding the actual deprecation is the last of them: - Ensure the new api actually fully covers all use cases of the old (easy here) - Make the gtk tree free of the old api - Write good docs (migration guide), and cross-references from old to new api - Add the actual deprecation
- Ensure the new api actually fully covers all use cases of the old (easy here) For this, we may be able to use Benjamins new reftests: write two ui files, one for the old ui, and one for the new, and compare
(In reply to comment #15) > There's a number of things that need to happen, and adding the actual > deprecation is the last of them: > > - Ensure the new api actually fully covers all use cases of the old (easy here) Done (we are not covering the GtkHBox/GtkVBox case in this bug) > - Make the gtk tree free of the old api Done > - Write good docs (migration guide), and cross-references from old to new api > - Add the actual deprecation I'm working on this
I've gone ahead and did most of this work now. Better documentation would still be useful.