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

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

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

SaanicaG
Copy link
Contributor

@SaanicaG SaanicaG commented Apr 18, 2024

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
  • Added focus indicators/clear tab on left nav. Screenshots added for reference-

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

  • Addresses an existing issue: (Left nav lacks clear tab or focus indicators with Windows HC modes #4263)
  • Ran yarn fastpass
  • Added/updated relevant unit test(s) (and ran yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • PR title AND final merge commit title both start with a semantic tag (fix:, chore:, feat(feature-name):, refactor:). See CONTRIBUTING.md.
  • (UI changes only) Added screenshots/GIFs to description above
  • (UI changes only) Verified usability with NVDA/JAWS

@SaanicaG SaanicaG changed the title color contrast fix fix: Color Contrast Issue Apr 22, 2024
@SaanicaG SaanicaG changed the title fix: Color Contrast Issue fix: Left nav lacks clear tab or focus indicators with Windows HC modes Apr 22, 2024
@v-viyada v-viyada marked this pull request as ready for review April 25, 2024 17:31
@v-viyada v-viyada requested a review from a team as a code owner April 25, 2024 17:31
Copy link
Contributor

@madalynrose madalynrose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@v-viyada
Copy link
Contributor

v-viyada commented May 1, 2024

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
Copy link
Contributor

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
Copy link

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

  • To change the background color of the left navigation bar in high contrast mode to match the system settings.
  • To add underlines and color changes to the links in the content area to indicate hover state.
  • To invert the text color of the get started and help links on hover to match the save assessment button.

@SaanicaG SaanicaG closed this Jul 5, 2024
@SaanicaG SaanicaG reopened this Jul 5, 2024
@madalynrose
Copy link
Contributor

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!

@microsoft microsoft deleted a comment from v-rakeshsh Sep 20, 2024
@v-rakeshsh v-rakeshsh closed this Sep 23, 2024
@v-rakeshsh v-rakeshsh reopened this Sep 23, 2024
@v-rakeshsh
Copy link
Contributor

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!

HI @madalynrose
Fixed the underline style for Link.
please find the screenshot from Dev
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants