[Link] Improve accessibility of focus styles

[ry5n]

WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-ux/issues/40 and https://github.com/Shopify/polaris-ux/issues/156

Original design

In the original issues, we landed on some more accessible focus styles for links:

Implementation challenges

In implementing these, however, I ran into some problems. The main one is that if Link remains styled as an inline element, image links look bad out of the box:

We could fix this by making all images in Polaris block-level by default. This is usually what you want, but may have knock-on effects in consuming apps. Another way would be to set links to be inline-flex, but has similar risks for existing apps.

Current approach: go with browser defaults

To avoid breaking changes, I’ve opted to go a different route from the original designs and strip back to browser default styles instead.

Here’s how things look now in different browsers. I left out iOS Safari since I haven’t been able to get the external keyboard emulation working in order to visualize element focus.

Chrome (Android)

Chrome (Mac)

Safari (Mac)

Edge (Windows)

Firefox (Mac)

---

How to review

My big question is: are we OK moving forward with this approach? It’s not ideal, but it’s also a big improvement on the inaccessible styles we have today.

Please give feedback based on either the screenshots attached, or 🎩using the playground below:

Copy-paste this code in playground/Playground.tsx:

import * as React from 'react';
import {Page, Link} from '../src';

interface State {}

export default class Playground extends React.Component<never, State> {
  render() {
    return (
      <Page title="Playground">
        <p style={{marginBottom: 16}}>
          Focus styles have to account for text wrapping.{' '}
          <Link url="#">Text links</Link> are intended for use within other
          text.
        </p>

        <p style={{marginBottom: 16}}>
          Focus styles have to account for text wrapping.{' '}
          <Link monochrome url="#">
            Monochrome links
          </Link>{' '}
          are also intended for use within other text.
        </p>

        <p style={{marginBottom: 16, maxWidth: 400}}>
          Focus styles have to account for text wrapping. Focus styles have to
          account for text wrapping.{' '}
          <Link url="#">Very long link that will wrap at some point</Link> are
          intended for use within other text.
        </p>

        <Link url="#">
          <img src="http://placekitten.com/80/80" alt="Kitten" />
        </Link>
      </Page>
    );
  }
}

Next steps

If we’re OK with this experience, I’ll update documentation and changelog and ship. We can then consider what larger changes we’d want to make to Polaris’ foundations to support a better implementation/breaking change later.

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• [N/A] Tophatted documentation changes in the style guide

[kaelig]

I see the proposed solution to be sub-optimal (as mentioned above), but considering the state of CSS in 2019, it's probably the most appropriate way to achieve the intended results today.

The future is promising, though!

Suggestions for the long term

Some pretty cool CSS pseudo-classes are coming as part of the CSS Selectors level 4 spec. For reference, I thought I'd mention them in this issue.

2020? :focus-visible

Partial support today, expect good support for most of our users by 2020?

Targets element to show a focus ring (custom or native) only when appropriate (as in: shows a ring to a keyboard user, but not to a mouse-only user), using the :focus-visible pseudo-class.

a:focus-visible {
  // custom focus outline, only appears when appropriate
}

>2020? a:has(> img)

No support today… perhaps in a year or 2?

To improve the look and feel of custom focus rings on images, we could use the :has pseudo-class and target specifically images that happen to be in links (rather than showing the focus ring on the inline link itself.

a:has(> img):focus-visible {
  // remove outline on the link itself

  img {
    // apply custom focus outline on the img
  }
}

---

⚠️ the above code is purely speculative, the spec may change 🤷‍♂

[sarahill]

@kaelig @ry5n I'm curious as to why this approach, using the browser defaults, is suboptimal? It is visually inconsistent across browsers but I'm generally all for using the defaults. Is there more to this than visual consistency?

This seems like a big improvement to me 👍

[dpersing]

@kaelig We're tracking :focus-visible viability in #818, just for reference.

@ry5n I wouldn't worry so much about testing on iOS specifically, but making sure focus looks okay when the page is viewed at a mobile breakpoint. We can do that by resizing the window or resizing text-only in Firefox.

[dpersing]

This looks good to me. I tested in Firefox and Edge as well, including in High Contrast for Edge, and with text resizing. Everything works as I'd expect from the defaults!

We should probably prepare a statement about why we've decided to do this, since I expect that we'll get some complaints about it being "ugly".

[ry5n]

Good idea @dpersing! I drafted an explanation which we can jam on here: https://docs.google.com/document/d/17uH9fLWPN7ENugsQ1C91I_9izodqd_wR2U5Lcb_9B20/edit?usp=sharing

[ry5n]

I checked the style guide and I don’t think this needs any component documentation updates. Going to ship when everything goes green 🍏

[dpersing]

This PR also fixes https://github.com/Shopify/polaris-ux/issues/242!

[ry5n]

@sarahill I realized I never answered your question about why browser defaults are sub-optimal.

There’s a couple of reasons in my opinion:

1. On the face of it, some browsers have more accessible styles than others. Safari’s styles, seem pretty darn light to me, and the 1px dotted border used by several others is hard to see around images. If we can do better, why not?

2. Not all browsers adjust their focus style based on the background. On dark backgrounds, even otherwise good browser defaults can be hard to see.

3. It would be nice to have focus styles that matched the look and feel of the interface (assuming they’re visible and familiar).

[dpersing]

Im optimistic we can bake a few universal, accessible, custom focus styles into the next style guide iteration!

[sarahill]

@ry5n Thanks for the reply! I dug into it a bit more after I asked the question too. I found pretty much the same things you stated above. This'll be a good one to see evolve. Nice work, Ryan 🎉

[mirualves]

Looking good!

[mirualves]

👍