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

theme: add starlight #8787

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Conversation

eemilhaa
Copy link
Contributor

Hi! This is a higher contrast, usability-focused theme. It uses the colors of the Starlight ansi color palette that is designed with accessibility and usability as a priority.

starlight

My use case with this theme is to have a dark theme that still maintains readability in brighter ambient light and allows for less monitor brightness.

Below are a couple design choices I've made that differ from many other bright themes already in Helix:

  • Fewer highlighting scopes: Keep scopes (and colors) distinguishable
  • Even brightness: I've avoided big differences in brightness to keep all scopes readable even in bright light
  • UI should be readable too: All selections and cursors are very visible, and differentiating between primary and secondary selections should be easy

@saccarosium
Copy link
Contributor

I would suggest to make whitespace color the same as the indent guide to make it less intrusive on the eyes.

"ui.virtual"                = "dark-fg"
"ui.virtual.ruler"          = { bg = "medium-bg" }
+ "ui.virtual.whitespace"   = "indent"
"ui.virtual.indent-guide"   = "indent"
Before After
Screenshot 2023-11-12 at 22 13 15 Screenshot 2023-11-12 at 22 12 56

@eemilhaa
Copy link
Contributor Author

Thanks for testing this! I do agree that aesthetically the dim whitespace looks better. However, I would not want to decrease its brightness, since the point of this theme is readability. My thought process is that If a UI element is enabled, it should be clearly visible (within reason, of course).

Also, the current relative brightness of whitespace is pretty much in line with some other brighter themes (for example dark plus, monokai, molokai) so I don't think it's exceedingly bright.

@saccarosium
Copy link
Contributor

However, I would not want to decrease its brightness, since the point of this theme is readability

I get your point but, readability is about make stand out more the important stuff. Currently the whitespace color is very similar, in brightness and shade, to the foreground color. This makes the two the same level of importance, in terms of visual hierarchy. And this is a bad thing since we are trying to make the code stand out more and not the whitespace. Also in a file tents to be a lot of whitespaces; that will produce a lot of visual noise, that will hurt readability.

To prove this, set your display to black and white (commonly used tactic to test the accessibility of a design) and open whatever file you want.

Result

Screenshot 2023-11-13 at 12 13 42

or simply open a markdown file.

Result

Screenshot 2023-11-13 at 12 21 12

My eyes, as they are parsing throw the text, do a lot of work to try distinguish what to focus on.

My thought process is that If a UI element is enabled, it should be clearly visible (within reason, of course).

Yes, but not all UI elements have the same weight and whitespaces aren't as important as the text itself.

Also, the current relative brightness of whitespace is pretty much in line with some other brighter themes (for example dark plus, monokai, molokai) so I don't think it's exceedingly bright.

Ok, but those theme don't have the goal of readability and accessibility. Also I've open a PR to change the dark_plus whitespace color for the same reason.

@eemilhaa
Copy link
Contributor Author

eemilhaa commented Nov 13, 2023

I agree on the visual noise.

With my testing, whitespace characters become impossible to see in bright conditions if they are much dimmer. But, if their brightness hurts usability, then I agree that's a larger issue.

I personally never use whitespace rendering, so feedback from others is very welcome on this. As you obviously do use the feature, I think your input is valid.

If someone has opinions on this, feel free to comment! If no one else feels strongly about this, I'll dim the whitespace.

I'll convert this pr into a draft for a couple of days.

@eemilhaa eemilhaa marked this pull request as draft November 13, 2023 13:06
@mlemesle
Copy link
Contributor

Hey there @eemilhaa !
Very nice theme ! What do you think about adding one more color, like light purple or something to nuance a bit more some elements like enum variants, macros, builtin constants etc ?

@eemilhaa
Copy link
Contributor Author

@mlemesle Hi! I'm pretty set on the syntax scopes as they are. I'd rather not add more colors since I'm already using all of the colors in the starlight palette (well technically I don't use red in syntax, but I've made that choice due to its lower brightness).

@mlemesle
Copy link
Contributor

@eemilhaa I undersand, thanks !

@eemilhaa eemilhaa marked this pull request as ready for review November 15, 2023 12:35
@eemilhaa
Copy link
Contributor Author

Whitespace is now dimmer (uses the indent color instead of dark foreground). Also tweaked the indent color to be the exact dark-gray from starlight.

@kirawi kirawi added the A-theme Area: Theme and appearence related label Nov 16, 2023
@the-mikedavis the-mikedavis added the S-waiting-on-review Status: Awaiting review from a maintainer. label Nov 16, 2023
@pascalkuthe pascalkuthe merged commit 3c8bf9d into helix-editor:master Nov 17, 2023
6 checks passed
danillos pushed a commit to danillos/helix that referenced this pull request Nov 21, 2023
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 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 S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants