[sugar] Merging sugar-toolkit changes from tomeu repository

[Eben Eliason]

Hmm, Firefox didn't upload my file...trying again.

On Mon, Mar 31, 2008 at 3:46 PM, Eben Eliason <eben.eliason at gmail.com> wrote:
> Here is my attempt at consolidating the changes to palette.py.  The
>  layout is tested with respect to primary text, secondary text, and
>  icons, but I have not visually tested the accel labels.  All of the
>
> other changes applicable have been merged.
>
>  - Eben
>
>  On Mon, Mar 31, 2008 at 1:48 PM, Marco Pesenti Gritti
>
>
> <mpgritti at gmail.com> wrote:
>  > Looks good. Now that the logic is much simpler I don't think the
>  >  helper is worth...
>  >
>  >  Marco
>  >
>  >
>  >
>  >  On Mon, Mar 31, 2008 at 7:18 PM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
>  >  > On Fri, Mar 28, 2008 at 10:55 PM, Marco Pesenti Gritti
>  >  >
>  >  >
>  >  > <mpgritti at gmail.com> wrote:
>  >  >  > On Fri, Mar 28, 2008 at 9:52 PM, Marco Pesenti Gritti
>  >  >  >  <mpgritti at gmail.com> wrote:
>  >  >  >  > On Fri, Mar 28, 2008 at 8:13 PM, Tomeu Vizoso <tomeu at tomeuvizoso.net> wrote:
>  >  >  >
>  >  >  > >  >  >  +    def set_tooltip(self, tooltip):
>  >  >  >  >  >  >  +        if self._tooltip != tooltip:
>  >  >  >  >  >  >  +            self._tooltip = tooltip
>  >  >  >  >  >  >  +            if tooltip and self.palette is None:
>  >  >  >  >  >  >  +                self.palette = Palette(tooltip)
>  >  >  >  >  >  >  +            elif self.palette:
>  >  >  >  >  >  >  +                self.palette.set_primary_text(tooltip)
>  >  >  >  >  >  >
>  >  >  >  >  >  >  Is the if at the top really necessary? I can't think of use cases
>  >  >  >  >  >  >  where we would be hitting it.
>  >  >  >  >  >  >  Also I'd replace the palette if you set_tooltip and a Palette already exist.
>  >  >  >  >  >
>  >  >  >  >  >  How so?
>  >  >  >  >
>  >  >  >  >  Are you trying to optimize the change tooltip use case there?
>  >  >  >  >
>  >  >  >  >  What about this logic:
>  >  >  >  >
>  >  >  >  >  * palette != None and tooltip == None -> replace the palette
>  >  >  >  >  * palette != None and tooltip != None -> replace primary text
>  >  >  >  >
>  >  >  >  >  (It would require to set tooltip to None on set_palette, but that make sense)
>  >  >  >  >
>  >  >  >  >  The semantic is going to be a bit unclear there in any case and we
>  >  >  >  >  should document it. But if I set_palette(fancy_dialog_like_palette)
>  >  >  >  >  and then set_tooltip, I'd expect a tooltip to appear, not the primary
>  >  >  >  >  text of my fancy palette to be changed.
>  >  >  >
>  >  >  >  Btw it seem like it would be fairly easy to factor out this logic to
>  >  >  >  toolbutton.setup_tooltip()
>  >  >
>  >  >  Looks good?
>  >  >
>  >  >
>  >  >     def set_tooltip(self, tooltip):
>  >  >         """ Set a simple palette with just a single label.
>  >  >         """
>  >  >         if self.palette is None or self._tooltip is None:
>  >  >             self.palette = Palette(tooltip)
>  >  >         elif self.palette is not None:
>  >  >
>  >  >             self.palette.set_primary_text(tooltip)
>  >  >
>  >  >         self._tooltip = tooltip
>  >  >
>  >  >         # Set label, shows up when toolbar overflows
>  >  >         gtk.ToolButton.set_label(self, tooltip)
>  >  >
>  >  >  Do you think it is worth to put into a toolbutton.setup_tooltip() func?
>  >  >
>  >  >  Thanks,
>  >  >
>  >  >  Tomeu
>  >  >
>  >  _______________________________________________
>  >  Sugar mailing list
>  >  Sugar at lists.laptop.org
>  >  http://lists.laptop.org/listinfo/sugar
>  >
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: palette_merge.patch
Type: text/x-patch
Size: 11433 bytes
Desc: not available
Url : http://lists.laptop.org/pipermail/sugar/attachments/20080331/b3d93639/attachment-0001.bin