- From: Owen Taylor <otaylor redhat com>
- To: Matthias Clasen <maclas gmx de>
- Cc: gtk-devel-list gnome org
- Subject: Re: GtkColorPickerButton
- Date: 17 Mar 2003 16:50:57 -0500
On Mon, 2003-03-17 at 15:38, Matthias Clasen wrote:
> Hmm, so it seems that this widget doesn't need any discussion...
> I have attached my latest version to
> The only change wrt to the proposal posted last week is a name change to
> GtkColorSelectionButton, in order to keep it more in line with
> GtkColorSelection and GtkColorSelectionDialog.
> I'd like to commit this soon, if no further comments arrive.
Just to raise the issue ... perhaps simply 'GtkColorButton' would
be better? I don't have any strong objection to GtkColorSelectionButton,
but it seems a little long.
A few things from studying the headers and scanning the
- it is still using GTK_CHECK_CAST, etc.
- It would be nice to switch it to allocating the private chunk
as instance-private-data, now that's in GLib. (Though would
require doing the version number bump for glib.) I don't
think you need to get rid of the ->priv member in the
instance... just make it point to the object data chunk. (*)
Not urgent though, since we'll need to do a pass over all
the current widgets anyways.
- I think making the size dependent on the font size as was
proposed earlier is a good idea... If it doesn't make it
to small it might be nice if the total height of the
button was the same as that of a button with a string in it.
Could be done after committing to CVS.
- The indentation in the C file seems to be a little uneven
(set_property/get_property in particular)
- The state_changed() handler probably also needs to check
REALIZED. To avoid the possibility of excessive renders
I think it would be better if style_set() and state_changed()
simply cleared priv->pixbuf unconditionally, and the render()
was only called by expose().
- It would be a good cleanup to never to store the color
as doubles internally, and always keep that as integers.
Prevents possible roundoff errors, removes a bunch of
- The keypress connection on the dialog is no longer necessary ..
GtkDialog handles that automatically.
(*) Though we do need to come up with a policy on this time/space
] [Thread Prev