Material-ui: Broken example for Dialogs on material-ui-next.com beta 21

[jamesst20]

Hi,

Just leaving a bug report here for this URL: https://material-ui-next.com/demos/dialogs/#confirmation-dialogs

Click on the "Phone ringtone" item, and you will notice that the dialog is not coming out properly: https://i.imgur.com/R34PdlR.png

I did not test the code at all, I have only clicked on the example there.

Thank you

[vladimirpekez]

Just stumbled onto this as well on Chrome on Win10

[kgregory]

Probably due to maxWidth="xs" and the width of xs being set to 1px

[kgregory]

The problem is with the implementation of the width styles in Dialog.js:

  paperWidthXs: {
    maxWidth: theme.breakpoints.values.xs,
  },
  paperWidthSm: {
    maxWidth: theme.breakpoints.values.sm,
  },
  paperWidthMd: {
    maxWidth: theme.breakpoints.values.md,
  },

In createBreakpoints, xs is defined: theme.breakpoints.values.xs = 1

If the intent is to ensure that the dialog stays within the bounds of the breakpoints, maybe we need to do something like this:

  paperWidthXs: {
    maxWidth: theme.breakpoints.values.sm - 1,
  },
  paperWidthSm: {
    maxWidth: theme.breakpoints.values.md - 1,
  },
  paperWidthMd: {
    maxWidth: theme.breakpoints.values.lg - 1,
  },

[oliviertassinari]

This is a regression coming from https://github.com/callemall/material-ui/pull/9078.
I have normalized the breakpoint behavior with the PR. The value now always correspond to the start of the breakpoint. It's an inclusive start and an exclusive end with the next breakpoint value. So a given breakpoint with index i will match the following interval: [breakpoint(i), breakpoint(i+1)[.

But, we use the breakpoint value directly in the Dialog styles:
https://github.com/callemall/material-ui/blob/9322a5125ee4ec9f9f9b9f6740352901d09d0af0/src/Dialog/Dialog.js#L33

I think that we should be using maxWidth: 360 here.

[mbrookes]

@oliviertassinari That would remove the ability for users to set the breakpoints in the theme, and have the Dialog maxWidth values conform to them; however I"m wondering if maxWidth should instead simply accept a numeric value?

[oliviertassinari]

Dialogs may expand proportionally with content or in specific increments.

From the specification

@mbrookes I have found an interesting tradeoff with:

  paperWidthXs: {
    maxWidth: Math.max(theme.breakpoints.values.xs, 360),
  },

[stunaz]

This is breaking in 1.0.0-beta.21, when the 1.0.0-beta.22 is scheduled for?

[stunaz]

And the solution in https://github.com/mui-org/material-ui/pull/9078 is a littler counter intuitive.
@oliviertassinari what about

values = {
      xss: 1,
      xs: 360,
      sm: 600,
      md: 960,
      lg: 1280,
      xl: 1920,
    }