-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add an ansi theme #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
src/tui/theme.rs
Outdated
| charge_bar: Color::Indexed(6), | ||
| bar_background: Color::Indexed(254), | ||
| highlighted_text: Color::Indexed(3), | ||
| informative_text: Color::Indexed(4), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: I used the numbers that looks good with my theme, but maybe some of them must be changed to match other themes. I'd welcome if someone with another theme can test this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmhh not great. I need to revisit that.
|
Hi! I'm happy to merge it once the CI is fixed. |
|
with another light theme (github):
With gruvbox-dark-hard:
Seems ok to me, so maybe indeed it is #80 indeed |
|
I would support this being the default! I suspect most people that have gone to the trouble of setting up a color scheme in their terminal uses it because they like it and are used to it. This brings the app features into focus instead of having to try to interpret what the color scheme is about. |
Sounds good to me. I don't mind making ANSI the default. In this case, we might want to rename it to |
|
Nice, glad you like this! I made some adaptations to make it look like the already present gruvbox theme, when I use the same theme in the terminal. |
|
And I renamed it default and made it the default. |
|
Once this has been merged (or before) we'll need to move the Should Default take its place first in the list or should it also be alphabetical? - Or is there a different system we should use? |
I think it should go first in the list. The rest of them must be in alphabetical order (just my opinion). |
Agreed
You may need to modify the tests in I have also been trying to keep the themes in I can add a PR to your repo if you'd like me to apply this fix. |
Same in my case. |
Actually, For designing this theme, I used the base16 convention. It is widely used, but AFAIK not an universal convention. The way I did it, it is guaranteed to display well for those themes, but will it display ok with - for instance - the default ubuntu theme? Maybe we should not use color n° 16 for the default theme, and have another theme for base16 terminal theme? I will test on a default installation of ubuntu to check. I can test other distro as well. |
|
My screenshot was from the TTY. Which is probably where it should work no? |
It makes sense too. |





This adds a theme that uses user defined colors in the terminal, so that framework-tool-tui is consistent with system themes.
Fix #96