Re: focus line theme patch

[Bill Haneman <bill haneman sun com>]

Owen Taylor wrote:
> 
> I've now applied much of this patch with some changes. Mostly,
> the changes consisted of figuring out how things are supposed
> to draw as opposed to how they actually draw now, and fitting
> wide focus lines into that.
> 
> Highlights of the changes from your patch:
> 
>  - Reordered where the state-type argument was
>    the argument list for gtk_paint_focus() to
>    be consistent with other gtk_paint_*() functions.
> 
>  - Changed the interpretation of the focus rectangle
>    passed to paint_focus() so that it is the bounding
>    box of the focus as drawn, not the rectangle passed
>    to gdk_draw_rectangle. (This gets rid of the
>    - 1 that was in every old usage of paint_focus()
>    and all sorts of focus_width / 2 factors now.)
> 
>  - Made the default focus-padding 1 pixel, not 0, and
>    used that it as the padding between the button and
>    the interior focus rectangle. Since exterior focus
>    is not the default, I think it's fine you have
>    to explicitely set the focus-padding to 0 to get
>    back the GTK+-1.2 look.
> 
>  - Created docs/widget_geometry.txt which documents the
>    exact painting behavior for a handful of widgets.
> 
>  - Did some work on making the default 1-1 stippled
>    focus rectangle match properly at the corners.
> 
>  - A fair bit of small style-fixes. Main thing was that
>    operators need spaces around them. (focus_width / 2,
>    not focus_width/2)
> 
> Things I didn't commit:
> 
>  - GtkCalendar changes. Didn't work properly with large
>    focus widths, I didn't understand the changes, and
>    the colors weren't really right. I think the focus
>    should always be drawn to contrast with the background
>    (base color) not with the selected

This of course depends on where focus is being drawn.  My patch worked
for the high-contrast-accessible theme, if you contrasted to background
it did not.  I wonder what problems you were having with large focus
widths, I tested up to 12 pixels wide.

>    I also don't think it makes sense to have a focus
>    rectangle drawn around the whole widget and also
>    around the focused day. However, this probably does
>    mean that we need to always focus one day so we
>    have some indication of focus. (Another possibility
>    would be to change the headers and selected day
>    color as we do in GtkTreeView ... in fact, this
>    is probably right to do in any case.)
> 
>    I did a fair bit of cleanup in gtkcalendar.c trying
>    to get this to work and committed those changes, but
>    left size requisition and focus drawing alone
>    in the end.
> 
>    Note that in the calendar and similar widgets, you
>    can get away with focus_width space between the day
>    widgets instead of 2 * focus_width.
> 
>    The GtkCalendar sizing code is pretty complex and
>    will take some study to figure out how to add focus
>    widths into it.

As I said, it was working for both exterior and interior focus with
large focus widths when I created it, I wonder what changed?

>  - GtkTreeViewColumn changes. Didn't make much sense
>    to me; needs to be discussed with Jonathan to come
>    up with the right way of drawing things and
>    reserving extra space.

I did actually ;-)
 
>  - GtkRange changes. Need formal specification in the
>    style of widget_geometry.txt for how this should
>    work ... the code in GtkRange currently is
>    "vestigal" and I don't think trying to change a bit
>    without figuring out what is supposed to be drawn,
>    for GtkScale and probably for GtkScrollbar too.
>    (GtkScrollbar isn't normally focusable, but can
>    be _made_ focusable by setting the CAN_FOCUS flag;
>    this might have legitimate uses.)

The focus draw was pretty broken as-is, it only created a visible line
on one side of a slider, etc. 

> Other notes:
> 
>  - I have GtkCheckButton putting focus-padding between
>    the focus rectangle and the child in interior-focus
>    mode, but this is probably wrong since in GtkButton
>    you only have a fixed spacing in this position
>    and focus-padding is outside. focus-padding should
>    probably be ignored for checkbuttons in interior-focus
>    mode.

Yes, probably.
 
>  - The GtkColorSel swatch need to be expanded for bigger
>    focus widths.
> 
>  - Bigger focus widths can cover over the entire ring
>    in GtkHSV; extra space needs to be allowed.

Both of these are corner cases I think, it means that the focus width is
50% of the ring width, which unless the rung width becomes configurable,
is not a usage that will be helpful to anyone, realistic focus widths
should never go above about 4 or 5 pixels - beyond that we have onscreen
magnifiers.

>  - For GtkColorsel and GtkHSV, a non solid focus-line-pattern
>    looks ugly; we need to either somehow use a different
>    default here or just focus-line-pattern.

I disagree, it's important that our focus indication be consistent, and
I think more important that having it always be 'pretty'.  Calum?
 
>  - GtkNotebook should probably have focus-padding spacing
>    between the tab borders and the focus indicator for
>    consistency with GtkButton.
> 
>  - GtkEntry in exterior-focus mode should probably have
>    focus-padding  spacing bewteen the bevel and focus
>    rectangle llike GtkButton, etc.

Yes.
 
> I don't really have time to work on these remaining issues -
> it might be good to file bug reports on them (or just one
> for all of them) so we don't lose track. Patches fixing
> up the remaining stuff would be much appreciated.
> 
> Regards,
>                                         Owen

Thanks for (partially) committing this patch.  I don't particularly look
forward to rejigging the uncommitted bits, but will send patches as soon
as possible.  GtkCalendar and GtkTreeview are important and I am a bit
puzzled as to why they did not work for you - but then a lot of change
has occurred since I made the patch.

Regards,

Bill