[Dialog] Form + scroll in DialogContent

[bogdansoare]

• [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.

I know this has been discussed in #12126, but there is still an issue with the scrolling, the title and the actions aren't fixed and they scroll

Here is the codesandbox: https://codesandbox.io/s/qlo16r5v59

Here is the code

<Dialog open aria-labelledby="form-dialog-title">
  <form
    onSubmit={e => {
      alert("form submit!");
      e.preventDefault();
    }}
  >
    <DialogTitle id="form-dialog-title">Log in</DialogTitle>
    <DialogContent>
      <DialogContentText>
        Please enter your account number and password.
      </DialogContentText>
      <TextField
        autoFocus
        margin="dense"
        label="Account Number"
        type="text"
        fullWidth
      />
      <TextField
        margin="dense"
        label="Password"
        type="password"
        fullWidth
      />
      <div style={{ height: 1000 }} />
    </DialogContent>
    <DialogActions>
      <Button onClick={() => alert("cancel")} color="primary">
        Cancel
      </Button>
      <Button
        type="submit"
        onClick={() => alert("login")}
        color="primary"
        variant="contained"
      >
        Log in
      </Button>
    </DialogActions>
  </form>
</Dialog>

[oliviertassinari]

Should this demo https://material-ui.com/demos/dialogs/#confirmation-dialogs uses a form?

[bogdansoare]

that example doesn't contain a form as a direct child of Dialog

if DialogTitle, DialogContent, DialogActions aren't direct children of the Dialog then the scrolling breaks, isn't this a bug?

[oliviertassinari]

@bogdansoare https://codesandbox.io/s/o74xj837y9 Maybe?

[bogdansoare]

@oliviertassinari thanks for the solution, the only issue is the big spacing between the title and the content

[oliviertassinari]

@bogdansoare There is an ugly hack to solve the spacing issue:

      <Dialog open aria-labelledby="form-dialog-title">
        <DialogTitle id="form-dialog-title">Log in</DialogTitle>
        <form
          onSubmit={e => {
            alert("form submit!");
            e.preventDefault();
          }}
          style={{
            display: "flex",
            flexDirection: "column"
          }}
        >
          <span />
          <DialogContent>
            <DialogContentText>
              Please enter your account number and password.
            </DialogContentText>
            <TextField
              autoFocus
              margin="dense"
              label="Account Number"
              type="text"
              fullWidth
            />
            <TextField
              margin="dense"
              label="Password"
              type="password"
              fullWidth
            />
            <div style={{ height: 1000 }} />
          </DialogContent>
          <DialogActions>
            <Button onClick={() => alert("cancel")} color="primary">
              Cancel
            </Button>
            <Button
              type="submit"
              onClick={() => alert("login")}
              color="primary"
              variant="contained"
            >
              Log in
            </Button>
          </DialogActions>
        </form>
      </Dialog>

[bogdansoare]

@oliviertassinari thanks for the hack. Haha, yeah it's ugly but it works 🙈

[Ethorsen]

+1 We face the same issue. I will implement that ugly fix for now.

But it would be nice to fix MUI at the source and allow <Dialog*> elements to be wrapped in a form without affecting layout/scrolling functionality.

[liitfr]

Hi there This fix doesn't work on Firefox (65.0.1)

[liitfr]

looks like adding overflow-y: auto; to form's style does the trick !

[jlil]

@oliviertassinari the suggested fix doesn't work anymore. Can we reopen this to allow support for non direct <Dialog*> children?

[jlil]

For reference - the following styling workaround did it:

form: {
    display: 'flex',
    flexDirection: 'column',
    flex: '1 1 auto',
    overflowY: 'auto',
  },

[oliviertassinari]

We have changed the styles in v4. It should no longer be a problem.

[hayatae]

This same problem still exists in v4. Has another ticket been opened to solve this issue? https://codesandbox.io/s/smoosh-morning-wc9f1

[oliviertassinari]

@hayatae Interesting, you need to apply this CSS

      <Dialog>
        <form
          onSubmit={(event) => {
            alert('form submit!');
            event.preventDefault();
          }}
          style={{
            overflowY: 'auto',
            display: 'flex',
            flexDirection: 'column',
          }}
        >
          <DialogTitle

on the <form> element if you want to the dialog content to scroll instead of the dialog paper.

Alternatively, you can make the paper to host your form:

      <Dialog
        PaperProps={{
          component: 'form',
          onSubmit: (event) => {
            alert('form submit!');
            event.preventDefault();
          }
        }}
      >
        <DialogTitle

For modern browser, we can use display: contents: https://github.com/mui/material-ui/issues/13253#issuecomment-742708515

[seamuslowry]

It seems the solution using PaperProps does not work with TypeScript. Unless I'm missing something. Currently on v4.2.0

[seamuslowry]

To clarify, it works, but TypeScript complains that component and onSubmit are invalid properties on PaperProps.

I've set up a codesandbox reproduction here

It seems that the problem is two-fold. Both component and onSubmit have independent issues (as far as I can determine).

1. To resolve the issue on component, the component prop of the PaperProps interface should be changed to include HTMLFormElement as a valid element type. component?: React.ElementType<React.HTMLAttributes<HTMLDivElement | HTMLFormElement>

2. To resolve the issue on onSubmit (which I do not believe comes from Material), the type of onSubmit should be changed from (e: React.FormEvent<HTMLDivElement>) => null | undefined to (e: React.FormEvent<HTMLFormElement>) => null | undefined

I am learning about the underlying structure at the same time I am investigating it, so there may be cases I missed or design decisions I am unaware of, but making the change to Paper.d.ts and forcibly typing the onSubmit function as the presented value resolved both issues.

If there's nothing wrong with 1, I can submit a merge request with the change.

[seamuslowry]

@oliviertassinari I don't think so, such that TypeScript is happy.

Is something like this what you were thinking?

const DialogForm: React.FC<{onSubmit: (e: React.FormEvent<HTMLFormElement>) => void}> = ({onSubmit, children}) => {
  return (
    <form onSubmit={onSubmit}>
      {children}
    </form>
  )
}

Passing that into PaperComponent results in errors. I can pass it in for the component prop in PaperProps, but only if I disable TS for the line

[seamuslowry]

Actually, scratch that last bit about getting the custom Form component to work with PaperProps if I disable TS. I must have tested it before it recompiled. It's not actually working

[oliviertassinari]

~I don't think that https://material-ui.com/components/dialogs/#form-dialogs is following the best practice, I believe it should have a <form> element.~ Fixed in #40470

[DASPRiD]

@oliviertassinari I definitely like the second solution, as one doesn't have to repeat existing CSS. The problem though is that (in the TypeScript definitions) PaperProps does not support form specific attributes like noValidate. For now I solved that by putting a @ts-ignore above it, but I assume this is actually a bug?

[oliviertassinari]

@DASPRiD noValidate should be supported. Looks like an issue.

[DASPRiD]

@oliviertassinari I think the problem is here:

https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Paper/Paper.d.ts#L5

It's using React.HTMLAttributes instead of React.AllHTMLAttributes.

[DASPRiD]

@oliviertassinari Any news on this topic? :)

[Djspaceg]

I have a different solution to this problem, which requires a slightly newer browser, but doesn't inject styling overrides.

      <Dialog>
        <form
          onSubmit={e => {
            alert("form submit!");
            e.preventDefault();
          }}
          style={{
            display: "contents"
          }}
        >
          <DialogTitle
          ...

This allows the inner flex element of Dialog to still be the operator of the flex-box container rules, and allows the children DialogTitle, DialogContent, and DialogActions to retain their flex-box children rules, allowing for correct scrolling.

display: contents; effectively removes the <form> element from the DOM layout, in the eyes of CSS, so the parent and children can interact as if it wasn't even there. As stated before, this does require a slightly newer browser, but that's ok for my case.

Setting component="form" is, in my opinion, the next best option, even though this causes awkward Typescript issues regarding the event handler in my case.

It'd be great to have first-class support for a "FormDialog" component exported out of MaterialUI which did this component-override in a few lines: (with appropriate typescript mappings)

const FormDialog = (props) => <Dialog {...props} component="form" />;

Or at very least, an official example of the "right" way to do this, with proper Typescript definition, etc.

[michael-land]

@Djspaceg Elegant solution. I can now remove all the hacks and do it once in theme level:

    MuiDialog: {
      styleOverrides: {
        paper: {
          '& > form': {
            display: 'contents',
          },
        },
      },
    },

[wescopeland]

Beware: display: 'contents' has a major implementation bug with all versions of Safari causing its child content to be inaccessible by assistive technologies.

We are still seeing this issue in 5.0.0-beta.1.

This code does seem to fix it:

export const Form = styled("form")({
  overflowY: "auto",
  display: "flex",
  flexDirection: "column"
})

[trickreich]

overflow-y: auto on DialogContent is causing this bug... what do you guys do in such situations?