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

Close button doesn't use a tab's text color when hovered #8046

Closed
TylerEich opened this issue Oct 26, 2020 · 5 comments · Fixed by #8209
Closed

Close button doesn't use a tab's text color when hovered #8046

TylerEich opened this issue Oct 26, 2020 · 5 comments · Fixed by #8209
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

@TylerEich
Copy link

Environment

Windows build number: 10.0.18363.0
Windows Terminal version (if applicable): 1.3.2651.0

Any other software?
No.

Steps to reproduce

Open a tab. Color it yellow. Hover over its close button.

47103599-d504-44ba-bc36-a39aa98ef7c4

Expected behavior

The × should be black, like it was before I hovered.

Actual behavior

The × is white, like the default text color.

Related to #5789
@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 Oct 26, 2020
@zadjii-msft
Copy link
Member

Wow yep, that's a real bug. Probably similar to #5780, but for the tab hover color.

@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 Oct 27, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 27, 2020
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Oct 27, 2020
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 2, 2020
@dcheatha
Copy link
Contributor

I fixed the tab text coloring issue, however this fix seems to introduce another problem.

Light theme white tab mouse hover effect:
Screenshot_2020-11-07_152110_3

Dark theme white tab mouse hover effect:
Screenshot_2020-11-07_153021_2

Should I calculate and set the button shadow manually here, or is there some way to set the button theme?

@Don-Vito
Copy link
Contributor

@dcheatha - I think that the behavior you describe already exists prior to you fix. I checked this with vanilla, but please give it a try as well. (Hence I think your fix is already of high value 😊).

Regarding your question, I am not an author, but given that in the current implementation all brushes are computed manually, I guess the same should apply to the button shadow.

@dcheatha dcheatha mentioned this issue Nov 10, 2020
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Nov 10, 2020
@ghost ghost closed this as completed in #8209 Nov 18, 2020
@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 Nov 18, 2020
ghost pushed a commit that referenced this issue Nov 18, 2020
@dcheatha
Fix close button not using the tab's text color (#8046) (#8209)
2a34080 
<!-- 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 sets the tab close button color to match the tab text color.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> 
## References
#8046
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #8046  
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #8046 

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
This sets the tab close button color to match the tab text color.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Test light theme white tab mouse hover effect:
![Screenshot_2020-11-07_152110_3](https://user-images.githubusercontent.com/7143383/98640319-ec735e80-22de-11eb-8d35-08188405b566.png)

Test dark theme white tab mouse hover effect:
![Screenshot_2020-11-07_153021_2](https://user-images.githubusercontent.com/7143383/98640377-f006e580-22de-11eb-9bb5-dde9fe9b81b6.png)
@DHowett DHowett mentioned this issue Nov 23, 2020
Closed
DHowett pushed a commit that referenced this issue Jan 25, 2021
@dcheatha @DHowett
Fix close button not using the tab's text color (#8046) (#8209)
a388adf 
<!-- 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 sets the tab close button color to match the tab text color.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#8046
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #8046
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #8046

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
This sets the tab close button color to match the tab text color.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Test light theme white tab mouse hover effect:
![Screenshot_2020-11-07_152110_3](https://user-images.githubusercontent.com/7143383/98640319-ec735e80-22de-11eb-8d35-08188405b566.png)

Test dark theme white tab mouse hover effect:
![Screenshot_2020-11-07_153021_2](https://user-images.githubusercontent.com/7143383/98640377-f006e580-22de-11eb-9bb5-dde9fe9b81b6.png)

(cherry picked from commit 2a34080)
@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8209, 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 #8209, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.:tada:

Handy links:

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 close button not using the tab's text color (#8046)
4 participants
@TylerEich @Don-Vito @dcheatha @zadjii-msft