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

Rating: Fluent theme #6745

Closed
mikewheaton opened this issue Oct 18, 2018 · 6 comments · Fixed by #9596
Closed

Rating: Fluent theme #6745

mikewheaton opened this issue Oct 18, 2018 · 6 comments · Fixed by #9596

Comments

@mikewheaton
Copy link
Contributor

No description provided.

@Vitalius1
Copy link
Contributor

@betrue-final-final
Can you verify please if the toolkit for fluent Rating is correct? I will make the small change I see in colors of the unchecked star but will wait for your confirmation to merge. Thanks!

@betrue-final-final
Copy link
Member

Nice work. @Vitalius1, can you take the layout improvements and open a PR on current Fabric?

@mikewheaton mikewheaton changed the title Rating: Fluent experiment Rating: Fluent theme Nov 6, 2018
@marygans
Copy link
Collaborator

marygans commented Jun 13, 2019

@betrue-final-final Can you confirm the following changes and I will make them? thanks!

Fluent Updates Needed

Update star icon rest state color

  • Current: #a19f9d
  • Expected (based on toolkit): #605e5c

Update on hover outline to fill (see screenshot below)

  • Current: star outlined with #0078d4
  • Expected(based on toolkit): fill star icon with #0078d4

Add label for Rating component (see screenshot below)

  • Current: There is no label to indicate the rating level
  • Expected(based on toolkit): an inline label indicating the rating level

rating-onHover-example

rating-missing-label-example

@Vitalius1
Copy link
Contributor

On the color issue make sure you get some context from #8246 and #7955 and the issue that these PRs were fixing. It might be that the toolkit was not updated or something. Definitely check with Ben.

On the label issue... good question 🤔 . Perhaps Ben can vote on that one.

@marygans
Copy link
Collaborator

@Vitalius1 thanks for referencing those PRs, it helped a lot with context! It seems like the accessibility issue came from the previous design with grey unchecked state. Since this change is just a matter of changing the hover state from a border outline to a fill, it might be safe to assume that the color used has been checked and passes accessibility, I'll confirm/deny with Ben!

@betrue-final-final
Copy link
Member

Update star icon rest state color: #605e5c

Update on hover outline to fill (see screenshot below)
Star outlined with #0078d4 - Will fix toolkit. https://tasks.office.com/microsoft.onmicrosoft.com/en-US/Home/Task/-n1dHizaYE6sMqHMnojaS5UAFZfS?Type=TaskLink&Channel=Link&CreatedTime=636967391979245044

Will remove label for Ratings in toolkit.

@marygans marygans mentioned this issue Jun 26, 2019
2 tasks
@msft-github-bot msft-github-bot added Status: Fixed Fixed in some PR and removed Status: In PR labels Jul 8, 2019
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants