storybookjs / design-system

🗃 Storybook Design System
https://master--5ccbc373887ca40020446347.chromatic.com/
1.91k stars 585 forks source link

Only change the look of a link when it is hovered over or tabbed to #359

Closed andrewortwein closed 2 years ago

andrewortwein commented 2 years ago

Primary bug

The Link component presents text and/or icons as an element that can be selected. When a user focuses on a link, the content is visibly shifted up and its color changes, letting the user know that they can interact with the element.

A bug exists in which a link appears to still be hovered over after it has been pressed, even though the mouse has been moved off the element. For example, note the following video where the button with text "is actually a button" appears to be stuck in a focused state:

https://user-images.githubusercontent.com/3391672/170604202-3ad4c603-7f57-44fe-bc8b-d0a2eba3b355.mov

As the mouse moves over the link, you see it shift up and the color darkens, as expected. But after the link is clicked and the mouse moves away, the link stays elevated with its darker hue. It is only after the mouse clicks somewhere else that the link returns to its normal appearance.

This is due to a combination of how the "focused" style is applied and how a browser chooses when an element remains in focus. The Link component uses the :focus pseudo-class to alter the appearance of the link (Link.tsx lines 38-42). This correctly applies these styles to a link when it has been tabbed to, a good workflow for accessibility (a11y) reasons.

But there is a catch when using a mouse: In most browsers, after a mouse clicks on a link, the link remains in the focused state. That means that the altered appearance of the link remains, even though the mouse has moved away. This feels unusual for a mouse user. They are no longer focused on the button, so why should it still appear elevated and darker?

It's tempting to solve this by forcibly moving the focus away from the button after clicking in it, but I strongly recommend against that. For one, to where would you move it? More importantly, that can be a very disorienting experience for a keyboard user. Programmatically moving focus from one element to another is usually frowned upon in the a11y community.

The solution in this PR is to use :focus-visible instead of :focus. The difference is that the former lets the user agent use heuristics to determine when the element should appear in focus (see https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible for more info). For a keyboard user, this is when you tab to it. For a mouse user, this is likely only when you have hovered over it. Here is how it acts with the change:

https://user-images.githubusercontent.com/3391672/170605083-d57a9a0d-7d5e-4830-905b-de369c6fc369.mov

Here, the link correctly shifts up and darkens its color when hovered over, but returns to its normal state after it has been clicked and the mouse has moved away. The change in line 38 of Link.tsx resolves this.

Secondary issue

While investigating this behavior, I was slightly puzzled by some related behavior. Links can be in various forms. For example, a "secondary" link presents the content in a grey color instead of blue. An "inverse" link present the content as white text on a black background.

image

But when I tab through these links, I didn't expect all of them to change to a blue color. Here is what it looks like before this fix:

https://user-images.githubusercontent.com/3391672/170606897-f2c89c3d-317e-4c83-8980-7e2d3e1a89f2.mov

Since each of these variants uses its own hue (with a lighter or darker shade) when hovered over, I expected to see the same thing when focused. To see what that would look like, I went ahead and included the changes in this PR:

https://user-images.githubusercontent.com/3391672/170607000-b29f23f4-7fa6-44fe-859b-68c1816eb941.mov

I don't know if this goes with or against the desired UX, which is really the whole reason for this huge description. Should the variant links retain their hue and only change shade when focused on or should they always have a blue color when focused? If it is the latter, I can remove that part of the change from this PR and still satisfy the primary bug above.

📦 Published PR as canary version: 7.3.8-canary.359.76f5d46.0
:sparkles: Test out this PR locally via: ```bash npm install @storybook/design-system@7.3.8-canary.359.76f5d46.0 # or yarn add @storybook/design-system@7.3.8-canary.359.76f5d46.0 ```
MichaelArestad commented 2 years ago

@andrewortwein I would prefer we work out the hover colors in a followup issue. My instinct is that primary-tertiary links should be blue on hover. Inverted should probably go full white, but it's hard to say out of context. I also wonder if there is a need for so many variants of the link component.

andrewortwein commented 2 years ago

@andrewortwein I would prefer we work out the hover colors in a followup issue. My instinct is that primary-tertiary links should be blue on hover. Inverted should probably go full white, but it's hard to say out of context. I also wonder if there is a need for so many variants of the link component.

Cool, I'll split that part out into a separate issue for the backlog. Thanks!

github-actions[bot] commented 2 years ago

:rocket: PR was released in v7.3.8 :rocket: