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

Expose selection background and alpha through the WPF control #7339

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

javierdlg
Copy link
Member

Summary of the Pull Request

Adds the ability to set the selection background opacity when setting the selection background. This also exposes the selection background and alpha through the terminal WPF container.

Validation Steps Performed

Patched VS and ran the terminal tool window with multiple selection background colors and alphas.

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

those are definitely nits so feel free to ignore them ☺️

src/renderer/dx/DxRenderer.cpp Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.hpp Outdated Show resolved Hide resolved
Comment on lines +72 to +77
public uint DefaultSelectionBackground;

/// <summary>
/// The opacity alpha for the selection color of the terminal, must be between 1.0 and 0.0.
/// </summary>
public float SelectionBackgroundAlpha;
Copy link
Member

Choose a reason for hiding this comment

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

this isn't going to be an issue for VS, but... in the future, there will be more consumers for these things. How should we ensure that new fields don't break them?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good question to ask. I'm not sure what the best plan for this might be, and I don't want to maintain different versions of this API for every update we push.

What is the design patter the rest of the Windows Console follows for API breaking changes?

Copy link
Member

Choose a reason for hiding this comment

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

Our solution is don't touch the Console API. I believe we've started using a few unused bits in other flags in existing APIs, but we don't touch the console API.

@DHowett DHowett changed the title Expose selection background and alpha through the WPF terminal container. Expose selection background and alpha through the WPF terminal container Aug 18, 2020
@DHowett DHowett changed the title Expose selection background and alpha through the WPF terminal container Expose selection background and alpha through the WPF control Aug 18, 2020
@DHowett DHowett merged commit 20b7fe4 into master Aug 18, 2020
@DHowett DHowett deleted the dev/jadelaga/WPFTheming branch August 18, 2020 23:11
DHowett pushed a commit that referenced this pull request Aug 24, 2020
Adds the ability to set the selection background opacity when setting the
selection background. This also exposes the selection background and alpha
through the terminal WPF container.

(cherry picked from commit 20b7fe4)
@javierdlg
Copy link
Member Author

@DHowett while we get out own WPF Signed package pipeline set up, could we get a signed package of the terminal WPF control from the main branch that includes this change?
Anything at or above this commit is fine.

@javierdlg
Copy link
Member Author

Or if you can point me to the pipeline that builds and signs this package I can reference that :)

@DHowett
Copy link
Member

DHowett commented Aug 25, 2020

I've got good news on that front 😄

We're about to move 1.2 to stable, and I published an update to the wpf control in our TerminalDependencies feed with this change cherry-picked into it.

Formal release announcement e-mail is coming later, but: 1.2 brings a lot of new VT support and some better color fidelity when coupled with a 1.2+ openconsole

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.

4 participants