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

Caption button animations use wrong color #7314

Closed
OuSimeng opened this issue Aug 17, 2020 · 10 comments · Fixed by #8649
Closed

Caption button animations use wrong color #7314

OuSimeng opened this issue Aug 17, 2020 · 10 comments · Fixed by #8649
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) 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 v2.0

Comments

@OuSimeng
Copy link

For titlebar of terminal:

image
image

When the hover effect is about to end, the icon turns black, but it turns to white when it done

I'm not sure you can understand my bad English clearly
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 17, 2020
@DHowett
Copy link
Member

DHowett commented Aug 17, 2020

Are you using "theme":"dark" even though you are on a Light-theme OS?

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 17, 2020
@OuSimeng
Copy link
Author

image

maybe?

There is no problem in light theme
image
image

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Aug 17, 2020
@OuSimeng
Copy link
Author

I can't understand why it needs to "turn black" in dark theme when hover

@DHowett
Copy link
Member

DHowett commented Aug 17, 2020

It is a bug. Thank you for the info.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. 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 Aug 17, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 17, 2020
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Aug 17, 2020
@zadjii-msft zadjii-msft removed Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 17, 2020
@zadjii-msft zadjii-msft changed the title A Problem about ui(I'm not sure it is a bug) Caption button animations use wrong color Aug 17, 2020
@AnuthaDev
Copy link
Contributor

@DHowett Can I get some pointers on where to start with this🙏

@skyline75489
Copy link
Collaborator

@AnuthaDev The caption control are defined in MinMaxCloseControl.xaml and related. You may found it useful.

@AnuthaDev
Copy link
Contributor

@AnuthaDev The caption control are defined in MinMaxCloseControl.xaml and related. You may found it useful.

Thanks @skyline75489 , but I already know that😅.

Unfortunately I won't be able to contribute for ~2 months (exams!) , so feel free to work on it😊

@skyline75489
Copy link
Collaborator

You're always welcome to contribute . Good luck with the exam 😄 . By the way Dustin is the leader of this project so he's kinda busier. Feel free to reach me as I'm more of a community member.

@satken2 satken2 mentioned this issue Dec 24, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Dec 24, 2020
@ghost ghost closed this as completed in #8649 Jan 7, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jan 7, 2021
ghost pushed a commit that referenced this issue Jan 7, 2021
@satken2
Fix color animation bug of title bar buttons (#8649)
713027b 
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
DHowett pushed a commit that referenced this issue Jan 25, 2021
@satken2 @DHowett
Fix color animation bug of title bar buttons (#8649)
677ab36 
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

🎉This issue was addressed in #8649, which has now been successfully released as Windows Terminal v1.5.10271.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8649, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.:tada:

Handy links:

mpela81 pushed a commit to mpela81/terminal that referenced this issue Jan 28, 2021
@satken2 @mpela81
Fix color animation bug of title bar buttons (microsoft#8649)
0a0e751 
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 issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) 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 v2.0
Development

Successfully merging a pull request may close this issue.

Fix color animation bug of title bar buttons satken2/terminal
5 participants
@DHowett @skyline75489 @zadjii-msft @AnuthaDev @OuSimeng