Skip to content
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

Update everforest themes #11459

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Update everforest themes #11459

merged 1 commit into from
Aug 13, 2024

Conversation

0rphee
Copy link
Contributor

@0rphee 0rphee commented Aug 10, 2024

  • add background colors to diff gutters and lsp errors/warnings/etc
  • stylize v2 picker columns

Hi! I made this changes so that the git diff and lsp warnings are more visible, and the column names of the new picker are differentiated from the data.

@0rphee
Copy link
Contributor Author

0rphee commented Aug 10, 2024

This is how the changes look like:

Screenshot 2024-08-09 at 8 09 06 p m

@archseer
Copy link
Member

\cc @CptPotato

@CptPotato
Copy link
Contributor

I think the diff gutter without background looked a bit cleaner and had good visibility already. But I don't particularly dislike the new one.

Styling the table header is a good idea, though I think a background color looks a little out of place here. Maybe bold and underlined could look a bit better here?

Using the background color to highlight diagnostics is something I like and do in my personal themes because my terminal doesn't support colored underlines. Though, it's a matter of preference and some users might find it too distracting.

All in all, I'm not necessarily against these changes but maybe there are other users who use this theme that can give a second opinion.

@0rphee
Copy link
Contributor Author

0rphee commented Aug 10, 2024

The bold & underlined table header looks better the way you said, i think.
Screenshot 2024-08-10 at 2 16 14 p m
Screenshot 2024-08-10 at 2 16 46 p m

As for the background highlight of the diff and lsp warnings, I prefer it to this way, but don't know about more people.

@pascalkuthe
Copy link
Member

I don't think a theme should be setting the background color for any in-text highlights. That is known to cause issues with some UI elements like cudor line and I also find too distracting. Usually I don't comment about that in a eeview since themes are personal but I actually use everforest so I don't think I would want to see that change here.

I also prefer the diff gutter without the background color but I am not quite as opposed to that as it doesnt interfere with other ui elements.

@the-mikedavis the-mikedavis added the A-theme Area: Theme and appearence related label Aug 11, 2024
* stylize v2 picker columns
@0rphee
Copy link
Contributor Author

0rphee commented Aug 13, 2024

I've updated the branch so that the only changes are in the styling of the picker headers.

@0rphee 0rphee mentioned this pull request Aug 13, 2024
@CptPotato
Copy link
Contributor

I've updated the branch so that the only changes are in the styling of the picker headers.

Thanks, and sorry for the nit-picking.

@archseer archseer merged commit f65ec32 into helix-editor:master Aug 13, 2024
6 checks passed
mxxntype pushed a commit to mxxntype/helix that referenced this pull request Aug 14, 2024
kyruzic pushed a commit to kyruzic/helix that referenced this pull request Sep 27, 2024
plul pushed a commit to plul/helix that referenced this pull request Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-theme Area: Theme and appearence related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants