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.

Sign up for GitHub

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

Jump to bottom

Add a setting to control selection background color #3326

Closed
zadjii-msft opened this issue Oct 25, 2019 · 3 comments · Fixed by #3471
Closed

Add a setting to control selection background color #3326

zadjii-msft opened this issue Oct 25, 2019 · 3 comments · Fixed by #3471
Assignees
@leonMSFT
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal-1911

Comments

@zadjii-msft
Copy link
Member

From @cayos in #3322:

It'd be amazing if you guys could implement the few remaining color customization options from VSCode's terminal to reach feature parity. They are:

  • terminal.selectionBackground: The selection background color of the terminal.
@zadjii-msft zadjii-msft added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Oct 25, 2019
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Oct 25, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 25, 2019
@zadjii-msft zadjii-msft mentioned this issue Oct 25, 2019
Closed
@zadjii-msft
Copy link
Member Author

From @javierdlg in #3061:

We're actually very interested in this since we have found a few contrast ratio issues with our colors and the terminal. We've also received some feedback since the terminal is the only tool that has different highlighting colors in VS:
image

Any ideas on the cost for this feature request?

I honestly don't know what that would cost. Adding a simple color setting to the settings can be done in a day, but connecting that to the renderer might prove trickier. I don't really know enough about how the DX renderer works, so it's harder for me to cost what changing that might take. Maybe @miniksa could add his thoughts?

I think it is a lot trickier to provide an "invert selection" mode (like conhost), but setting a single arbitrary color for selection might be a lot simpler.

@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Help Wanted We encourage anyone to jump in on these. labels Oct 29, 2019
@zadjii-msft zadjii-msft mentioned this issue Oct 29, 2019
Open
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 29, 2019
@rub8n
Copy link

rub8n commented Oct 30, 2019

Thanks for looking into this guys! For additional context, this is important for us because of the accessibility implications.

For example, if you see the image below, you can see how the colors don't really meet contrast ratios:
image

@leonMSFT leonMSFT mentioned this issue Nov 7, 2019
Merged
4 tasks
@ghost ghost added the In-PR This issue has a related PR label Nov 7, 2019
@ghost ghost removed the Help Wanted We encourage anyone to jump in on these. label Nov 11, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements and removed In-PR This issue has a related PR labels Nov 13, 2019
zadjii-msft pushed a commit that referenced this issue Nov 13, 2019
@leonMSFT @zadjii-msft
Add Selection Background Color as a setting to Profiles and Col… (#3471)
a404778 
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
This introduces a setting to both Profiles and ColorSchemes called <code>selectionBackground</code> that allows you to change the selection background color to what's specified. If <code>selectionBackground</code> isn't set in either the profile or color scheme, it'll default to what it was before - white.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #3326
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [x] Requires documentation to be updated

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
- Added selectionBackground to existing profile and colorscheme tests.
- Verified that the color does change to what I expect it to be when I add "selectionBackground" to either/both a profile and a color scheme.


<hr>

* adding selectionBackground to ColorScheme and TerminalSettings

* Changing PaintSelection inside the renderers to take a SelectionBackground COLORREF

* changes to conhost and terminal renderdata, and to terminal settings and core

* IT WORKS

* modification of unit tests, json schemas, reordering of functions

* more movement

* changed a couple of unit tests to add selectionBackground, added the setting to schemas, also added the optional setting to profiles

* default selection background should be slightly offwhite like the default foreground is

* reverting changes to .sln

* cleaning up

* adding comment

* oops

* added clangformat to my vs hehe

* moving selectionBackground to IControlSettings and removing from ICoreSettings

* trying to figure out why the WHOLE FILE LOOKS LIKE ITS CHANGED

* here it goes again

* pls

* adding default foreground as the default for selection background in dx
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Nov 13, 2019
@zadjii-msft zadjii-msft mentioned this issue Nov 14, 2019
Open
@ghost
Copy link

ghost commented Nov 26, 2019

🎉This issue was addressed in #3471, which has now been successfully released as Windows Terminal Preview v0.7.3291.0.:tada:

Handy links:

@austin-beer austin-beer mentioned this issue Jan 6, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@leonMSFT leonMSFT

Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Settings Issues related to settings and customizability, for console or terminal Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal-1911
Development

Successfully merging a pull request may close this issue.

Add Selection Background Color as a setting to Profiles and Col… microsoft/terminal
4 participants
@rub8n @DHowett-MSFT @zadjii-msft @leonMSFT