Fix issue 244

[mcDevnagh]

Added support for Terminal Default Colors (which may or may not be transparent)

• Specifying "none" in your config file sets the color to the default color of the terminal, or tcell.ColorDefault, as defined here.
• Invalid colors still result in an error, so malformed config files alert the user instead of setting the color to the default.
• Updated the default config and color string for cview color tags to reflect this

On terminals which support transparency and are configured as such, setting bg = "none" keeps the transparency

example with the simple terminal using the alpha patch:

[makew0rld]

Nice solution, thank you. And it looks good in your screenshot! I've never seen Amfora like that before, it's the right way to go.

Since your solution only works for backgrounds (for text it just seems to use white every time) I've suggested a change that will make it only apply when used with bg. Please note that in default-config.toml.

Also, this should be changed from "none" to "default", I think that name works better.

After making your changes, run make gen (or just go generate ./...), like I did for this PR earlier in df21b0b. This will copy the config change into the code.

[mcDevnagh]

Sure thing! I'll get right to working on that.
Are you sure you want to limit it just to just bg? I think the user could decide what to make the default

Maybe, limit default only to values which affect the <background> of color tags [<foreground>:<background>:<flags>], which I'm assuming is all the settings ending in bg, but I'll double check before finalizing the PR, if this is the way you'd like to go.

Also, no need to worry about the "transparency". It will always show right through the terminal instead of revealing any widgets behind, because the background isn't really transparent, it's "filled" with the color, and overwrites the pixels there. Transparency to reveal other widgets would require this

Here is an example where both the bg and info_modal_bg are set to "none", soon to be "default". (I also set info_modal_text = "slateblue" to make it more readable)

[makew0rld]

Maybe, limit default only to values which affect the <background> of color tags

Sounds good to me! Good point that those ones could make use of this setting as well.

[mcDevnagh]

@makeworld-the-better-one
I completed what you asked of me.
I also found a UI quirk:
When a UI element was selected, the bg and text would swap. However, the if bg was "default", which could be dark, now the text was "default", which is always white. This lead to light text on a light background. I circumvent this with a new function GetTextColor(key, bg string) tcell.Color, which, can return black or white depending on the brightness of bg, only if the key is ColorDefault. Otherwise it just returns the color in the theme map

Also, "default" is valid for all theme settings ending in "bg", and if the user tries setting anything else to "default", they get the following error:

[makew0rld]

Thanks for updating!

the text was "default", which is always white. This lead to light text on a light background. This lead to light text on a light background. I circumvent this with a new function

I mentioned that in my previous comment. I don't think this function should happen. Until using default for text can work, whatever the user specified for text color (even white on white) should be respected.

[mcDevnagh]

Yes, so the user cannot specify "default" for text, but because they can specify UI elements to have a background of "default", and the text and bg switch colors on focus, text can become ColorDefault on focus.

My code always respects the user's config. The only time the text can change depending on the background is when a UI element is focused, and its background is "default". The effect of my code is the same as yours otherwise

[makew0rld]

The "middle gray" as perceived by humans is 119. So I would set gray equal to that. I would calculate the gray value of the color more accurately as 77*r+150*g+29*b + (1<<7) >> 8, and then compare that to gray. Overall this will be more accurate in determining color luminance.

Otherwise looks good, thank you!

[makew0rld]

Sorry to keep going back and forth on this, just noticed some more things as I went through the PR. Thanks for your work.

[makew0rld]

Isn't this function call, and all the others in this PR, incorrect? The function signature for GetTextColor is GetTextColor(key, bg string) tcell.Color, which would indicate having "btn_bg" second and "btn_text" first. Or am I misunderstanding?

[mcDevnagh]

for focused UI elements, the text color and bg color swap. When focused, the button has text color of whatever "btn_bg" is set to, and a bg of whatever color "btn_text" is set to.

Because "default" is not a valid text color in the config, this function is always called with a key ending with bg as its key, and a bg ending with text.

I tried to be as concise as possible with the naming. Maybe SwapColor(new_text_key, new_bg_key string) or something similar would be less confusing.

[makew0rld]

No, I think it's good the way it is then, I just misunderstood.

[makew0rld]

Thank you for working on this! Sorry for sleeping on it, don't be afraid to ping if you need in the future.

[mcDevnagh]

happy to help :)