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

Fix color animation bug of title bar buttons #8649

Merged
1 commit merged into from
Jan 7, 2021

Conversation

satken2
Copy link
Contributor

@satken2 satken2 commented Dec 24, 2020

In dark mode (and high contrast mode), color animation of title bar
button uses wrong color. Cause of this issue is using invalid data in
ColorAnimation. I fixed this bug by changing ColorAnimation value
in XAML layout file.

According to a forum post, To value of ColorAnimation must be
frozen. But original source code uses "color binding" which makes this
value dynamic. As a result, the value set by default is always used,
that means, light mode.

So I added new resource named CaptionButtonStrokeColor and
CaptionButtonBackgroundColor which has static color value.

In light mode and dark mode, I set SystemBaseHighColor in the color
resource. SystemBaseHighColor is the same as
SystemControlForegroundBaseHighBrush.Color which is originally used in
animation.

The background color animation happened to work correctly because its
value is the same between light mode and dark mode. But I also fixed
background color animation.

Validation Steps Performed

There is no need to add new test with this fix.

  • I changed the theme value in settings.json and confirmed that the
    correct color values were used.
  • I confirmed that it works correctly even if the Windows theme is
    changed.

Closes #7314

@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. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Dec 24, 2020
@satken2 satken2 marked this pull request as ready for review December 24, 2020 15:16
@satken2 satken2 changed the title Fixed color animation bug of title bar buttons Fix color animation bug of title bar buttons Dec 26, 2020
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

This is excellent work. Thank you for figuring it out!

@zadjii-msft zadjii-msft self-assigned this Jan 4, 2021
@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 7, 2021
@ghost
Copy link

ghost commented Jan 7, 2021

Hello @DHowett!

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 713027b into microsoft:main Jan 7, 2021
DHowett pushed a commit that referenced this pull request Jan 25, 2021
In dark mode (and high contrast mode), color animation of title bar
button uses wrong color.  Cause of this issue is using invalid data in
`ColorAnimation`.  I fixed this bug by changing `ColorAnimation` value
in XAML layout file.

According to a [forum post], `To` value of `ColorAnimation` must be
frozen. But original source code uses "color binding" which makes this
value dynamic.  As a result, the value set by default is always used,
that means, light mode.

So I added new resource named `CaptionButtonStrokeColor` and
`CaptionButtonBackgroundColor` which has static color value.

In light mode and dark mode, I set `SystemBaseHighColor` in the color
resource.  `SystemBaseHighColor` is the same as
`SystemControlForegroundBaseHighBrush.Color` which is originally used in
animation.

The background color animation happened to work correctly because its
value is the same between light mode and dark mode.  But I also fixed
background color animation.

## Validation Steps Performed

There is no need to add new test with this fix.

- I changed the `theme` value in `settings.json` and confirmed that the
  correct color values were used.
- I confirmed that it works correctly even if the Windows theme is
  changed.

[forum post]: https://social.msdn.microsoft.com/Forums/vstudio/en-US/027c364f-5d75-424f-aafd-7fb76b10b676/templatebinding-on-storyboard?forum=wpf

Closes #7314

(cherry picked from commit 713027b)
@ghost
Copy link

ghost commented Jan 28, 2021

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

Handy links:

@ghost
Copy link

ghost commented Jan 28, 2021

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

Handy links:

mpela81 pushed a commit to mpela81/terminal that referenced this pull request Jan 28, 2021
In dark mode (and high contrast mode), color animation of title bar
button uses wrong color.  Cause of this issue is using invalid data in
`ColorAnimation`.  I fixed this bug by changing `ColorAnimation` value
in XAML layout file.

According to a [forum post], `To` value of `ColorAnimation` must be
frozen. But original source code uses "color binding" which makes this
value dynamic.  As a result, the value set by default is always used,
that means, light mode.

So I added new resource named `CaptionButtonStrokeColor` and
`CaptionButtonBackgroundColor` which has static color value.

In light mode and dark mode, I set `SystemBaseHighColor` in the color
resource.  `SystemBaseHighColor` is the same as
`SystemControlForegroundBaseHighBrush.Color` which is originally used in
animation.

The background color animation happened to work correctly because its
value is the same between light mode and dark mode.  But I also fixed
background color animation.

## Validation Steps Performed

There is no need to add new test with this fix.

- I changed the `theme` value in `settings.json` and confirmed that the
  correct color values were used.
- I confirmed that it works correctly even if the Windows theme is
  changed.

[forum post]: https://social.msdn.microsoft.com/Forums/vstudio/en-US/027c364f-5d75-424f-aafd-7fb76b10b676/templatebinding-on-storyboard?forum=wpf

Closes microsoft#7314
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. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caption button animations use wrong color
3 participants