options that set foreground / background color don't seem to be propagated to tcell

[tcurdt]

options that set foreground / background color don't seem to be propagated to tcell

[mum4k]

I did not dig too deep, but after a short experimentation, this looks like a bug. We either aren't correctly propagating the default options back to tcell, or termdash is overwriting them somewhere later in the execution.

I am marking this as a bug and this currently needs investigation to see where the problem is. @tcurdt are you interested in / do you have the time to investigate this?

[tcurdt]

I'll see if I can find the time. But it's good to know that you consider it a bug and are willing to accept a PR.

[mum4k]

Thank you for reporting this @tcurdt. Sounds good, I will also keep an eye on this in case I find some time.

[tcurdt]

OK, since I soon need this to work I started poking a bit.

It seems like clear sets the background color correctly - but on the draw there is no mention of a background color.

When I set the bg color explicitly on the linechart there is some improvement

	linechart.AxesCellOpts(cell.FgColor(cell.ColorRed), cell.BgColor(cell.ColorBlack)),
	linechart.YLabelCellOpts(cell.FgColor(cell.ColorGreen), cell.BgColor(cell.ColorBlack)),
	linechart.XLabelCellOpts(cell.FgColor(cell.ColorCyan), cell.BgColor(cell.ColorBlack)),

but I don't find where to explicitly set the background color of the plotted value.
And it still does feel like a workaround.

As for the border on the container - I don't even see the bg color being inherited

[mum4k]

Thank you for looking into this. I think you are right, the container simply doesn't forward nor does it inherit the background color cell attribute. Looks like I completely forgot about it when writing that portion. Forwarding it through the layer is the first thing we should do, however that will only fix the problem for the cells that are drawn by the container and not overwritten by the widget.

The widgets in the container will also write some of the cells in the container, and for those we need to ensure that the widgets themselves support setting background color. Or we could think of inheriting the default from the container, but that would be a larger infrastructure change.

[tcurdt]

I would have assumed every container and widget has in some way access to the terminal (and with that to the colors)

https://github.com/mum4k/termdash/blob/master/terminal/tcell/cell_options.go#L60

If so the most low hanging fruit would be to use the background color of the terminal when drawing the widgets (and bypassing the inheritance).

But unfortunately I am still struggling with the internal abstraction layers of the project to see how things go together.

Looking at the drawing code

https://github.com/mum4k/termdash/blob/master/widgets/button/button.go#L177

I don't see how to get access to the terminal.

This seems like a lot of work to fix this retroactively.

[tcurdt]

I am OK to do the leg work, but if you have some guidance here that would be much appreciated. Feeling a little stuck.

[mum4k]

@tcurdt I am happy to help and discuss. Only the container does have direct access to the terminal, but neither the container nor the widgets draw on the terminal directly. There is an abstraction called Canvas which then gets applied to the terminal.

All drawing even at the container level happens on the Canvas. The only drawing on the terminal is by applying the canvas to the terminal. This abstraction exists, so that widgets don't have to worry about offsets. Regardless of where the widget is on the terminal, it always gets a canvas that starts at coordinates (0,0). The infrastructure takes care of positioning and limits.

This however does mean that we will have to plumb the background options through multiple layers if that was omitted. However I suspect the plumbing is done and the problem simply is that the container and the widgets don't set background attributes. Regarding the layers - the canvas stores what was drawn on it in a cell buffer which is essentially an array of cells. Looks like the cell options already have background attributes.

I would start by looking at the function that applies the canvas onto the terminal and confirm that all the attributes are correctly applied.

The above covers the verification we need to do to ensure the infrastructure allows widgets and container to set background attributes. But the widgets themselves still need to correctly set that attribute on their canvas (actually apply the option). The story is similar about container elements like the border. The container also has a canvas and we need to modify the code where it draws onto that canvas to correctly set background attributes. This may involve adding some new options on the API of the container and the widgets.

Hope this helps, let me know if I can provide further details.