cronvel / terminal-kit

Terminal utilities for node.js
MIT License
3.08k stars 198 forks source link

`Text` defaults to `{ bgColor: 'brightBlack' }`, but shouldn't #214

Closed d4h0 closed 1 year ago

d4h0 commented 1 year ago

Hi @cronvel,

Thanks for creating TerminalKit, it looks pretty neat!

(Unfortunately, the docs of the document model are missing a lot... 🙁).

I noticed that Text elements of the document model default to the attributes: { bgColor: 'brightBlack' }, and I'm wondering why (and if that behavior can be removed).

This happens on the following line:

this.attr = options.attr || { bgColor: 'brightBlack' } ;

This basically means, that I always have to pass in a fake attr property (if I'm not adding anything to attr), because I never what the background of any text to be brightBlack (it should be the default terminal background).

The above is relevant if we style the text via the fantastic markup feature (by setting the contentHasMarkup property true. By the way, it would be great, if that property somehow could be activated globally, so we don't have to add it to every Element).

cronvel commented 1 year ago

Hi @d4h0

Almost all Element instances have default attribute values, it is supposed to have good default, also it's obviously kind of subjective. Since it's part of the document model, it's important to override the terminal default background (unlike the inline mode API), since you must have full control over the styles. Think of it as an HTML page.

To be honest, it's impossible to build something reliable using default terminal colors: once you use more color, you must force background color to something known, and black is the best choice most of times. There are too many colors that doesn't play well with a white background (especially yellow and cyan).

d4h0 commented 1 year ago

That makes sense. Thanks for the explanation, @cronvel!