[sugar] [PATCH] add a secondary label and icon to palettes WAS Merging sugar-toolkit changes from tomeu repository

[Eben Eliason]

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