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

Dim primary selection in kanagawa #10094

Merged
merged 2 commits into from
Apr 1, 2024
Merged

Conversation

mxpv
Copy link
Contributor

@mxpv mxpv commented Apr 1, 2024

I'm using kanagawa style and sometimes its pretty difficult to recognize selections:
Here is an example.

No selection:
image
Selected:
image

This PR adds dimming to make it a bit more obvious:

image image

@the-mikedavis the-mikedavis changed the title Selections are barely visible in kanagawa Dim primary selection in kanagawa Apr 1, 2024
@the-mikedavis
Copy link
Member

\cc @zetashift what do you think?

IMO it would be better to make the selection background brighter rather than make the cursor and selection dimmer. The dim modifier turns down the contrast enough that it can be hard to read the selection contents

@the-mikedavis the-mikedavis added A-theme Area: Theme and appearence related S-waiting-on-review Status: Awaiting review from a maintainer. labels Apr 1, 2024
@zetashift
Copy link
Contributor

+1 for making the selection background brighter, the low constract between text and selection makes it harder for me to read.

Besides that, for me the contrast is "good enough", so I never noticed it:
image

But I definitely can see why this could be hard to read! I would like to see a version that has a brighter selection background.

Signed-off-by: Maksym Pavlenko <[email protected]>
@mxpv
Copy link
Contributor Author

mxpv commented Apr 1, 2024

Sounds good. I borrowed a few more colors from the original theme. Here is what I've got:

image image image

What do you think?

@mxpv
Copy link
Contributor Author

mxpv commented Apr 1, 2024

Here is another variation with sumiInk5 (slightly less bright):

image image

I'm ok with both options

@zetashift
Copy link
Contributor

Here is another variation with sumiInk5 (slightly less bright):
...
I'm ok with both options

Both of them look good to me as well, but if I have to chose I'll go with the sumiInk5 one, for me it reads the easiest!
Thank you :D.

@mxpv
Copy link
Contributor Author

mxpv commented Apr 1, 2024

I've played a bit more with colors, I feel like sumiInk6 might be to bright overall, so I've updated the PR to use sumiInk5. Unless you feel strongly about it, I'd go with this one.

with sumiInk6:
image

with sumiInk5:
image

@pascalkuthe pascalkuthe merged commit 8635913 into helix-editor:master Apr 1, 2024
6 checks passed
@mxpv mxpv deleted the kanagawa branch April 1, 2024 22:41
@mxpv
Copy link
Contributor Author

mxpv commented Apr 1, 2024

Thanks!

Desdaemon pushed a commit to Desdaemon/helix that referenced this pull request Apr 3, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
* [kanagawa] Change selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

* [kanagawa] Use less brigh selection color

Signed-off-by: Maksym Pavlenko <[email protected]>

---------

Signed-off-by: Maksym Pavlenko <[email protected]>
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.

4 participants