Re: #54720 -button children should optionally move

[Soeren Sandmann <sandmann daimi au dk>]

Owen Taylor <otaylor redhat com> writes:

> I took a look at this patch and ended up rewriting it quite a bit.
> 
>  * I simplified a lot of the logic in GtkButton and GtkToggleButton to centralize
>    all the state maintanence in one place for each widget.
> 
>  * I removed the movement for the child of check/radio buttons with 
>    draw_indicator == TRUE. This just looked very weird to me, and windows
>    does not do this.

My intention was that themes normally should set child_displacements
for check/radio buttons to 0.  I can imagine (pretty weird,
admittedly) themes that want to draw a radiobutton like a
togglebutton. It is not something I feel strongly about, though.

>  * Instead of adding a ->depressed() virtual function, I added a function
>    _gtk_button_set_depressed() that sets a depressed flag. This resulted
>    in more consistent code since the state of the widget was maintained via
>    gtk_widget_set_state() rather than a callback.

Sounds reasonable.

> Remaining possible questions about this patch:
> 
>  * Should we have separate child_displacement_x/child_displacement_y 
>    properties? Does it ever make sense for them to be separate

If a theme wants to make the light come in from left, it might also
want to make the children also move just to the right, not down or up.

>  * Should we increase the amount of space requested for the button
>    based on the amount of displacement? Currently, things don't
>    work very will if the displacement is more than 1.

I think I tried that, but decided it didn't look good if the extra
space was put entirely below the label.  If extra space should be
requested, it should be evenly distributed above and below the label.

>  * Should _gtk_button_set_depressed() be exported? If someone wanted
>    to write a like-toggle-button widget right now, they wouldn't 
>    be able to. On the other hand, making it public is inviting
>    confusion and abuse.

I think it should be exported.  The comment is pretty clear about when
it should be used and not used.

> Index: gtkbutton.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkbutton.c,v

I don't have any comments on the patch itself.