Material-ui: Way to add closeIcon to <Dialog />

[albinekb]

I have gotten a lot of user feedback that users don't understand how to close Dialogs. They don't understand that they can press esc nor that they can just click anywhere outside the Dialog to close it.

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

Expected Behavior

I'd like an option to include a "closeIcon" that should be shown top right, like facebook has for all their Dialogs

Current Behavior

Users don't understand how to close dialogs

Examples

Context

I'm trying to make my app easier to use. Is there a way to extend <Dialog /> without needing to re-implement all of the <Dialog /> behaviour?

[oliviertassinari]

@albinekb We do the following at onepixel.com, if you want to add a demo, I'm all in for that.

import React from 'react'
import PropTypes from 'prop-types'
import { withStyles } from '@material-ui/core/styles'
import IconButton from '@material-ui/core/IconButton'
import CloseIcon from '@material-ui/icons/Close'
import Typography from '@material-ui/core/Typography'

const styles = theme => ({
  root: {
    padding: `${theme.spacing.unit * 6}px ${theme.spacing.unit * 3}px 0`,
  },
  closeButton: {
    position: 'absolute',
    right: theme.spacing.unit / 2,
    top: theme.spacing.unit / 2,
    color: theme.palette.grey[500],
  },
})

function DialogTitle(props) {
  const { children, classes, onClose, TypographyProps } = props

  return (
    <div className={classes.root}>
      <Typography align="center" variant="title" {...TypographyProps}>
        {children}
      </Typography>
      {onClose ? (
        <IconButton aria-label="Close" className={classes.closeButton} onClick={onClose}>
          <CloseIcon />
        </IconButton>
      ) : null}
    </div>
  )
}

DialogTitle.displayName = 'DialogTitle'

DialogTitle.propTypes = {
  children: PropTypes.node.isRequired,
  classes: PropTypes.object.isRequired,
  onClose: PropTypes.func,
  TypographyProps: PropTypes.object,
}

export default withStyles(styles)(DialogTitle)

[albinekb]

Cool, I will add a demo for it 👍 Thanks!

[IssueHuntBot]

@issuehuntfest has funded $40.00 to this issue. See it on IssueHunt

[alexsanchezdev]

@oliviertassinari checking the example image provided I believe that the expected behavior would be to add a closeIcon on the top right corner of the overlay in absolute position relative to the viewport and not inside the Dialog itself, as this solution can be achieved easily with the current implementation.

[oliviertassinari]

@alxsnchez You are right, but I believe it would be beneficial to more people if it was displayed at the top right corner of the dialog rather than the overlay.

[alexsanchezdev]

@oliviertassinari what about a opt-in option? There are many cases where, because of the layout distribution, the icon inside the modal is not a good option. For example, in this Instagram modal, if the icon where inside the modal it could "conflict" with the Follow action in some cases.

While there's a way to include an icon inside the modal, there's no option to place it on the overlay, right? I believe this could benefit some projects.

[eps1lon]

I have gotten a lot of user feedback that users don't understand how to close Dialogs.

Are you talking about dialogs or modals? Because a dialog should always have a named close action according to the material guidelines. The user should know that he is cancelling the dialog and not just closing it. The material guidelines only mention close icons for fullscreen dialogs. Furthermore it's explicitly defined how a dialog should be dismissed. If you want to replace one of these actions we might enable this customization but we shouldn't add it to the docs. They should follow the guidelines.

The facebook examples are all modals. We should respect that distinction.

[oliviertassinari]

If you want to replace one of these actions we might enable this customization but we shouldn't add it to the docs.

We have different customization examples that don't follow the specification: Button, Input, Table, Tab, Switch. You can look at the analytics data, people use them. Maybe we should say in the doucement that it doesn't following the specification. Right now, it's implicit.

[eps1lon]

Maybe we should say in the doucement that it doesn't following the specification. Right now, it's implicit.

Sounds like to good compromise. I understand that different contexts have different requirements but material design is the headline so it should be clear when it is broken.

[dep-dt]

@oliviertassinari

const styles = theme => ({
  root: {
    padding: `${theme.spacing.unit * 6}px ${theme.spacing.unit * 3}px 0`,
  },
  closeButton: {
    position: 'absolute',
    right: theme.spacing.unit / 2,
    top: theme.spacing.unit / 2,
    color: theme.palette.grey[500],
  },
})

position: absolute would cause the close icon to scroll with the modal's contents if there were a large amount of text in the modal. Just something to be mindful of.