Material-ui: Modal has strange blue border

[mschipperheyn]

The modal has a strange blue border that disappears in the demo once you click open another modal and close it

• [ x] This is a v1.x issue (v0.x is no longer maintained).
• [ x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

The second screenshot is after opening and closing a second modal and is what should be the way it works

Current Behavior

Is the first screenshot.

Steps to Reproduce (for bugs)

1. Go to https://material-ui.com/utils/modals/#modals
2. Open a modal

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | v1.0.0 |
| React |16.3 |
| browser | Chrome |

[vuhrmeister]

That's native Chrome behavior which adds that outline. Either you set disableAutoFocus={true} on the Modal component or you tell the browser not to draw an outline: outline: 'none'.
Remember removing the focus or the outline reduces accessibility.

[oliviertassinari]

One of the best approaches for solving this problem could be to use the :focus-visible CSS primitive. But whatever solution you pick. It's most likely not the Modal responsability to handle it.

[gkiely]

@vuhrmeister I've noticed disableAutoFocus={true} doesn't actually work.
https://material-ui.com/api/modal/

[oliviertassinari]

@gkiely You are looking for the disableEnforceFocus property. But I won't encourage people using it for this issue. The best solution is to add this CSS: outline: none;.

[sebasrodriguez]

Hi @oliviertassinari, is this something worth considering adding to the CssBaseline component? I could create a PR for it if you think it would be beneficial.

[oliviertassinari]

@sebasrodriguez I have been benchmarking some alternative solutions:

• react-modal disable the outline http://reactcommunity.org/react-modal/examples/minimal.html
• the native dialog has no outline https://codepen.io/anon/pen/MZvzeo?&editable=true
• we disable the outline on our own Dialog component

I think that we can change the bahavior here too. What do you think of this change?

function getHasTransition(props) {
  return props.children ? props.children.props.hasOwnProperty('in') : false;
}

export const styles = theme => ({
  /* Styles applied to the root element. */
  root: {
    position: 'fixed',
    zIndex: theme.zIndex.modal,
    right: 0,
    bottom: 0,
    top: 0,
    left: 0,
+   // We disable the focus ring for mouse, touch and keyboard users.
+   // At some point, it would be better to keep it for keyboard users.
+   // :focus-ring CSS pseudo-class will help.
+   outline: 'none',
  },
  /* Styles applied to the root element if the `Modal` has exited. */
  hidden: {
    visibility: 'hidden',
  },
});

Do you want to submit a pull-request? :)

[sebasrodriguez]

Sure @oliviertassinari I'll submit a PR with your changes. Thanks for replying so fast to this.

[sebasrodriguez]

@oliviertassinari I've tried with your changes but there is a problem with them. Basically the rendered structure for the modal is this:

<div> {/* Root modal div */}
    <div /> {/* Optional backdrop div */}
    <div /> {/* Modal content given by the user. This div gets the focus outline */}
</div> 

So the changes you've proposed don't remove the focus ring.

How do you feel about this instead?

export const styles = theme => ({
  /* Styles applied to the root element. */
  root: {
    position: 'fixed',
    zIndex: theme.zIndex.modal,
    right: 0,
    bottom: 0,
    top: 0,
    left: 0,
+   // We disable the focus ring for mouse, touch and keyboard users.
+   // At some point, it would be better to keep it for keyboard users.
+   // :focus-ring CSS pseudo-class will help.
+  '& > :last-child': {
+       outline: 'none',
+   },
  },
  /* Styles applied to the root element if the `Modal` has exited. */
  hidden: {
    visibility: 'hidden',
  },
});

From what I see the last child is always going to be the user given content. Let me know your input on this.

[oliviertassinari]

@sebasrodriguez Oh right, sorry. It's just a matter of fixing the demo then. I think that using '& > :last-child': { is too brittle.

[vince83110]

That's native Chrome behavior which adds that outline. Either you set disableAutoFocus={true} on the Modal component or you tell the browser not to draw an outline: outline: 'none'.
Remember removing the focus or the outline reduces accessibility.

I may be naive, but where do you place this CSS code in a React app ? What is the best way ? Thanks

[vuhrmeister]

I may be naive, but where do you place this CSS code in a React app ? What is the best way ? Thanks

Directly on the element. Or, if you want to disable it globally (I won't recommend it), somewhere on the top level alongside your CssBaseline component (if you use it). Or in the global config: https://material-ui.com/customization/globals/#global-css