Command palette modal makes data table zebra stripes the wrong color in light mode

[godlygeek]

Run python docs/examples/widgets/data_table_cursors.py from a repo clone, and use the command palette to switch to light mode:

Then open the command palette, and while the modal is displayed note that the zebra stripes change colors past the last column containing data (to lilac or light purple):

This doesn't seem to happen in a dark mode theme:

[github-actions[bot]]

We found the following entries in the FAQ which you may find helpful:

• Does Textual support images?
• Why do some key combinations never make it to my app?
• Why doesn't Textual support ANSI themes?

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

[davep]

I would suspect this is a ModalScreen effect, not really the CommandPalette specifically.

[TomJGooding]

Presumably also this is due to 256-color limitations and looks fine on terminals with true color support.

[davep]

Good spot @TomJGooding, hadn't considered that (although I hadn't started to look at this yet either).

Very much one of the reasons we ask folk to run textual diagnose and include it in their report.

[godlygeek]

Very much one of the reasons we ask folk to run textual diagnose and include it in their report.

Fair enough:

Textual Diagnostics

Versions

Name	Value
Textual	0.38.1
Rich	13.5.2

Python

Name	Value
Version	3.11.5
Implementation	CPython
Compiler	GCC 10.2.1 20210130 (Red Hat 10.2.1-11)
Executable	/home/mwoznisk/.pyenv/versions/3.11/envs/textual/bin/python3

Operating System

Name	Value
System	Linux
Release	5.15.90.1-microsoft-standard-WSL2
Version	#1 SMP Fri Jan 27 02:56:13 UTC 2023

Terminal

Name	Value
Terminal Application	Unknown
TERM	putty-256color
COLORTERM	Not set
FORCE_COLOR	Not set
NO_COLOR	Not set

Rich Console options

Name	Value
size	width=239, height=64
legacy_windows	False
min_width	1
max_width	239
is_terminal	True
encoding	utf-8
max_height	64
justify	None
overflow	None
no_wrap	False
highlight	None
markup	None
height	None

[godlygeek]

Presumably also this is due to 256-color limitations and looks fine on terminals with true color support.

My terminal seems to have true color support:

[TomJGooding]

Here's how it looks for me in alacritty, which I know has true color:

textual diagnose is reporting your terminal as putty-256color.

[godlygeek]

And how does it look when the modal isn't displayed?

[TomJGooding]

[godlygeek]

OK, so that's interesting. Yeah, the row past the last cell is a different color for you even when the modal isn't showing. For me, it's the same color when the modal isn't showing, but a different color when it is. So I suppose you're right:

1. That probably implies that Textual is only using 256 colors for me despite my terminal having truecolor support
2. When the modal isn't showing both colors that textual wants to display get rounded to the same 256-color color, but when the modal is showing it rounds one of them to a different color

[TomJGooding]

Perhaps you could try export COLORTERM=truecolor?

Also just to clarify: I'm not saying the DataTable or ModalScreen shouldn't account for 256-color terminals. My comment was really just an observation.

[godlygeek]

Perhaps you could try export COLORTERM=truecolor?

That does make what I see match your screenshot.

[TomJGooding]

I guess your terminal does really support true color (despite being named putty-256color!), but doesn't actually advertise it by setting this variable. I'm not an expert by any means, but I'm not sure how Textual could work around that.

[godlygeek]

Well, let's set aside the question of whether Textual should be enabling true color mode for my terminal or not, and focus on whether Textual's behavior is right or wrong when 256-color mode is being used instead of true color mode.

The behavior we're seeing here is that Textual wants to display the zebra stripes in color X for the portion of the line where there are cells and color Y for the portion past the last cell. On a 256 color terminal, it displays them in roundToNearest256Color(X) and roundToNearest256Color(Y) instead, which is perfectly reasonable. On the light theme, roundToNearest256Color(X) and roundToNearest256Color(Y) happen to round to the same color.

Then, the modal pops up, and Textual wants to darken the layer under the modal. At that point it decides to display the stripes with colors darken(X) and darken(Y) instead. Again it needs to round those to the nearest 256-color color, and we wind up with roundToNearest256Color(darken(X)) and roundToNearest256Color(darken(Y)). On the light theme, those don't evaluate to the same color.

It seems to me that this behavior is wrong: Textual should be darkening the color that was actually displayed, not the color that it wanted to display, since otherwise things won't darken uniformly, and two things that were displayed as the same color prior to darkening become different colors after darkening, which is an extremely surprising result for darkening to have.

That is, currently it's doing roundToNearest256Color(darken(Y)), and I think it should probably instead be doing roundToNearest256Color(darken(roundToNearest256Color(Y)))

[TomJGooding]

Well, let's set aside the question of whether Textual should be enabling true color mode for my terminal or not, and focus on whether Textual's behavior is right or wrong when 256-color mode is being used instead of true color mode.

This is what I was hinting in my post before last, I think you make some good points. But I'm not a maintainer of Textual, just an ~annoying hanger-on~ eager contributor, so I think I should bow out here.

[willmcgugan]

@godlygeek Your terminal should be setting the COLORTERM env var. It's the only way for apps to detect truecolor.

Textual will reduce colors from 16.7 million to 240 if it doesn't detect truecolor. The results can never be perfect. I doubt there are any improvements that could be made to the color reduction code.

BTW The modal in light mode doesn't darken the background, it lightens it.

[github-actions[bot]]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

[godlygeek]

The results can never be perfect.

Sure, but they could be better, though. It's a subpar experience for users with terminals that don't set COLORTERM=truecolor that Textual takes two things that display as the same color, and lightens them to display as two different colors.

I can appreciate that it might be architecturally infeasible, but it's definitely not the case that nothing better could possibly be done: if the colors initially displayed were downgraded, then it would be better if the lightening was performed based on those downgraded colors, rather than the original not downgraded colors.