mirror of
https://github.com/mum4k/termdash.git
synced 2026-04-26 19:05:52 +03:00
[GH-ISSUE #326] options that set foreground / background color don't seem to be propagated to tcell #153
Labels
No labels
bug
cleanup
enhancement
enhancement
enhancement
good first issue
help wanted
help wanted
pull-request
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/termdash#153
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @tcurdt on GitHub (Apr 30, 2022).
Original GitHub issue: https://github.com/mum4k/termdash/issues/326
As terminal background might differ the color scheme needs to match.
To avoid this I would like to set background color to black.
I tried this:
but that doesn't quite do the right thing:
Is there a way to do this that I am not seeing?
@mum4k commented on GitHub (May 3, 2022):
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, ortermdashis 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 commented on GitHub (May 3, 2022):
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 commented on GitHub (May 5, 2022):
Thank you for reporting this @tcurdt. Sounds good, I will also keep an eye on this in case I find some time.
@tcurdt commented on GitHub (Jul 14, 2022):
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
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 commented on GitHub (Jul 15, 2022):
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 commented on GitHub (Jul 15, 2022):
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 commented on GitHub (Jul 18, 2022):
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 commented on GitHub (Jul 18, 2022):
@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.