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

#1363 - Campbell color scheme should change the foreground color from #F2F2F2 to #CCCCCC #1629

Conversation

RyanBee-MSFT
Copy link
Contributor

@RyanBee-MSFT RyanBee-MSFT commented Jun 26, 2019

Changed the foreground color from #F2F2F2 to #CCCCCC.

Summary of the Pull Request

I changed where foreground is set in CascadiaSettings.cpp, from #F2F2F2 : RGB(242, 242, 242) to #CCCCCC : RGB(204, 204, 204).

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

As detailed in the bug, this slightly reduces the brightness of the default/foreground color of the Campbell color scheme from BrightWhite to White. This then allows the use of ESC sequence codes to add an intensity bit to make White display as BrightWhite. Without this change, using ESC[1m would keep the display color the same.

Validation Steps Performed

This was tested by manipulating the profiles.json to find the correct values to use. The code change was not tested outside this commit because it is a direct replacement of hard coded data, so there are no behavioral changes which should need to be evaluated separately.

Changed the foreground color from #F2F2F2 to #CCCCCC.
@RyanBee-MSFT
Copy link
Contributor Author

Just leaving this for you @DHowett-MSFT. This is the change as discussed in the bug. You had it set to tag Help-Wanted, so I just put it together. The work is there if you want to take it.

@DHowett-MSFT
Copy link
Contributor

Thanks! If you’re up to it, you could do a whole color palette sweep and get solarized in there as well. What do you think? #1509

Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

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

I am good with subbing the one Campbell color per this PR at the time of review. If it expands to rev a whole bunch of other stuff, I'd love to re-review it then (or in a different PR).

Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

(If you want to do additional color schemes, we can handle that in a separate PR. Thanks for doing this.)

@DHowett-MSFT DHowett-MSFT merged commit f63cada into microsoft:master Jun 29, 2019
DHowett-MSFT pushed a commit that referenced this pull request Jul 2, 2019
Changed the foreground color from #F2F2F2 to #CCCCCC so that ESC[1m _actually_ brightens it.

(cherry picked from commit f63cada)
DHowett-MSFT pushed a commit that referenced this pull request Jul 2, 2019
Changed the foreground color from #F2F2F2 to #CCCCCC so that ESC[1m _actually_ brightens it.

(cherry picked from commit f63cada)
mcpiroman pushed a commit to mcpiroman/terminal that referenced this pull request Jul 2, 2019
Changed the foreground color from #F2F2F2 to #CCCCCC so that ESC[1m _actually_ brightens it.
@ghost
Copy link

ghost commented Aug 3, 2019

🎉Windows Terminal Preview v0.3.2142.0 has been released which incorporates this pull request.:tada:

Handy links:

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.

Campbell color scheme should change the foreground color from #F2F2F2 to #CCCCCC
3 participants