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

Add peekViewEditor colors for dark theme #39

Merged
merged 2 commits into from
May 18, 2020
Merged

Conversation

this-fifo
Copy link
Contributor

Fix #38

@this-fifo
Copy link
Contributor Author

hi @simurai , while the default colors seem to work well for the light theme they are a bit off for the dark theme as @orsenkucher pointed out

I tried using the same orange as the title for the borders but I thought the default blue looked better instead so I left that one as is

@orsenkucher
Copy link
Contributor

@this-fifo now it looks a lot better bro!
Though I think we can make background a bit more bluish or distinct again.
And also add "peekViewResult.matchHighlightBackground": "#ffd33d44",
and maybe decrease intensiveness from #ffd33d44 to #ffd33d33 for peekViewEditor

@orsenkucher
Copy link
Contributor

@this-fifo @simurai, I know it can get messed up rly quickly, but we can try to approach highlights like Pop N' Lock did

Basically give to it some borders and matching fill color. And maybe switch from orange to bluish green color

What do you think?

@this-fifo
Copy link
Contributor Author

this-fifo commented May 16, 2020

I added the peekViewResult.matchHighlightBackground and decreased the peekViewEditor as you suggested, it does feel nicer! Thanks @orsenkucher 🙏

As for the blueish/distinct background, I opted for something more along the lines of other popular dark themes that rely on the borders to create visual context, here's an example of how it looks like for

Dracula

image

And for Atom One Dark

image

And now github-vscode-theme

image

@orsenkucher
Copy link
Contributor

Okey, sounds reasonable) I think this issue received great treatment and is basically ready to merge👍👍 @this-fifo

Copy link
Contributor

@simurai simurai left a comment

Choose a reason for hiding this comment

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

Didn't even know about this. 😅 Another option would be to "raise" the peekViewEditor, but we can consider another time.

Let's get this 🚢 ed. 👍

@simurai simurai merged commit 694ce28 into primer:master May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Highlight colors in peekViewEditor
3 participants