search

helpscout / hsds-react

Help Scout Design System (HSDS) — React Component Library
MIT License
86 stars 17 forks source link

HSD- 261 Update CheckmarkCard disabled UI #1061

Closed plbabin closed 2 years ago

plbabin commented 2 years ago

Problem/Feature

CleanShot 2022-05-10 at 10 13 54

To support the work in an upcoming project, we had to implement some changes to the disabled state of the CheckmarkCard:

Update the UI

We removed all shadows and set a grey background (grey.200). Avatar is faded out and the heading/subtitle color are slightly lighter

CleanShot 2022-05-10 at 10 35 26@2x

Add tooltip support to the whole card

By passing a cardTooltipText prop to the CheckmarkCard, a tooltip will appear on focus/mouseover. The tooltip will still work when the card is disabled

CleanShot 2022-05-10 at 10 37 05@2x

Focus ring

we migrate the component to use the focusRing mixin for both the card and mark component. It simplifies the code but nothing should have change visually

plbabin commented 2 years ago

@phamburglar @tinkertrain It's possible to hide the focus state on when the CheckmarkCard is disabled. I did include it here because I wanted to show the tooltip by both mouse and keyboard.

The question is, what is more inline with a11y here: not focusable when disabled or not being able to show the tooltip when keyboard navigating to it 🤷. There is no perfect answer here I guess. The call we'll made will be the right one.

Long story short, anything's possible here 😄

phamburglar commented 2 years ago

@plbabin @tinkertrain Went down a mini rabbit hole and came across this link that explains how to make our scenario more accessible.

tldr; We should allow it to be focusable, even when disabled so that the user can see the tool tip. Essentially the "greyed" out opacity state will visually let them know it's disabled, but the focusable state will allow them to display the tooltip in line with what they're focused on. 👍

plbabin commented 2 years ago

@phamburglar so are we good with the changes in this PR? I know you approved it but just want to make sure that everything's ok following the a11y discussion

phamburglar commented 2 years ago

@plbabin Good to go! ✅ -- My bad didn't realize you were waiting for me 🥴

plbabin commented 2 years ago

@plbabin Good to go! ✅ -- My bad didn't realize you were waiting for me 🥴

@phamburglar no worries, waiting is a big word. more like the PR was in the background because I kinda forgot about it 😂