Re: [bugs] [PATCH] Scrollbar and right window border

On Tue, Dec 12, 2017 at 08:33:13AM +0100, Lukas Fleischer wrote:
> On Fri, 08 Dec 2017 at 21:09:50, Lars Henriksen wrote:
> > When a scrollbar is on display in APP or TOD windows, the right
> > vertical border (outside the scrollbar) is not highlighted
> > when the window is selected.
> > 
> > The scrollbar itself is always highlighted:
> > - when APP or TOD is deselected
> > - in configuration windows where borders otherwise are not
> 
> Thanks! I like the idea behind this patch.
> 
> What I do not like is that the scroll bar of a non-highlighted panel
> (which is black by default) now gets too much focus. Maybe we can
> compensate for this by using bold mode or a different character when the
> window is not active? Anyways, this should be done in a follow-up patch.

Yes, I thought so myself, as I tried it out, but you get used to it!
I believe it is a question of habits. But we could leave the scroll bar
as it was before this patch, i.e. always highlighted whatever the
situation.

> > The patch moves the scrollbar parameters from arguments of
> > draw_scrollbar() to the function itself.
> > 
> > The highlight argument to draw_scrollbar() was always 1.
> > Instead call circumstances are figured out and highlight set
> > accordingly.
> 
> If possible, I would like to keep the parameter and fix it in the caller
> instead. The reason is that draw_scrollbar() should be part of a generic
> framework to display scroll windows (easily reusable for additional
> panels later).

You are right. And there is another thing that I don't like, now that I look
again: draw_scrollbar() should of course be responsible for redrawing the
border correctly. So the condition should not be whether someone else
has done it, but whether it shold be highlighted or not when redrawn,
So the highlight argument is for the border, not for the scrollbar itself.

> ... There should not be any logic specifically for the "main"
> calcurse panels.
> 
> Do you think such a more generic approach would be feasible?

Yes certainly, if you mean setting highlight in the caller and not in
draw_scrollbar(), but I don't see how the "main panel logic" can be avoided when
figuring out how to call it. That puts the logic in wins_scrollwin_display().

Using the label as identifier is in princible wrong since it's not unique,
but I couldn't think of anything else without adding to the scrollwin
structure. It's mostly a theoretical problem.

Lars Henriksen

Links