mui-org/material-ui

Way to add closeIcon to <Dialog /> #13520

albinekb posted onGitHub

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.

<!-- Checked checkbox should look like this: [x] -->

  • This is not a v0.x issue. <!-- (v0.x is no longer maintained) -->
  • 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

image

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?


@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)

capture d ecran 2018-11-05 a 20 50 14

posted by oliviertassinari over 6 years ago

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

posted by albinekb over 6 years ago

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

posted by IssueHuntBot over 6 years ago

@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.

posted by alxsnchez over 6 years ago

@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.

posted by oliviertassinari over 6 years ago

@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.

image

posted by alxsnchez over 6 years ago

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.

posted by eps1lon over 6 years ago

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.

posted by oliviertassinari over 6 years ago

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.

posted by eps1lon over 6 years ago

Fund this Issue

$40.00
Funded
Only logged in users can fund an issue

Pull requests

Recent activities

issuehuntfest funded 40.00 for mui-org/material-ui# 13520
over 6 years ago