[Sugar-devel] [DESIGN] Re: [PATCH] fix OLPC #10488 - Color chooser does not always work when giving color to selected text

[Gonzalo Odiard]

Thanks. I will try changing the signal in the activities.

Gonzalo

On Mon, Dec 6, 2010 at 10:52 AM, Benjamin Berg <benzea at sugarlabs.org> wrote:

> Hello,
>
> On Mon, 2010-12-06 at 11:28 +0100, Simon Schampijer wrote:
> > We do only set the color when the palette is popped down. This does
> > break the following case:
> >
> > - select text you want to change the color of
> > - open the color chooser and change the color
> > - do click anywhere in the canvas (not the selected text) and don't wait
> > for the palette to popdown
> >
> > ---> The text is not selected anymore when the palette pops down, hence
> > the new color will not be applied.
> >
> > I think the reason why Benjamin decided to set the color when the
> > palette is closed is that when you would emit a color change on each
> > change in the selector using the sliders would end up in a lot of
> > signaling. That is why color selectors either have a fixed set of colors
> > the user can select from or the custom selector with sliders is a dialog
> > (ok, cancel) and therefore it is known when the user has made his
> decision.
>
> The signalling is actually happening in some way. It is possible for the
> application to listen to notify::color instead of color-set to do a live
> update of the preview.
>
> I have to admit that the issue with the text selection is rather
> unfortunate. A workaround for this would be nice to have and the
> solution may be to use notify::color instead.
>
> However, I do think that the real underlying issue is that a click to
> the canvas should *not* have any effect. Unfortunately implementing this
> is non-trivial, as it requires grabbing the input while still keeping
> the rest of the toolbar working (much like menu navigation in normal
> GTK+ applications).
>
>
> About the patch itself. It is wrong to remove the self.notify('color').
> If anything the self.emit('color-set') could be added in (some) places.
>
> Benjamin
>
> > On 12/02/2010 03:27 PM, Gonzalo Odiard wrote:
> > > diff --git a/src/sugar/graphics/colorbutton.py
> b/src/sugar/graphics/colorbutton.py
> > > index 1fed96d..c36320a 100644
> > > --- a/src/sugar/graphics/colorbutton.py
> > > +++ b/src/sugar/graphics/colorbutton.py
> > > @@ -125,7 +125,7 @@ class _ColorButton(gtk.Button):
> > >           self._preview.fill_color = get_svg_color_string(self._color)
> > >           if self._palette:
> > >               self._palette.props.color = self._color
> > > -        self.notify('color')
> > > +        self.emit('color-set')
> > >
> > >       def get_color(self):
> > >           return self._color
>
>
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20101206/bd8f1134/attachment.html>