Spacer doesn't render background color

[cceckman]

I've got a panel in my app that I'd like to draw with reversed colors. The way I've come up with to handle this is to embed the tui.Box in a custom struct, and override Draw with a custom function:

func (c *custom) Draw(p *tui.Painter) {
  p.WithStyle("reversed", c.Box.Draw)
}

But, this doesn't work if the Box contains Spacer widgets - which still render using the default background. I'd expect them to be filled with the current background color, if I'm understanding the intended semantics of Draw and WithStyle correctly. (Is that the intended behavior?)

Full example gist; I'm expecting a line across the whole thing.

It looks like Spacer inherits the default WidgetBase Draw function, which is blank. I'm not sure if the default Draw should fill its background, or whether this should just apply to Spacer; I see #45 has a maybe-related commentary on Box.

(All that said, tui is great to work with; looking forward to its future. :-) )

[marcusolsson]

Thanks for trying out tui!

Unfortunately, theming support is a bit limited right now. Basically the way to theme widgets right now is through the selectors that currently only are available on a few widgets. While we could introduce a selector for spacers, I pretty sure we don't want to specifically theme Spacers (they're just empty space). By the way, this is what needs to be done for the issue you mentioned (#45).

For spacers though, I think they should be transparent and just inherit the background color from the containing box. This however, implies a style hierarchy, which is also needed for #36 and is something I've been meaning to take a closer look at.

Right now, I don't see a quick fix to this but if you're interesting in pursuing this, I'd gladly help!

[cceckman]

"inherit the background color from the containing box" -> sounds good to me; that's effectively what I was trying to do in the linked gist, applying a Style to the Box and intending to apply to its contents. (I was assuming that WithStyle would effectively push a style onto the draw stack, but it looks like RestoreStyle just resets to "normal" rather than "restores".)

I guess I'm not quite following what the design for Styles is; is there a design doc / etc? (I would be interested in helping with / working on one. :-) ) The idea of CSS-style selectors is interesting, but I'm somewhat wary of it for reasons that are better articulated in a doc than a comment. :-)

[marcusolsson]

That's understandable, it's not very clear at the moment :) My initial take on theming was inspired by color schemes in vim, which (as far as I understand it) accomplishes this with globally defined identifiers like Normal, Visual or StatusLine. Though I'm thinking it might be a bit too naïve for a library like tui.

It probably won't be much, but I'll try and write down my thoughts on theming in the project wiki. I'd love to get your input/help on working on defining it! I've mostly been focusing on the more basic features, but improved color support seems to be highly requested so I'm very happy to get your thoughts on it!

[cceckman]

Great, sounds good! I've written up some thoughts but will hold off on sharing them till the Wiki page is up, to see exactly how off-base they are.

[marcusolsson]

I've tried to document the parts which I'm fairly confident about but feel free to question them and to add to it!

Documentation

[cceckman]

Thanks for the writeup!

I did a bit of brainstorming in a Google doc, but not sure how best to pull that back into the wiki format / make it a discussion.

In general, I'm -1 on having a domain-specific language for selectors (CSS-like) ("but that's just, like, my opinion, man.") IMO a cleaner way to provide dynamic styling would be to use embedding- allowing users that want to customize which styles are applied at what time to embed a Widget and override the style selection. I've started experimenting with that in a branch.

More to the point of this report, though - w/rt "style hierarchy" - I think one tweak to improve would be to make WithStyle properly stack styles, rather than reset: see demo with screencast. I'll try to clean up what I think is the fix and put a PR together.

Any thoughts on how to test style application?

[marcusolsson]

I'm intrigued to hear more and to take this further! As I indicated earlier, theming hasn't received my fullest attention and it does seem like you've given this more thought than I have. I will study this more closely and get back to you on this as soon as possible!

[marcusolsson]

@cceckman What do you think about creating another issue "Proposal: Theming" and add your thoughts from the Google Docs document there, and maybe we could continue the discussion there? Don't worry about formalizing it, just a copy-paste will do just fine! I'm digging the StylePolicy idea!

[cceckman]

Sure thing. I copied the existing doc, and added some of my reservations about the proposal + alternatives.

[cceckman]

On this bug in particular: I have a demo of the issue in this branch, and a fix in this branch.

Per my earlier comment: the fix makes WithStyle behave as a style stack rather than as a setter-with-automatic-reset. That's the way I'd like it to be, so... :-)

I'm holding off on a pull request because it has the same copyright stuff as #67; if you'd rather see the PR and just have it sit until the resolution (same for both of those), I'm happy to go ahead and file it.

[cceckman]

#89 effectively closed this.