microsoft / accessibility-insights-web

Accessibility Insights for Web
https://accessibilityinsights.io
MIT License
827 stars 147 forks source link

fix: Left nav lacks clear tab or focus indicators with Windows HC modes #7311

Open SaanicaG opened 4 months ago

SaanicaG commented 4 months ago

Details

Fixed the Color contrast issue of focus for left nav tab for high contrast theme. Added underlines for links as per the requirement.

Motivation

Addresses Issue - Left nav lacks clear tab or focus indicators with Windows HC modes

Context

Theme - Desert image

Theme - Aquatic image

Theme - Dusk image

Theme - Night Sky image

Screenshots of added underlines upon hover are pasted as below-

Theme - None

image

Theme - Aquatic

image

Theme - Desert image

Theme - Dusk image

Theme - Night Sky

image

Pull request checklist

v-viyada commented 4 months ago

This is functional, but I find that the hover styles in contrast themes are nausea-inducing. Could we implement it closer to how it is in the default theme where we change the background color on hover? Or maybe underline the button text?

@nang4ally what do you think?

I remember, we discussed about this in one of standup meeting and finalized only to outline for hover style which is similar to windows file explorer in high contrast but we can still change if this does not look as required.

Btw we have limited color selection in contrast themes. Below are the color options in aquatic theme and users can change them according to their choice.

image showing color option for windows aquatic theme
madalynrose commented 2 months ago

I believe the new directive is to change the left nav item's background color on hover, and also to add text underline on hover to all links (e.g. on the Getting Started and Overview pages in Assessment). @nang4ally can you add/amend anything I missed?

nang4ally commented 2 months ago

Agree with Madalyn- that is what we agreed on with the Design call. This is a summary of what we decided:

madalynrose commented 2 weeks ago

This looks great! I see one link that is missing hover styles: the target page link in the command bar the accessibility insights details view command bar with the target page link circled Once that's updated, I think this is ready for merge!