Button Link missing keyboard focus style

[ianschmitz]

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

Link component with component="button" prop should have :focus outline style which is important for a11y.

Current Behavior 😯

Link component with component="button" prop overrides the outline style to none. See https://github.com/mui-org/material-ui/blob/6ce1de9ac755db61461df55ff5163c41a990b407/packages/material-ui/src/Link/Link.js#L34

Steps to Reproduce 🕹

Link: https://material-ui.com/components/links/#accessibility

1. Keyboard focus on the Button Link
2. Notice that there's no keyboard focus indicator like there is on Link components that render an a element.

Your Environment 🌎

Tech	Version
Material-UI	v4.0.0
React	v16.8.4
Browser	Chrome 74

Let me know if you want me to send a PR. I would propose to remove the outline style from Link to use the default browser style, which is consistent with Link rendered as an a element.

[oliviertassinari]

@ianschmitz What do you think of?:

diff --git a/packages/material-ui/src/Link/Link.js b/packages/material-ui/src/Link/Link.js
index 8d2cce4f4..ad565e212 100644
--- a/packages/material-ui/src/Link/Link.js
+++ b/packages/material-ui/src/Link/Link.js
@@ -3,6 +3,8 @@ import PropTypes from 'prop-types';
 import clsx from 'clsx';
 import { capitalize } from '../utils/helpers';
 import withStyles from '../styles/withStyles';
+import { useIsFocusVisible } from '../utils/focusVisible';
+import { useForkRef } from '../utils/reactHelpers';
 import Typography from '../Typography';

 export const styles = {
@@ -44,27 +46,59 @@ export const styles = {
     '&::-moz-focus-inner': {
       borderStyle: 'none', // Remove Firefox dotted outline.
     },
+    '&$focusVisible': {
+      outline: 'auto',
+    },
   },
+  focusVisible: {},
 };

 const Link = React.forwardRef(function Link(props, ref) {
   const {
     classes,
     className,
-    component = 'a',
     color = 'primary',
+    component = 'a',
+    onBlur,
+    onFocus,
     TypographyClasses,
     underline = 'hover',
     variant = 'inherit',
     ...other
   } = props;

+  const linkRef = React.useRef();
+  const { isFocusVisible, onBlurVisible } = useIsFocusVisible(() => linkRef.current.ownerDocument);
+  const [focusVisible, setFocusVisible] = React.useState(false);
+  const handlerRef = useForkRef(linkRef, ref);
+
+  const handleBlur = event => {
+    if (focusVisible) {
+      onBlurVisible(event);
+      setFocusVisible(false);
+    }
+    if (onBlur) {
+      onBlur(event);
+    }
+  };
+
+  const handleFocus = event => {
+    if (isFocusVisible(event)) {
+      setFocusVisible(true);
+    }
+
+    if (onFocus) {
+      onFocus(event);
+    }
+  };
+
   return (
     <Typography
       className={clsx(
         classes.root,
         {
           [classes.button]: component === 'button',
+          [classes.focusVisible]: focusVisible,
         },
         classes[`underline${capitalize(underline)}`],
         className,
@@ -72,7 +106,9 @@ const Link = React.forwardRef(function Link(props, ref) {
       classes={TypographyClasses}
       color={color}
       component={component}
-      ref={ref}
+      ref={handlerRef}
+      onFocus={handleFocus}
+      onBlur={handleBlur}
       variant={variant}
       {...other}
     />

This wouldn't be the final implementation, but it gives an idea of the direction we can take.

[oliviertassinari]

The only example I have found so far, the Medium strategy on the "Write a response" button:

default

focus

hover

[ianschmitz]

@oliviertassinari your diff looks good 🙂. Thanks!

Got it running locally just working on unit tests then I'll have a PR up shortly.

Are you thinking of changing default styles? I'm happy to add that in but would be a breaking change no?

[eps1lon]

Pretty sure I asked before: Why do you need <Link component="button" /> over <Button />?

[oliviertassinari]

@eps1lon To rephrase it: why is Medium using a link style over a button style for its "Write a response" button? Why is GitHub using a link style for the "Show more" button?

Why does Bootstrap has a button link? https://getbootstrap.com/docs/4.3/components/buttons/#examples.

I don't have an answer to this question, I do think that mixing the button behavior with a link style is deceptive for a user. But at the same time, it's a common practice.

[ianschmitz]

In general I would agree. Our specific use case was to have a link within a sentence that is explaining something to the user.