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

Reduce alpha value of patch background colors #159

Conversation

bluebartle
Copy link
Contributor

@bluebartle bluebartle commented Jun 20, 2020

Adjust the alpha of the background for diff inserted and modified blocks down to 50 to improve contrast with foreground (especially comments), and adjust diff deleted down to 80 as a minor improvement of the same.

Addresses issue: #103

…cks down to 50 to improve contrast with foreground (especially comments), and adjust diff deleted down to 80 as a minor improvement of the same
@bluebartle
Copy link
Contributor Author

bluebartle commented Jun 20, 2020

I've finally got around to having a play with this. I'm perfectly happy to just change the diff_inserted background to #81a1c133 as per the vscode PR here: nordtheme/visual-studio-code#105, but thought I would have a quick play with the alpha of the existing current colours first.

I set the alpha for diff_inserted and diff_modified (which also has issues with contrast) down to 50, and just for good measure set the alpha for diff_deleted down to 80. The result seems much more legible to me:

Before After
diff_with_100_alpha diff_with_reduced_alpha

What do you reckon?

@bluebartle
Copy link
Contributor Author

This doesn't seem to work on mac, for some reason. rgba values don't seem to be parsed and the colours default to black. I guess this is known, and possibly why alpha wasn't just tweaked in the first place.

@nmaves
Copy link

nmaves commented Jun 22, 2020

I was just going to ask how you altered the alpha as I don't have that option on a Mac

@bluebartle
Copy link
Contributor Author

The doc states that 6 digit rgb and 8 digit rgba codes are supported, and the latter work for me on Linux. On Mac it doesn't. I haven't looked any deeper yet.

@bluebartle
Copy link
Contributor Author

bluebartle commented Jun 25, 2020

Another attempt... Support for RGBA when set in the XML is not consistent, and alpha isn't available via the theme editor in settings, so I've used a picker to pick out the colours with the alpha changes as rgb values and produced the following:

Conflict Deleted Inserted Modified
nord9 @50% #3d4c61 nord11 @50% #5b424d nord14 @33% #454f4e nord12 @33% #53524e
Before After
diff_with_100_alpha Screenshot 2020-06-25 at 21 13 13

@arcticicestudio
Copy link
Contributor

arcticicestudio commented Jul 2, 2020

Hi @ratskinmahoney 👋, thanks for your contribution 👍
I also forgot about the fact that, for whatever reason, the JetBrains IDE rendering engine doesn't play nice with alpha values at all so an exact port of nordtheme/visual-studio-code#105 is not possible, but your solution of simply using darker shades would have been my approach too 😄

But I'd like to suggest that we should still go with nord9 instead of nord14 for additions.
Even though a greenish color is more familiar for most users the bluish color visually matches better for "visual edge cases" like the comment color.

When you compare both of screenshots you might notice that the one that uses nord9 is "less distracting" and comments are quite more readable. This is not only a visual effect, but is explained by the fact that the comment color and "transparent"nord9 have been calculated on the same base color so both have a nearly equal hue, saturation and brightness values:

  • comment (brightened nord3): 221° hue, 28% saturation and 54% brightness.
  • "transparent" nord9: 216° hue, 29% saturation and 36% brightness

The "transparent" nord14 color on the other side is derived from another base color and comes with a hue value of only 193°, only 14% saturation and 30% brightness. The low saturation in combination with it's hue value makes it look kind of musty when used with transparency and it "bites" with the more bluish comment color.

Using nord14 with "transparency"

Using nord9 with "transparency"

I know that the comment color doesn't really matches when used with "transparent" nord11 for deletions, but using something different that a red-based color for this is out of the question 😄

Let me know that you think about this change.

@bluebartle
Copy link
Contributor Author

bluebartle commented Jul 2, 2020

@arcticicestudio - I've no complaint with that. I should find time tomorrow to make the change.

Cheers!

@bluebartle
Copy link
Contributor Author

@arcticicestudio - a day late, but here it is...

I've used nord9 for insertion. Based on 33 alpha this comes out as #3e4a59. I've also changed the error stripe colour to nord9 to match (let me know if you'd prefer that to remain as nord14). I've tweaked a couple of the other colours very slightly to make sure there's a consistent "use the nord colour as though it had 33 alpha" thing going on.

This is the result for the diff I was using previously:
diff_with_nord9

And here's what it looks like in the settings preview:
diff_settings_with_nord9

The only potential issue is that the colour used for conflict is based on nord10, which is obviously quite close to nord9 (see lines 4&5 on the left of the settings preview, vs line 7). I personally don't have a problem with this. I can tell the difference when I see both together, and context is clear anyway. I actually like it like this. If you think this will be a problem though, let me know.

Now that I've been looking at it a while. I definitely agree that nord9 is a better choice than nord14

@arcticicestudio
Copy link
Contributor

arcticicestudio commented Jul 5, 2020

Please don't stress yourself, after all this is open source not paid work with deadlines 😄
Nice to see you also favor nord9.

The overall design looks much better in my opinion now, but I'd like to suggest some small adjustments to the colors to make them more precise. You said you've used a color picker to get the actual HEX values, but this can be different based on your monitor, color profile and OS settings. Unfortunately colors are quite more complex when you dive deeper into the topic, e.g. the following methods of the screenshot will produce different colors when picked in the IDE.

To solve this we can calculate the colors to get exact and predictable colors, at least for usage in other ports if required as well as in languages like CSS.
Sass comes with the color module that provides the scale function that allows to “fluidly scale one or more properties of a color“ like e.g. alpha or saturation.

@use "sass:color";

$nord0: #2e3440;
$nord9: #81a1c1;
$nord11: #bf616a;
$nord13: #ebcb8b;

@function rgba-to-rgb($rgba, $background: $nord0) {
  @return mix(
    rgb(red($rgba), green($rgba), blue($rgba)),
    $background,
    alpha($rgba) * 100%
  );
}

:root {
  --diff-added-reduced-alpha: #{rgba-to-rgb(color.scale($nord9, $alpha: -80%))};
  --diff-deleted-reduced-alpha: #{rgba-to-rgb(color.scale($nord11, $alpha: -80%))};
  --diff-modified-reduced-alpha: #{rgba-to-rgb(color.scale($nord13, $alpha: -80%))};
}

Compiling this Sass code using the official Dart reference implementation via the CLI results in the following colors:

:root {
  --diff-added-reduced-alpha: #3f4a5a;
  --diff-deleted-reduced-alpha: #4b3d48;
  --diff-modified-reduced-alpha: #54524f;
}

As you can see the color for deleted lines already matched perfectly with your color, but for added and modified lines the values a slightly different. Even though the changes are minimal, this should be changed to make this change consistent with other themes.

@bluebartle
Copy link
Contributor Author

@arcticicestudio - That is not something I would have considered, though it makes perfect sense. Colours have been corrected as suggested. I used the converter to check the value for nord10 (for conflict) as well, and the value currently in the commit was correct.

Copy link
Contributor

@arcticicestudio arcticicestudio left a comment

Choose a reason for hiding this comment

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

Alright, let's get this merged :D
Thanks again for this PR, this will make many users happier.

@arcticicestudio arcticicestudio changed the title Adjust the alpha of the background for diff inserted and modified blo… Reduce alpha value of patch background colors Jul 9, 2020
@arcticicestudio arcticicestudio merged commit f452956 into nordtheme:develop Jul 9, 2020
@bluebartle
Copy link
Contributor Author

@arcticicestudio - great! Thanks very much for your help. It's a pleasure to contribute.

arcticicestudio pushed a commit to nordtheme/alacritty that referenced this pull request Aug 25, 2020
The colors have been calculated using the Sass `scale` function [1] that allows to "fluidly scale one or more properties of a color" like e.g. alpha or saturation. Also see nordtheme/jetbrains#159 [2] for more details.

```scss
@use "sass:color";

$nord0: #2e3440;
$foreground: #d8dee9;

$black: #3b4252;
$red: #bf616a;
$green: #a3be8c;
$yellow: #ebcb8b;
$blue: #81a1c1;
$magenta: #b48ead;
$cyan: #88c0d0;
$white: #e5e9f0;

@function dim($color) {
  @return rgba-to-rgb(color.scale($color, $alpha: -30%));
}

@function rgba-to-rgb($rgba, $background: $nord0) {
  @return mix(
    rgb(red($rgba), green($rgba), blue($rgba)),
    $background,
    alpha($rgba) * 100%
  );
}

:root {
  --foreground: #{dim($foreground)};
  --black: #{dim($black)};
  --red: #{dim($red)};
  --green: #{dim($green)};
  --yellow: #{dim($yellow)};
  --blue: #{dim($blue)};
  --magenta: #{dim($magenta)};
  --cyan: #{dim($cyan)};
  --white: #{dim($white)};
}
```

[1]: https://sass-lang.com/documentation/modules/color#scale
[2]: nordtheme/jetbrains#159 (comment)

Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com>

Closes GH-12
arcticicestudio added a commit that referenced this pull request Sep 22, 2020
Adjusted the background colors for patches by reducing their alpha value
by 80%. The resulting RGBA color was converted precisely into the
respective RGB color (HEX format) since JetBrains rendering engine still
has problems when using the 8-digit HEX format that includes the value
for the alpha layer.

The values have been calculated to get exact and predictable colors,
at least for usage in other ports if required as well as in languages
like CSS.
Sass [1] comes with the color module that provides the scale
function [2] that allows to "fluidly scale one or more properties of a
color" like e.g. alpha or saturation:

```scss
@use "sass:color";

$nord0: #2e3440;
$nord9: #81a1c1;
$nord11: #bf616a;
$nord13: #ebcb8b;

@function rgba-to-rgb($rgba, $background: $nord0) {
  @return mix(
    rgb(red($rgba), green($rgba), blue($rgba)),
    $background,
    alpha($rgba) * 100%
  );
}

:root {
  --diff-added-reduced-alpha: #{rgba-to-rgb(color.scale($nord9, $alpha: -80%))};
  --diff-deleted-reduced-alpha: #{rgba-to-rgb(color.scale($nord11, $alpha: -80%))};
  --diff-modified-reduced-alpha: #{rgba-to-rgb(color.scale($nord13, $alpha: -80%))};
}
```

Compiling the Sass code above using the official Dart reference
implementation [3] via the CLI resulted in the following colors:

```css
:root {
  --diff-added-reduced-alpha: #3f4a5a;
  --diff-deleted-reduced-alpha: #4b3d48;
  --diff-modified-reduced-alpha: #54524f;
}
```

[1]: https://sass-lang.com 
[2]: https://sass-lang.com/documentation/modules/color#scale
[3]: https://github.com/sass/dart-sass

GH-159

Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com>
Co-authored-by: Sven Greb <development@svengreb.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poor contrast between 'added block' background and comments in comparison view
4 participants