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

Sonokai theme: style secondary selections differently #5440

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

jlebon
Copy link
Contributor

@jlebon jlebon commented Jan 7, 2023

Without styling the primary and secondary selections differently, it's
impossible to tell them apart when cycling through selections.

Make the primary selection slightly brighter and secondary selections
slightly paler.

@the-mikedavis
Copy link
Member

\cc @p4ymak what do you think of the new color?

@jlebon
Copy link
Contributor Author

jlebon commented Jan 8, 2023

I suppose I should've included a screenshot. Here's what it looks like:

image

In the above, the third test is the primary selection. The other ones use the paler secondary styling. It's subtle but really obvious when cycling through them.

@Omnikar
Copy link
Contributor

Omnikar commented Jan 8, 2023

That seems like a pretty subtle difference; shouldn't it be a bit more noticeable?

@jlebon
Copy link
Contributor Author

jlebon commented Jan 9, 2023

The thing is that it's already quite pale. If we make it paler, it's harder to see against the background. It's more invasive, but we could also make the primary selection whiter to increase the contrast. E.g.:

image

This is with:

diff --git a/runtime/themes/sonokai.toml b/runtime/themes/sonokai.toml
index 18417f00..45ad6ef6 100644
--- a/runtime/themes/sonokai.toml
+++ b/runtime/themes/sonokai.toml
@@ -55,7 +55,8 @@
 "ui.cursor.match" = { fg = "orange", bg = "diff_yellow" }
 "ui.cursor.insert" = { fg = "black", bg = "grey" }
 "ui.cursor.select" = { fg = "bg0", bg = "blue" }
-"ui.selection" = { bg = "bg4" }
+"ui.selection" = { bg = "bg5" }
+"ui.selection.primary" = { bg = "bg4" }
 "ui.linenr" = "grey"
 "ui.linenr.selected" = "fg"
 "ui.cursorline.primary" = { bg = "bg1" }
@@ -86,7 +87,8 @@ bg0 = "#2c2e34"
 bg1 = "#33353f"
 bg2 = "#363944"
 bg3 = "#3b3e48"
-bg4 = "#545862"
+bg4 = "#5C606A"
+bg5 = "#444852"
 bg_red = "#ff6077"
 diff_red = "#55393d"
 bg_green = "#a7df78"

@archseer
Copy link
Member

archseer commented Jan 9, 2023

It's more invasive, but we could also make the primary selection whiter to increase the contrast.

This seems reasonable to me 👍🏻

@NickCondron
Copy link
Contributor

I also have a PR to change this theme (#5379) fyi. I don't think they conflict though.

@p4ymak
Copy link
Contributor

p4ymak commented Jan 9, 2023

@the-mikedavis
sorry for long respond.

in my opinion, in general, the palette has become better. there are only two things I don't like: macros are no different from functions now, and parentheses and punctuation are too dim.

@kirawi kirawi added A-theme Area: Theme and appearence related S-waiting-on-review Status: Awaiting review from a maintainer. labels Jan 9, 2023
Without styling the primary and secondary selections differently, it's
impossible to tell them apart when cycling through selections.

Make the primary selection slightly brighter and secondary selections
slightly paler.
@jlebon
Copy link
Contributor Author

jlebon commented Jan 22, 2023

Updated this as per discussion!

in my opinion, in general, the palette has become better. there are only two things I don't like: macros are no different from functions now, and parentheses and punctuation are too dim.

Did you mean to comment this in the other PR modifying the Sonokai theme?

@p4ymak
Copy link
Contributor

p4ymak commented Jan 23, 2023

Did you mean to comment this in the other PR modifying the Sonokai theme?

Yes! You are right.

@the-mikedavis the-mikedavis merged commit 4726ae9 into helix-editor:master Jan 24, 2023
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.

7 participants