Skip to content

refine darcula and darcula-solid themes - #8412

Merged
pascalkuthe merged 5 commits into
helix-editor:masterfrom
boofexxx:darcula-theme
Sep 28, 2023
Merged

refine darcula and darcula-solid themes#8412
pascalkuthe merged 5 commits into
helix-editor:masterfrom
boofexxx:darcula-theme

Conversation

@boofexxx

@boofexxx boofexxx commented Sep 27, 2023

Copy link
Copy Markdown
Contributor

master branch:

darcula:

Screenshot from 2023-09-27 16-21-30

darcula-solid:

Screenshot from 2023-09-27 16-21-26

changed version:

darcula:

Screenshot from 2023-09-27 16-21-16

darcula-solid:

Screenshot from 2023-09-27 16-21-03

@the-mikedavis the-mikedavis changed the title make it easier to distinguish between primary and secondary selections darcula: distinguish between primary and secondary selections Sep 27, 2023
@the-mikedavis

the-mikedavis commented Sep 27, 2023

Copy link
Copy Markdown
Member

\cc @nogden & @jesusmgg - what do you think?

@the-mikedavis the-mikedavis added A-theme Area: Theme and appearence related S-waiting-on-review Status: Awaiting review from a maintainer. labels Sep 27, 2023
@archseer

archseer commented Sep 27, 2023

Copy link
Copy Markdown
Member

I would keep the default cursor shape for secondary selections, the underline seems more like a stylistic choice

@jesusmgg

jesusmgg commented Sep 27, 2023

Copy link
Copy Markdown
Contributor

I concur, the secondary selection color seems like an upgrade to me but let's keep the original cursor shape.

@nogden

nogden commented Sep 27, 2023

Copy link
Copy Markdown
Contributor

This looks good to me. I also agree that we should keep the default cursor shape for secondary selections. Thanks for the contribution.

nogden
nogden suggested changes Sep 27, 2023
View reviewed changes
Comment thread runtime/themes/darcula.toml Outdated
"ui.statusline.select" = { bg = "orange", fg = "grey01" }
"ui.help" = { fg = "grey04", bg = "grey01" }
"ui.cursor" = { fg = "grey04", modifiers = ["reversed"] }
"ui.cursor" = { bg = "grey02", modifiers = ["underlined"] }

@nogden nogden Sep 27, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed lets keep this as the default.

@boofexxx boofexxx Sep 27, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@boofexxx

boofexxx commented Sep 27, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I made changes to secondary cursor because for some reasons they seem to be distracting for me

master

image

changed version

image

Bug

And seems like I'm introducing a bug with these changes for darcula-solid because now it's not possible to say which argument we're on in documentation popups

image

With the following changes on darcula-solid.toml it gets better a lil bit:

"ui.popup" = { fg = "grey05", bg = "grey00" }

image

how it looks in master

image

for just darcula it looks pretty fine

master

image

changed version

image

markup.raw

And if you don't mind can we change markup.raw in darcula.toml from white to purple or any other color

"markup.raw" = "white"

image

to

"markup.raw" = "purple"

image

@jesusmgg

jesusmgg commented Sep 27, 2023

Copy link
Copy Markdown
Contributor

Everything looks great now to me. Just the PR title should be updated to reflect the more extensive changes.

@boofexxx boofexxx changed the title darcula: distinguish between primary and secondary selections refine darcula and darcula-solid themes Sep 27, 2023
@boofexxx
boofexxx requested a review from nogden September 27, 2023 19:34
@pascalkuthe
pascalkuthe merged commit 77fe8f2 into helix-editor:master Sep 28, 2023
danillos pushed a commit to danillos/helix that referenced this pull request Nov 21, 2023
@boofexxx @danillos
refine darcula and darcula-solid themes (helix-editor#8412)
b18adda
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
@boofexxx @dgkf
refine darcula and darcula-solid themes (helix-editor#8412)
87359db
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
@boofexxx @mtoohey31
refine darcula and darcula-solid themes (helix-editor#8412)
81cf554
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
@boofexxx Schuyler Mortimer
refine darcula and darcula-solid themes (helix-editor#8412)
6ea4136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@the-mikedavis the-mikedavis the-mikedavis approved these changes
@pascalkuthe pascalkuthe pascalkuthe approved these changes
+2 more reviewers
@nogden nogden nogden approved these changes
@jesusmgg jesusmgg jesusmgg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-theme Area: Theme and appearence related S-waiting-on-review Status: Awaiting review from a maintainer.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@boofexxx @the-mikedavis @archseer @jesusmgg @nogden @pascalkuthe