[sugar] [PATCH] add a secondary label and icon to palettes WAS Merging sugar-toolkit changes from tomeu repository
Eben Eliason
eben.eliason at gmail.com
Tue Apr 1 10:21:36 EDT 2008
On Tue, Apr 1, 2008 at 9:22 AM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
> Hi, follow some nitpicks, Marco can hopefully give some insights in
> reducing the containers used for the layout, looks quite complex right
> now.
>
> __gproperties__ = {
> - 'invoker' : (object, None, None,
> - gobject.PARAM_READWRITE)
> + 'invoker' : (object, None, None,
> + gobject.PARAM_READWRITE),
> + 'primary-text' : (str, None, None, None,
> + gobject.PARAM_READWRITE),
> + 'secondary-text' : (str, None, None, None,
> + gobject.PARAM_READWRITE),
> + 'icon' : (object, None, None,
> + gobject.PARAM_READWRITE),
> + 'icon-visible' : (bool, None, None, True,
> + gobject.PARAM_READWRITE),
> + 'group-id' : (str, None, None, None,
> + gobject.PARAM_READWRITE),
> +
> + 'menu-after-content' : (bool, None, None, False,
> + gobject.PARAM_CONSTRUCT_ONLY)
> }
>
> Any reason for that empty space?
Not really -- only that it separates the READWRITEs from the
CONSTRUCT_ONLY, and when it was added (last), I didn't want to redo
the left margin on all of the other properties.
> __gsignals__ = {
> @@ -143,61 +156,105 @@ class Palette(gtk.Window):
> }
>
> + palette_box = gtk.VBox()
>
> - self._secondary_anim = animator.Animator(1.0, 10)
> - self._secondary_anim.add(_SecondaryAnimation(self))
> + primary_box = gtk.HBox()
> + palette_box.pack_start(primary_box, expand=False)
> + primary_box.show()
>
> - self._popdown_anim = animator.Animator(0.6, 10)
> - self._popdown_anim.add(_PopdownAnimation(self))
> + self._icon_box = gtk.HBox()
> + self._icon_box.set_size_request(style.zoom(style.GRID_CELL_SIZE), -1)
> + primary_box.pack_start(self._icon_box, expand=False)
>
> - vbox = gtk.VBox()
> + labels_box = gtk.VBox()
> + self._label_alignment = gtk.Alignment(xalign=0, yalign=0.5,
> + xscale=0, yscale=0.33)
> + self._label_alignment.set_padding(0, 0, style.DEFAULT_SPACING,
> + style.DEFAULT_SPACING)
> + self._label_alignment.add(labels_box)
> + self._label_alignment.show()
> + primary_box.pack_start(self._label_alignment, expand=True)
> + labels_box.show()
>
> I think the most common thing is to do:
>
> container = SomeContainer()
> container.props.foo = bar
> grand_father.add(container)
> container.show() # if appropriate
>
> child = SomeWidget()
> child.props.foo = bar
> container.add(child)
> child.show() # if appropriate
>
> etc.
>
> Makes sense?
Hmm, I guess our minds differ in how we read this logically. I follow
the same steps as you propose, but I mentally split them between "pre"
and "post" commands (the first two lines, and the second two lines),
and then construct all of a containers children in the middle. That
is, I "push" after child.props.foo = bar, set up all subchildren, and
"pop" with container.add(child) and child.show() where needed.
>
> self._label = gtk.AccelLabel('')
> - self._label.set_size_request(-1, style.zoom(style.GRID_CELL_SIZE) -
> - 2 * self.get_border_width())
> self._label.set_alignment(0, 0.5)
> - self._label.set_padding(style.DEFAULT_SPACING, 0)
>
> if text_maxlen > 0:
> self._label.set_max_width_chars(text_maxlen)
> self._label.set_ellipsize(pango.ELLIPSIZE_MIDDLE)
>
> - vbox.pack_start(self._label)
> + labels_box.pack_start(self._label, expand=True)
> +
> + self._secondary_label = gtk.Label()
> + self._secondary_label.set_alignment(0, 0.5)
> +
> + if text_maxlen > 0:
> + self._secondary_label.set_max_width_chars(text_maxlen)
> + self._secondary_label.set_ellipsize(pango.ELLIPSIZE_MIDDLE)
> +
> + labels_box.pack_start(self._secondary_label, expand=True)
> + #self._secondary_label.show()
>
> self._secondary_box = gtk.VBox()
> - vbox.pack_start(self._secondary_box)
> + palette_box.pack_start(self._secondary_box)
>
> self._separator = gtk.HSeparator()
> self._secondary_box.pack_start(self._separator)
>
> self._menu_content_separator = gtk.HSeparator()
>
> - if menu_after_content:
> + self._popup_anim = animator.Animator(0.3, 10)
> + self._popup_anim.add(_PopupAnimation(self))
> +
> + self._secondary_anim = animator.Animator(1.0, 10)
> + self._secondary_anim.add(_SecondaryAnimation(self))
> +
> + self._popdown_anim = animator.Animator(0.6, 10)
> + self._popdown_anim.add(_PopdownAnimation(self))
> +
>
>
> - self.set_primary_text(label)
> - self.set_group_id('default')
> + #self.set_primary_text(self._primary_text)
> + #self.set_group_id('default')
>
> Can we remove this?
Yup, that can go. Sorry.
> So, apart from this style nitpicks, my only two remaining concerns are
> the complexity of using so many containers and passing an Icon as a
> property.
I don't think the container situation is really very complex any more.
The only container that could possibly be extraneous, I believe, is
_icon_box, which is used so that all necessary formatting can be done
without needing to have an icon widget in hand (which may change).
> What worries me about this last one is that if the user of this API
> sets its own widget or icon, Palette may need to change some
> properties so it fits correctly on the palette. So the user could be
> setting some properties and later these are overridden by Palette.
> This could be disconcerting to the user unless we document which
> properties are set by Palette. But that would be a complex API and we
> probably will need to set other properties at future revisions, so
> some earlier activities may look wrong, etc.
I think that size would be the only one we care about. As long as it
doesn't break the layout, we shouldn't care.
> So, what I propose is having the properties 'icon-name',
> 'icon-filename' and 'icon-xo-colors'. For more advanced uses, we could
> have a method create_icon() that could be overriden for palettes that
> wish to set its own Widget in place of the normal Icon.
This could work, too.
- Eben
More information about the Sugar
mailing list