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

Change user, host, and rowname colors to yellow for better visibility #97

Merged
merged 2 commits into from
Aug 28, 2021

Conversation

bilditup1
Copy link
Contributor

@bilditup1 bilditup1 commented Jul 25, 2021

Dark blue on black is kind of hard to read.

cmd-old-cropped-censored

If you're actually using the old powershell terminal with its default colors, it's a bit harder than that.

pwsh-old-cropped-censored

When you have something like f.lux on, it can be almost illegible if you forget to shut it off for your terminal app--and if you normally don't do this, why would you? (I tried to capture this with a screenshot but the colors are getting mangled right now).

Yellow on black and yellow on blue, though, both read easily.

cmd-new-for-real_cropped_censored

pwsh-new-cropped-censored

Seems like an easy quality-of-life win. (Screenshots censored to remove my user and host, but I have also changed the code so that their colors are also changed from blue to yellow.)

@rashil2000
Copy link
Member

Hmm, I remember coming across this readability issue when testing out with the default PowerShell color scheme. Seems fair to me.

Copy link
Member

@jcwillox jcwillox left a comment

Choose a reason for hiding this comment

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

The premise is good I'm just not sure about the color choice, I think we should go with cyan instead, this is the color neofetch uses and it also keeps with the blue theme of the windows logo. As for the legacy logo, we can either just go with the cyan for simplicity or switch to another color (a green/red might look nicer than the yellow).

image

image

@rashil2000
Copy link
Member

Cyan on a dark blue background seems like unreadable. Yellow has the maximum contrast on that background (that's why PSReadline uses yellow by default).
Also, I didn't get what you were trying to show in the screenshots, the first one seems to have no distinction between info title and the info itself.

@jcwillox
Copy link
Member

jcwillox commented Aug 2, 2021

Cyan on a dark blue background seems like unreadable. Yellow has the maximum contrast on that background (that's why PSReadline uses yellow by default).

I mean look at the image I sent or try it on your own machine the bright cyan is pretty readable and keeps with the windows blue theme.

Also, I didn't get what you were trying to show in the screenshots, the first one seems to have no distinction between info title and the info itself.

The screenshots were just examples of alternatives, they are the color schemes that neofetch uses by default. The difference between the title and info is irrelevant.

@rashil2000
Copy link
Member

image

See for yourself, which has the best contrast (blue, yellow, cyan in order).

The difference between the title and info is irrelevant.

Wait, how? What's the point of differentiating the colour of Resolution: and 1920x1080 then?

@jcwillox
Copy link
Member

jcwillox commented Aug 2, 2021

That's not the colour I used, I said bright cyan, on the colour bar it's the 2nd from the right on the bottom row.

Wait, how? What's the point of differentiating the colour of Resolution: and 1920x1080 then?

it was "irrelevant" to the point I was making, we don't current use a different colour for the title (i.e. user@host) and info titles and I wasn't talking about changing that.

@rashil2000
Copy link
Member

Ok, here are the bright variants (in same order):
image

Here too the yellow looks much better (and is also markedly different from the text that follows the info-title, compared to cyan)

@rashil2000
Copy link
Member

@jcwillox this is awaiting your approval.

@jcwillox
Copy link
Member

Thanks, forgot about this PR 👍

On a related note, we may want to consider adding some sort of text colour option in the future

@jcwillox jcwillox merged commit 0b5c91f into lptstr:master Aug 28, 2021
@rashil2000
Copy link
Member

That's a nice feature suggestion!

@iliyan61
Copy link

is there any way to change the colour of the text? as i dont much like the yellow.

@rashil2000
Copy link
Member

Not yet

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

4 participants