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

Use dark-theme friendly colors for compiler messages #3013

Merged
merged 1 commit into from Dec 15, 2021

Conversation

techee
Copy link
Member

@techee techee commented Nov 20, 2021

When playing with the macOS bundle to look well both in light and dark themes, I noticed that in the dark theme colors in the message window are extremely hard to read with the default colors. While it may be hard to determine whether users use light or dark themes as discussed here

#2644

I think we should rather improve the default colors we use so they look reasonably well both in light and dark environments. I took the colors from the middle of the color spectrum here

https://uxplanet.org/create-an-easily-switchable-light-dark-styles-in-figma-ffee3cd542a7

and the result looks quite good IMO:
Screen Shot 2021-11-20 at 18 22 16

Screen Shot 2021-11-20 at 18 22 46

I changed just the geany-compiler-context and geany-compiler-message colors which are the worst dark-theme offenders but we could update all the colors so we use a single color palette. The palette I used is just an example which looks good IMO but we could use other palette if someone has some better idea.

What do you think?

@techee
Copy link
Member Author

techee commented Nov 20, 2021

For reference, here's the compiler window with the currently used default colors:
Screen Shot 2021-11-20 at 18 51 06
Screen Shot 2021-11-20 at 18 45 19

@elextr
Copy link
Member

elextr commented Nov 21, 2021

Good idea.

Blue is ok, not so fond of the baby poop brown .. sorry gamboge 😀 I use mid grey #7f7f7f which shows on my dark theme, and likely shows ok on a light theme [Edit: oh look github shows the colour for us, and shows fine on light].

@techee
Copy link
Member Author

techee commented Nov 21, 2021

I use mid grey #7f7f7f which shows on my dark theme, and likely shows ok on a light theme

I'm just worried there may be some grey themes out there. So maybe just pick some other color nobody hates...

Edit: oh look github shows the colour for us, and shows fine on light

Yeah, cool, didn't know that.

@eht16
Copy link
Member

eht16 commented Nov 21, 2021 via email

@elextr
Copy link
Member

elextr commented Nov 21, 2021

Well the good thing is that users can change any questionable choice the team makes 😄

No colour is perfect for every theme, the important thing is that they have a difference in saturation from the standard light or dark themes, but that of course makes them unsuitable for mid themes. 🤷

@techee
Copy link
Member Author

techee commented Nov 21, 2021

I share the concerns that grey foreground color could interfere with
grey themes. And I actually like the proposed "baby poop brown" :D.

Yeah, it's not baby poop, it's amber :-)

We could change the colors this way (had to try the color highlighting on github myself ;-):

  1. #0085ff (#0000d0 before): geany-compiler-message
  2. #00ba34 (#007f00 before): geany-document-status-readonly
  3. #f98600 (#7f0000 before): geany-compiler-context, geany-document-status-disk-changed
  4. #e92c2c (#ff0000 before): geany-compiler-error, geany-terminal-dirty, geany-document-status-changed

Some comments:
The biggest change are the colors for (1) and (3). The original green geany-document-status-readonly has slightly worse legibility in dark themes but it's not that tragic. The red is nearly identical. But I would change these colors too for more unified appearance.

The original color #ff7f00 for geany-document-status-disk-changed is nearly identical to the new infamous baby poop color #f98600 so we could use the new one.

I would keep the pink #ff6666 for geany-search-entry-no-match - I think it visually matches the new colors nicely and since it's a background color with white text, it has the same legibility in all themes (the selection color geany-search-entry-no-match selection #771111 is more or less in the "who cares" category and can stay this way too).

@elextr
Copy link
Member

elextr commented Nov 21, 2021

Colours 1 and 2 are fine (here).
Colour 3 Gamboge I have already commented on. Note that it depends on what the user brightness setting is for the monitor if it shows as yellow or brown or some orangy colour in between. I forsee much angst from users.
Colour 4 does not have sufficient contrast with the background on my Mint-Y-Dark theme, and as the background has a slight green tinge (well its a MINT theme after all) red is a bad contrast for the 10% of males that have reduced red/green colour perception.

@techee
Copy link
Member Author

techee commented Nov 21, 2021

I don't care about colors (2) and (4) much myself. As for (1) and (3), these are the ones I found most legible in both light and dark themes from the options I tried. It's quite possible there is some better color so feel free to suggest some.

@techee
Copy link
Member Author

techee commented Nov 21, 2021

Basically the only option for #f98600 I see is going more towards red so it would become orange which is OK too I think.

@elextr
Copy link
Member

elextr commented Nov 21, 2021

As for (1) and (3), these are the ones I found most legible in both light and dark themes from the options I tried.

I don't disagree that 3 is practical, I just do not like it. And therein is the problem with colours, two of us can't agree, whats going to happen with ALL the Geany users involved 😄 Agree more orange than brown might be better.

For 4 I use a similar colour but less saturated, its not pretty, its sort of strawberry milk shake, but visible and obviously reddish for error, but probably not suitable for a default colour.

@techee
Copy link
Member Author

techee commented Nov 21, 2021

@elextr What about this:

  1. #006eff
  2. #00a500
  3. #ff6f00

(1) is slightly darker - still legible in dark theme and better contrast in light theme
(2) I just made the current Geany color slightly brighter to improve contrast in dark theme
(3) Moved slightly towards red, hopefully outside the poopy area - could do it even more, it should just be easy to distinguish from red.

@techee
Copy link
Member Author

techee commented Nov 21, 2021

Or maybe something like #bf4b40 for (3).

@elextr
Copy link
Member

elextr commented Nov 22, 2021

The new 1 and 2 are still fine (here) but so were your previous ones, your choice.

For (3) #ff6f00 is sort of ok, #bf4b40 does not have much contrast to the grey background of textview, I will still override either with my mid grey #808080.

I found the Mint-Y-Dark theme gtk.css and it uses #404040 as the background for text view, so #bf4b40 is only a bit redder and a bit brighter so thats why it has little contrast. What does your dark theme use as background?

For (4) #e92c2c used as geany-document-status-changed I just noticed it is pretty hard to read against the tab background which is #404040 again, but then dimmed when not the current page (I think, my CSS is not brilliant and the themes gtk.css is complex, but think I found the right entry, and its an rgba so partly transparent, so not sure what the resultant colour value would be). I use #ff4040.

Note that as I am already overriding the existing colours, changing the defaults won't bother me, the current ones were unacceptable anyway. I am just trying to be in the place of a user who wants a reasonable experience out of the box and doesn't want to edit geany.css. Maybe in the end we have to merge something and I will send the complaints to you 😈

@techee
Copy link
Member Author

techee commented Nov 22, 2021

OK, I played with the colors for (3) a bit more - basically, we have 2 choices - either dim the red a little or go towards the orange color. The original color #7f0000 was basically the dimmed red and to avoid user shock from different colors, I would rather go this direction (the orange is a bit too flashy to my liking). As you mentioned, it might not be red enough to distinguish from grey so I added a little more red and the result is #cc4033 but I think I wouldn't add more red than that, it then becomes hard to distinguish from the error red.

So the colors would be:

  1. #006eff
  2. #009000 - after playing with it, made it a little darker than what I suggested before
  3. #cc4033
  4. #ff0000 - the original red (also used in the editor to underline errors so these should be the same)

The colors are a little on the darker side in dark themes but still usable and I don't want to make the colors much worse for light themes which I think more users still use so I prioritized contrast for light themes with the colors.

I guess it's impossible to find perfect colors, you can always find some backgrounds for which it doesn't work. Also, maybe I have a little too good monitor for this - but I tried with an external monitor which is worse than the one on the MacBook and these colors seem to be OK there too.

@elextr
Copy link
Member

elextr commented Nov 23, 2021

For me (1) and (2) are just rearranging the deckchairs on the titanic (unless you had problems with them since I guess your theme is different to Mint-Y-Dark, and you keep changing them each time I said they were fine 😁).

This value of (3) #cc4033 is legible and not Gamboge, so fine (oh no, I said it is fine, I guess you will change it again 😁).

The (4) #ff0000 was the value that caused the CSS options to be added to change it from being hard coded, because there were issues raised by people who could not see it on dark themes.

My colour vision is slightly off "normal", but I can still tell red from green well enough to be allowed to fly aeroplanes. There are others who are much worse. But even for me, #ff0000 made error messages and changed filenames on tabs basically impossible to read on a dark theme, it has weird artefacts making it illegible, thats why I merged the ability to change it with the comment.

So that is actually the most important one to change to give a useful out of the box experience. Therefore we havn't actually helped anybody leaving #ff0000 which is the original "bad" value 😞.

Since (3) and (4) have to be different, you can have your Gamboge #ff6f00 for (3) if I can have pinker red #ff3030 for (4). And @eht16 likes that (3) so it must be good 😁.

PS, it was a genius idea of yours to reduce it to four colours, imagine what it would have been like if we tried to agree the original seven colours.

@xiota
Copy link
Contributor

xiota commented Nov 23, 2021

Note, I normally use a light theme. According to this test, I have normal color vision.

  1. #006eff – This looks good to me.
  2. #309030 – A slightly brighter green might work better with dark themes.
  3. #ee8000 – I prefer a slightly darker orange.
  4. #ff3030 – Pink red is fine with me.

Based on checking with RGBlind browser plugin, an orange for 3 would be preferable to dark red because shades of red all become pretty much indistinguishable.

It's also difficult to get 2, 3, and 4 to all be different for both protanopia and deuteranopia. Based on the proposed uses, it seems more important to distinguish 4 from 2/3 than to distinguish 2 and 3 from each other Making 2 more cyan would help with this: #009075 (but might be aesthetically objectionable?).

@techee
Copy link
Member Author

techee commented Nov 23, 2021

@elextr @xiota For me the most important colors are (1) and (3).

We agree on (1).

For (3) I'm fine both with #ff6f00 and @xiota's #ee8000 (I personally prefer @xiota's orange as it is not as flashy as #ff6f00, not sure what @elextr thinks).

(2) for me is "whatever", @xiota's #309030 looks good to me too (don't like the cyan #009075 much myself).

#ff3030 for (4) sounds good to me too, we should just update the Scintilla error highlighting color to this one too so we don't have 2 different reds.

According to this test, I have normal color vision.

Since we all use different monitors, monitor brightness and different lighting conditions, I wouldn't take such tests too seriously. That said, I took the test and have normal color vision too.

@techee
Copy link
Member Author

techee commented Nov 23, 2021

PS, it was a genius idea of yours to reduce it to four colours, imagine what it would have been like if we tried to agree the original seven colours.

Actually I reduced it to 2 initially but still, it is 2^48 combinations to fight for ;-). And yeah, I expected a long discussion here. But seriously, in the next release nobody will notice Scintilla 5, updated ctags, meson build, etc., but everybody will see the color changes made here so better not to screw up much.

@elextr
Copy link
Member

elextr commented Nov 23, 2021

It's also difficult to get 2, 3, and 4 to all be different for both protanopea and deuteranopia.

Yes, thats why the colours need to be such that they will contrast to both dark and light backgrounds but not generate artefacts, so they are legible even if the colour can't be perceived. Unfortunately mid range themes will likely need adjustment, but thats why we have geany.css, we can never make everybody happy.

(1) agreed.

(2) happy to shuffle the deckchair to @xiota's #309030

(3) @xiota's suggestion of #ee8000 is ok for me.

(4) #ff3030 seems to be accepted. @techee I thought that the error squiggle used this CSS setting already? Looks like it to me but I didn't check the code.

Based on the proposed uses, it seems more important to distinguish 4 from 2/3 than to distinguish 2 and 3 from each other.

Correct.

(3) and (4) need to be distinguishable since they show in the same compiler message window, but the lines coloured (4) are clickable and those coloured (3) are not. For simple languages like C its pretty obvious which are clickable, but guess which ones are clickable in this single error message from the C++ file I have been using for testing of the compiler colours.

But seriously, in the next release nobody will notice Scintilla 5, updated ctags, meson build, etc., but everybody will see the color changes made here so better not to screw up much.

Sadly you are very much correct 😜

These colors seem to provide good legibility for both light and dark
themes and seem to be better defaults regardless of what theme users
use.
@techee
Copy link
Member Author

techee commented Nov 23, 2021

I've updated the commit with the new colors.

@techee I thought that the error squiggle used this CSS setting already? Looks like it to me but I didn't check the code.

That color is inside filetypes.common and is independent of the used theme so it should be also this theme-independent red. I updated it in the commit too.

@xiota
Copy link
Contributor

xiota commented Nov 23, 2021

@elextr Since you have a color-vision deficiency, is it any easier to tell #009075/#ff3030 apart (vs #309030/#ff3030) in editor tabs for read-only/changed documents? I suppose color-blind users can open an issue if there's a major problem. (This PR should come up in searches for various keywords.)

Otherwise, the colors look good to me.

@elextr
Copy link
Member

elextr commented Nov 24, 2021

is it any easier to tell #009075/#ff3030 apart (vs #309030/#ff3030) in editor tabs for read-only/changed documents?

As a single reference point within a range of colour perceptions I prefer the latter.

@xiota
Copy link
Contributor

xiota commented Nov 24, 2021

As a single reference point within a range of colour perceptions I prefer the latter.

One is better than none, and it's impossible to address everything. At the least, the most likely problems will have been considered, and it will have been deemed reasonable to direct people to the manual (Customizing Geany's appearance using GTK+ CSS).

@elextr
Copy link
Member

elextr commented Nov 24, 2021

Leave this PR for a week in case any others have an objection, then merge before any more changes 😁

@eht16
Copy link
Member

eht16 commented Nov 24, 2021

I'm much less experienced with colors and also cannot properly judge so detailed changes.
But I can say that I'm currently using the colors from this PR changes and they look very good on my setup (old Thinkpad display + dark grey GTK theme), being totally aware that this is just another random combination of display type, ambient light, theme, personal preferences, moonlight, etc. :D.

I'd also vote for merge!

@techee
Copy link
Member Author

techee commented Dec 15, 2021

OK, nobody seemed to be against it here so I merged it (so people who are against it notice it and become vocal ;-).

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.

None yet

5 participants