- From: Matthias Clasen <maclas gmx de>
- To: gtk-devel-list gnome org
- Subject: Re: GtkColorPickerButton
- Date: 18 Mar 2003 02:23:03 +0100
On Mon, 2003-03-17 at 22:50, Owen Taylor wrote:
> 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
> > http://bugzilla.gnome.org/show_bug.cgi?id=107597
> > 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
> C file:
> - 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
> code, etc.
> - The keypress connection on the dialog is no longer necessary ..
> GtkDialog handles that automatically.
Thanks for the feedback. I've done all except for the private chunk; see
the latest patch in bugzilla.
] [Thread Prev