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 <[EMAIL PROTECTED]> wrote: > On Fri, Mar 28, 2008 at 10:55 PM, Marco Pesenti Gritti > > > <[EMAIL PROTECTED]> wrote: > > On Fri, Mar 28, 2008 at 9:52 PM, Marco Pesenti Gritti > > <[EMAIL PROTECTED]> wrote: > > > On Fri, Mar 28, 2008 at 8:13 PM, Tomeu Vizoso <[EMAIL PROTECTED]> 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 [email protected] http://lists.laptop.org/listinfo/sugar
