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

TabView: Fix issues with lacking contrast issues for new style #3951

Merged

Conversation

marcelwgn
Copy link
Contributor

Description

Fixes the bugs as outlined in #3950:

  • Update color on icons
  • Set the selected items header fontweight to semibold

@DHowett @zadjii-msft this will impact terminal as the selected state changes, just FYI.

Motivation and Context

Closes #3950

How Has This Been Tested?

Screenshots (if appropriate):

TabView with updated style in light theme

TabView with updated style in dark theme

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jan 15, 2021
@mdtauk
Copy link
Contributor

mdtauk commented Jan 15, 2021

I know its slightly out of scope - but in the future when WinUI moves from MDL2 Icons to Fluent UI System Icons - will the currently selected tab need to use a Filled Icon, instead of the Regular ones?

image

@marcelwgn
Copy link
Contributor Author

This is something for the developers to decide, they can use any icon they want I think,

@mdtauk
Copy link
Contributor

mdtauk commented Jan 15, 2021

Just thinking it may need some visual state code or an API change to support a TabViewItem Icon and CurrentIcon property

And would it even be a desired behaviour?

@marcelwgn
Copy link
Contributor Author

Ohhh I see. This might actually be a very neat thing to have though I fear that it might not be ease to use in most cases. As a user I would probably not care if the icon switches when selecting a tab.

@ranjeshj
Copy link
Contributor

ranjeshj commented Jan 15, 2021

l the currently selected tab need to use a Filled Icon, instead of the Regular ones?

The app developer should be able to do this today by setting the icon, so it is more of a friendlier way to achieve the same thing I think that will need to be a new feature.

@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj added area-UIDesign UI Design, styling team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jan 15, 2021
@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj
Copy link
Contributor

@chingucoding can you please merge in master ?

@marcelwgn
Copy link
Contributor Author

There don't seem to be any merge conflicts, but I can merge master branch in this if you want @ranjeshj .

@ranjeshj
Copy link
Contributor

There don't seem to be any merge conflicts, but I can merge master branch in this if you want @ranjeshj .

Yes please merge, so we can do another pass over the tests.

@marcelwgn
Copy link
Contributor Author

Merge commit pushed now.

@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj merged commit 4ee15c6 into microsoft:master Jan 20, 2021
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Jul 10, 2022
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TabView area-UIDesign UI Design, styling team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TabView color problems
3 participants