makew0rld / amfora

A fancy terminal browser for the Gemini protocol.
GNU General Public License v3.0
1.16k stars 67 forks source link

Fix issue 244 #245

Closed mcDevnagh closed 3 years ago

mcDevnagh commented 3 years ago

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

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: example

mcDevnagh commented 3 years ago

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) transparent-bg-and-modal.png

makew0rld commented 3 years ago

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 commented 3 years ago

@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 pic-full-210629-180837 pic-full-210629-180845

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: pic-selected-210629-1758-02

makew0rld commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

mcDevnagh commented 3 years ago

happy to help :)