Improve support for different color depths (16, 1) #563
Conversation
neiljp
force-pushed
the
2020-03-terminal-color-depth
branch
from
238dc5f to
c094a46
Compare
neiljp
force-pushed
the
2020-03-terminal-color-depth
branch
2 times, most recently
from
e4f6057 to
99c6519
Compare
There was a problem hiding this comment.
@neiljp Thanks, this will extend Zulip Terminal's color depth support and make it easier to add 24-bit support later down the road. 👍
This seems to work well and close to being merged. However, this doesn't add support for configuring color-depth via zuliprc. We may choose to address this later.
zulipterminal/ui_tools/buttons.py
Outdated
| @@ -136,7 +136,8 @@ def __init__(self, properties: List[Any], | |||
| inverse_text = background if background else 'black' | |||
| break | |||
| view.palette.append(( | |||
| self.color, '', '', '', self.color+', bold', background)) | |||
| self.color, 'dark blue, bold', 'black', '', | |||
There was a problem hiding this comment.
While this makes streams standout in dark themes, it doesn't add much (aesthetically) to the light themes (light and blue). We might want to drop (as you mention in the PR) and address this in a follow-up PR later.
zulipterminal/config/themes.py
Outdated
| ('bold', 'white, bold', '', 'bold'), | ||
| ('footer', 'white', 'dark red', 'standout'), | ||
| ('starred', 'light red, bold', '', 'bold'), | ||
| ('category', 'light blue, bold', '', 'standout'), |
There was a problem hiding this comment.
Given that theme_with_monochrome_added is introduced in the next commit, which essentially does similar amendments, are these changes required?
There was a problem hiding this comment.
I had these present to prototype what an improved monochrome theme might look like, without adding it to every theme - then realized that actually we would have to add it everywhere.
I've essentially skipped this commit in the next push, and instead removed the mono styles for each theme.
There was a problem hiding this comment.
Note that having this in here isn't an issue - it's easy enough to drop an individual commit before merging :)
There was a problem hiding this comment.
I understand the prospect now; thanks for the explanation. :)
neiljp
force-pushed
the
2020-03-terminal-color-depth
branch
from
99c6519 to
b03048d
Compare
There was a problem hiding this comment.
@neiljp Thanks for the improvements. The changes and commits look good to me. 👍
Monochrome styles in each theme were limited at best and out of date; this commit removes them in favor of a default theme when in monochrome mode. The required_styles set is extended into a dict, now including the values of monochrome settings for each style. The new theme_with_monochrome function amends each style in a theme to use these values, overwriting them if they already exist in a theme. Tests added for theme_with_monochrome_added.
neiljp
force-pushed
the
2020-03-terminal-color-depth
branch
from
b03048d to
e48d67e
Compare
Currently we just run with 256 colors and assume the user system supports that.
This is reasonable to a great degree, but urwid supports palettes from 1 (monochrome), 16, and others too. The first commit in this PR enables specifying color depths of 16 or 256 on the command line, defaulting to 256, which may help with reproducing color rendering issues. The subsequent commit is a minor change which makes the 256-bit stream colors be rendered with a color; I'm open to dropping this commit as we could do something different.
The last three commits extend the first commit to allow running in monochrome, add reasonable defaults in the default theme, and finally overwrite the monochrome settings in all themes at run-time. I can't imagine we'll actually want monochrome themes, which is why I made the final change, and these commits could be squashed/dropped and combined with other work to just have empty values in the monochrome settings in themes if this looks good. I was using it just now 👍