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

Strings are too light #19

Open
simurai opened this issue May 8, 2020 · 22 comments
Open

Strings are too light #19

simurai opened this issue May 8, 2020 · 22 comments
Labels
dark type: syntax Syntax highlighting

Comments

@simurai
Copy link
Contributor

simurai commented May 8, 2020

Feedback from a DM:

The contrast of the string is a bit light

Screenshot 2020-05-08 at 10 20 47

compared to the default

Screenshot 2020-05-08 at 10 20 08

@simurai simurai added the dark label May 8, 2020
@minimaxir
Copy link

minimaxir commented May 8, 2020

tbh, inversely the string color is a bit dark in Light mode too. I cannot see any contrast between normal text and strings either. (albeit that's the case with the GitHub UI in general; but more notable when using it as a primary IDE theme)

@mattaningram
Copy link

This is my first reaction to using this theme as well. Beautiful theme but the light strings makes HTML and CSS harder to read.

@posthardcode
Copy link

I've been experimenting and it's trickier to make them darker if you want to stick to Primer (as other scopes are using darker hues of blue). An alternative would be #ffab70 which doesn't seem to clash with other colors. However, it introduces a new color to the theme palette, which might be undesired...

@samuelgozi
Copy link

Agree. That it the first thing I noticed.
Hurts the eyes... I think that if needed we should deviate from Primers colours in this case.
Maybe use a darker shade of the current color?

@balazs4
Copy link

balazs4 commented May 11, 2020

I am not a designer, but #fff5b1 $yellow-200 looks good to me:

image

Reference image (without patch)
image

/// really nice theme btw 👏

@estevanmaito
Copy link

It gets even worse with template literals, as the variables look almost the same as the surrounding string.

GitHub Dark
github-dark

Dracula
dracula-dark

@simurai
Copy link
Contributor Author

simurai commented May 12, 2020

tbh, inversely the string color is a bit dark in Light mode too.

Yeah, it's a tricky question. How much should this "GitHub theme" try to mimic the syntax highlighting on github.com? Below an example and a screenshot from VS Code. It doesn't have to be 100% the same, but would still be nice to stay somewhat close. Not that github.com's syntax highlighting is perfect, but people should feel familiar. 😬

const darkTheme = getTheme({
style: "dark",
name: "GitHub Dark",
});

CleanShot 2020-05-12 at 18 53 00@2x

As for the dark version.. we might can be less strict since that doesn't exist for github.com.

I've been experimenting and it's trickier to make them darker if you want to stick to Primer (as other scopes are using darker hues of blue).

Yeah, blue[7] is still pretty light and blue[6] already gets used a lot elsewhere. Here a mix in between:

CleanShot 2020-05-12 at 18 51 50@2x

Maybe as quick fix before considering a bigger change.

@simurai simurai mentioned this issue May 12, 2020
@loreina
Copy link

loreina commented May 12, 2020

I feel a lighter blue is too close to $blue-300 #79b8ff and can be hard to read in the same line:

image

I'm using $yellow-200 #fff5b1 as @balazs4 suggested as a temporary fix:

image

@paramaggarwal
Copy link
Contributor

As a user, I come to use this theme because I want to feel at home when switching between GitHub web and VSCode. I think we should honour this expectation and try to stay close to the GitHub web theme.

@simurai
Copy link
Contributor Author

simurai commented May 13, 2020

I'm using $yellow-200 #fff5b1 as @balazs4 suggested as a temporary fix:

👍 If anyone else wants to test it out, here the config you can add to your settings.json:

  "editor.tokenColorCustomizations": {
    "[GitHub Dark]": {
      "strings": "#fff5b1"
    }
  },

image

@minimaxir
Copy link

The corresponding TextMate rule will have to be added too:

"[GitHub Dark]": {
      "strings": "#fff5b1",
      "textMateRules": [
        {
          "scope": "punctuation.definition.string",
          "settings": {
            "foreground": "#fff5b1"
          }
        }
      ]
    }

I've been testing that out and it works fine.

@MrSunshyne
Copy link

As a user, I come to use this theme because I want to feel at home when switching between GitHub web and VSCode. I think we should honour this expectation and try to stay close to the GitHub web theme.

Not at the expense of readability

@paramaggarwal
Copy link
Contributor

@MrSunshyne Your claim is that readability is lacking on GitHub website? Because my point was about sticking to the GitHub website design as much as possible.

@estevanmaito
Copy link

@MrSunshyne Your claim is that readability is lacking on GitHub website? Because my point was about sticking to the GitHub website design as much as possible.

I think you're missing the point of this issue. GitHub uses a light theme, and as pointed earlier (#19 (comment)) there is no intent on changing it.

We're trying to find a way to improve readability for the Dark Theme, and with this particular theme, there is more room for changes, as GitHub doesn't have a dark mode, so there is no standard for it.

@balazs4
Copy link

balazs4 commented May 18, 2020

We're trying to find a way to improve readability for the Dark Theme, and with this particular theme, there is more room for changes, as GitHub doesn't have a dark mode, so there is no standard for it.

And how about the Github Mobile (iOS) dark mode?

image

Same color palette (?), but they are slightly different assigned to the tokens.

I personally find the mobile version somehow cleaner. 🙄

What do you think?

@estevanmaito
Copy link

You're right, mobile apps (Android uses the same theme as iOS) already have a dark theme, and the color palette seems the same, but better applied.

@simurai
Copy link
Contributor Author

simurai commented May 19, 2020

And how about the Github Mobile (iOS) dark mode?

Yeah, that's a great idea. Let's see if we can 🔦 🔍 spy on their dark theme and steal a few hex values.

@this-fifo
Copy link
Contributor

There's also a dark theme available for the GitHub Desktop app

@CatsMiaow
Copy link

  • VS Code v1.45.1
  • GitHub Theme v1.1.2 dark

Is there any reason why the color of the const variable is similar to the color of the text?
image

@MrSunshyne
Copy link

@MrSunshyne Your claim is that readability is lacking on GitHub website? Because my point was about sticking to the GitHub website design as much as possible.

@paramaggarwal Yep that is my claim. I would 100% prefer loading a project in vscode/webstorm darkmode before doing any serious code review. I didn't even know I was doing that until I thought about it after reading your question. I can glance at the code on github.com, but not for long. Also, to be clear, this is my opinion and I understand thousands of people do code review on github since years and it's working perfectly fine for them.

@estevanmaito since there is no standard for it, it's probably makes inserting new variations(removing while text) more acceptable?

@estevanmaito
Copy link

@MrSunshyne actually we found that there is already a Dark Mode for GitHub, mainly used on mobile, and it actually looks good #19 (comment)

It would make sense using it.

@icebeam030
Copy link
Contributor

icebeam030 commented May 29, 2020

This might be irrelevant to this issue but I feel using #fff as white in general is too bright in a dark theme and kind of hurts my eyes. I am not sure if the iOS app uses #fff as white but we can test that out.

@simurai simurai added the type: syntax Syntax highlighting label Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dark type: syntax Syntax highlighting
Projects
None yet
Development

No branches or pull requests