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

[Benjamin Berg]

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