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

Make sure focused tab text color accounts for alpha #14643

Merged
1 commit merged into from
Jan 16, 2023

Conversation

zadjii-msft
Copy link
Member

Basically what it says on the tin. For transparent tabs, we should layer on to the tab row to evaluate what the actual color of the tab will be. We did this for deselected tabs, but not for selected ones.

Gif below.

Closes #14561

@ghost ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Jan 6, 2023
@zadjii-msft
Copy link
Member Author

gh-14561-done

@zadjii-msft zadjii-msft added this to the Terminal v1.17 milestone Jan 10, 2023
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Simple enough. Thanks!

@carlos-zamora carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Jan 10, 2023
@ghost ghost requested review from PankajBhojwani, DHowett and lhecker January 10, 2023 22:49
@@ -388,6 +379,25 @@ namespace winrt::TerminalApp::implementation
subtleFillColorTertiaryBrush.Color(subtleFillColorTertiary);
}

// The tab font should be based on the evaluated appearance of the tab color layered on tab row.
const auto layeredTabColor = til::color{ color }.layer_over(_tabRowColor);
Copy link
Member

Choose a reason for hiding this comment

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

What does this do if the _tabRowColor is fully transparent?

Copy link
Member Author

Choose a reason for hiding this comment

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

oooooh good question

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd guess that it would treat it as black, cause alpha=0 * (r,g,b) => (0,0,0)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, we treat it as 0,0,0 and end up with white text.

@DHowett
Copy link
Member

DHowett commented Jan 11, 2023

Do you have an inkling as to whether this reproduces on 1.16?

@zadjii-msft
Copy link
Member Author

Do you have an inkling as to whether this reproduces on 1.16?

I'd bet it does

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 16, 2023
@ghost
Copy link

ghost commented Jan 16, 2023

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 4c7879b into main Jan 16, 2023
@ghost ghost deleted the dev/migrie/b/14561-invisible-tab-text-color branch January 16, 2023 20:04
DHowett pushed a commit that referenced this pull request Jan 20, 2023
Basically what it says on the tin. For transparent tabs, we should layer on to the tab row to evaluate what the actual color of the tab will be. We did this for deselected tabs, but not for selected ones.

Gif below.

Closes #14561

(cherry picked from commit 4c7879b)
Service-Card-Id: 87567529
Service-Version: 1.16
@ghost
Copy link

ghost commented Jan 24, 2023

🎉Windows Terminal v1.16.1023 (10231 and 10232) has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jan 24, 2023

🎉Windows Terminal Preview v1.17.1023 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.17] Tab text may be illegible if you switch to light theme
3 participants