Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dialogue component doesn't size to width specified by user #3675

Closed
niole opened this issue Mar 13, 2016 · 8 comments
Closed

Dialogue component doesn't size to width specified by user #3675

niole opened this issue Mar 13, 2016 · 8 comments
Labels
component: dialog This is the name of the generic UI component, not the React module! package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. v0.x

Comments

@niole
Copy link

niole commented Mar 13, 2016

Problem Description

When I specify a width for the Dialogue component, it doesn't size to the width I want, but 75% percent of that. Also, if I specify a width, the component does not center itself accordingly. I am specifying the width via the style attribute e.g.:

<Dialogue
   style={{ width: 500 }}
/>

I realize that this style attribute corresponds more to the width of window.

As the background behind the modal always grays out the entire screen and not only the user specified width, it doesn't make sense for the style props's width key to correspond to the width of the window.

I find this unintuitive and think the usability of this component could be revised so that it corresponds more to how a user would approach it's customization.

I would include a screen shot, but I'm working under NDA.

Versions

  • Material-UI: 0.14.4
  • React: 0.14.7
  • Browser: Chrome
@alitaheri
Copy link
Member

@niole you should use contentStyle for that and also set maxWidth to none, take a look at the Styled Dialog example here

@niole
Copy link
Author

niole commented Mar 13, 2016

Thanks for the reply. Is there a reason for the dialog having such specific functionality? Do you think it would make more sense to have width refer to dialog's inner panel and have the more specifically named width prop for the outer part?

@alitaheri
Copy link
Member

@niole It was like that once, I added that prop and then we saw the complexity it would introduce so we deprecated it and added an example to demonstrate how it should be done. the complexity comes from maxWidth. There are no good ways to have props controlling all that without over complicating the component.

@niole
Copy link
Author

niole commented Mar 14, 2016

Speaking of overcomplication, I have also been experimenting with the
buttons. They look great, but seem to hinder performance in highly
interactive UIs. Is it just me or can you think of any reason why that
would happen?
On Mar 13, 2016 11:52 PM, "Ali Taheri Moghaddar" notifications@github.com
wrote:

@niole https://github.com/niole It was like that once, I added that
prop and then we saw the complexity it would introduce so we deprecated it
and added an example to demonstrate how it should be done. the complexity
comes from maxWidth. There are no good ways to have props controlling all
that without over complicating the component.


Reply to this email directly or view it on GitHub
#3675 (comment)
.

@alitaheri
Copy link
Member

@niole No all our components are slow right now. Because they have heavy render functions and they don't implement shouldComponentUpdate. We plan on changing all that so they become faster. But for now you can wrap our components and implement shouldComponentUpdate yourself. We can't do it yet because we have other plan that would make the implementation unnecessary.

@niole
Copy link
Author

niole commented Mar 15, 2016

No kidding. My first instinct would be to use shouldComponentUpdate to save performance. What else would you do to save performance in a react component?

@alitaheri
Copy link
Member

@niole shouldComponentUpdate is useless ( if not harmful ) with props that always change with each render. The perfromance problem is our render functions too many calls to Object.assign which performs one of the slowest operations in javascript ( object key iteration ). So we should fix that if we hope to improve performance, not just another function that does the same thing only to always return true.

@niole
Copy link
Author

niole commented Mar 16, 2016

This has been really informative! Closing ...

@niole niole closed this as completed Mar 16, 2016
@oliviertassinari oliviertassinari added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 21, 2022
@zannager zannager added component: dialog This is the name of the generic UI component, not the React module! v0.x package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. v0.x
Projects
None yet
Development

No branches or pull requests

4 participants