Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Improve/fix some built in themes #3314

Closed
ChrHorn opened this issue Aug 3, 2022 · 14 comments
Closed

Improve/fix some built in themes #3314

ChrHorn opened this issue Aug 3, 2022 · 14 comments
Labels
A-theme Area: Theme and appearence related C-enhancement Category: Improvements

Comments

@ChrHorn
Copy link
Contributor

ChrHorn commented Aug 3, 2022

Some themes that come with helix need some polish.

acme: statusline, menu, maybe errors
acme

base_16_transparent: menu bg same as standard bg (intentional, add border once it's possible)
base16_transparent

nord_light: ui menu selected bg very similar to standard bg
nord_light

rose_pine_dawn: ui menu selected bg same as standard bg
rose_pine_dawn

FIXED

dark_plus: indent guide bg
dark_plus

ayu_light: statusline fg, menu fg
ayu_light

@AlexanderBrevig
Copy link
Contributor

I agree, and made a tool for helping out #3234. A lot of the issues you point out are not yet checked against, but they could :)

@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-theme Area: Theme and appearence related labels Aug 3, 2022
@GreasySlug
Copy link
Contributor

About base16_transparent, it is intentionally the same as the background.
The reason for this is to maintain the uniformity of transparency.
I agree that it is hard to see, but it is not possible to change the transparency #2751 or add a border of the pop-up background.

@ChrHorn
Copy link
Contributor Author

ChrHorn commented Aug 4, 2022

Some of these might be intentional and fixes would be a bit opinionated. Pinging

@two-six for Acme, Nord Light
@enkodr for Ayu Light
@chunghha for Rose Pine Dawn

About base16_transparent, it is intentionally the same as the background. The reason for this is to maintain the uniformity of transparency. I agree that it is hard to see, but it is not possible to change the transparency #2751 or add a border of the pop-up background.

Makes sense, added a note.

@xcdkz
Copy link
Contributor

xcdkz commented Aug 4, 2022

Thanks for the ping, I'll look into fixing acme.
The main point of the nord light is the low contrast between elements of the ui, so in my opinion it looks just like it should although I'll try experimenting a little with it.

@AlexanderBrevig
Copy link
Contributor

Thanks for the ping, I'll look into fixing acme. The main point of the nord light is the low contrast between elements of the ui, so in my opinion it looks just like it should although I'll try experimenting a little with it.

It is very hard to read the undefined ["warning","error","info","hint","diagnostic"].

image

@ChrHorn
Copy link
Contributor Author

ChrHorn commented Aug 4, 2022

Thanks for the ping, I'll look into fixing acme. The main point of the nord light is the low contrast between elements of the ui, so in my opinion it looks just like it should although I'll try experimenting a little with it.

I think the main issue is that the code and the selected element blend into each other a bit too much. The contrast looks fine for the standard nord theme:
nord
Looks also fine in the VSCode Nord Light theme:
image

@xcdkz
Copy link
Contributor

xcdkz commented Aug 4, 2022

Are the colors terminal-dependent? I have no problems in Konsole, can you tell me what terminal do you use?
scrot helix

@AlexanderBrevig
Copy link
Contributor

It maybe/probably falls back to some base16 color set for the terminal? I use a gruvbox theme.

| alacritty | wezterm |
|   Foot    |  kitty  |

image

@ChrHorn
Copy link
Contributor Author

ChrHorn commented Aug 4, 2022

Acme uses a mix of hard coded colors and colors derived from the terminal theme. That's why it can look pretty bad when a dark terminal theme is used.

The issue for the Nord Light errors is that you used diagnostic.error instead of error. error will color the gutter and the overlaid text, diagnostic.error will only color the text.

@yelfathi
Copy link

yelfathi commented Jan 10, 2023

Using Acme theme on iterm2 (acme theme also), and I got this white background for errors don't know if it is related to this issue. Thanks!

EDIT: also the buffer line background.

image

@xcdkz
Copy link
Contributor

xcdkz commented Jan 10, 2023

@olbitset I have already fixed this in #5019, it just wasn't included in the latest release. You would need to compile helix from source or update $HOME/.config/helix/runtime/themes/acme.toml file to the current version.

@yelfathi
Copy link

Thanks @two-six I included as a personal theme. What about the TOP bufferline (as the selection one), is it expected to have a gray background? If so what is the parameter in the toml config to adapt it to my need?

@xcdkz
Copy link
Contributor

xcdkz commented Jan 10, 2023

Thanks @two-six I included as a personal theme. What about the TOP bufferline (as the selection one), is it expected to have a gray background? If so what is the parameter in the toml config to adapt it to my need?

Do you mean foreground? I see background is light blue in your screenshot, not gray.

Set "ui.bufferline" = { fg = "indent", bg = "acme_bar_bg" } to "ui.bufferline" = { fg = "black", bg = "acme_bar_bg" }.
If you want you can put any color you want there instead of black, like this: "ui.bufferline" = { fg = "#000000", bg = "acme_bar_bg" }

I've never used bufferline before, but as you mentioned current configuration doesn't really fit this theme really well. I will open a pull request to change it from gray to black.

@yelfathi
Copy link

yelfathi commented Jan 10, 2023

Sorry I was talking about the bg. Thanks for the inputs I will adapt to my preferences!

@helix-editor helix-editor locked and limited conversation to collaborators Apr 20, 2024
@pascalkuthe pascalkuthe converted this issue into discussion #10516 Apr 20, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
A-theme Area: Theme and appearence related C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

6 participants