Sitelet https://github.com/primer/github-vscode-theme/pull/39
Skip to content

Add peekViewEditor colors for dark theme - #39

Merged
simurai merged 2 commits into
primer:masterfrom
this-fifo:master
May 18, 2020
Merged

simurai merged 2 commits into
primer:masterfrom
this-fifo:master

Conversation

@this-fifo

Copy link
Copy Markdown
Contributor

Fix #38

@this-fifo

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

this-fifo commented May 16, 2020 •

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor

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

@simurai simurai left a comment

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.

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