[Dialog] Theme not updating on muiTheme change

[puranjayjain]

Problem description

• The mui theme is passed using context to down the tree.
• It is controlled by a state in the main parent.
• When the state is changed to switch to light theme to dark (or vice-versa).
  It is not changed for the dialog.
  Every other component's theme is updated (as discussed in [List] [Listitem] Theme not updating on muiTheme change #4362).

Steps to reproduce

• Change state controlling the theme
• Theme changed for all components except <Dialog />

Note: I also tried forceUpdate to update it but it doesn't update to the new state

Versions

• Material-UI: 15.0.1
• React: 15.1.0
• Browser: Chrome 50.0.2661.102

[bezrodnov]

Hi guys,

Original issue may be related to the way layer is rendered.
Currently React.createElement(DialogInline, this.props) is used to render the layer.
As per my observation context is not updated in DialogInline in this case after theme change.
Looks like react owner is not derived properly in this case and thus context is not captured during update phase.
I am new to React so I may be wrong with above assumptions about context...

However I was able to find workaround which worked fine for me.
I can see that open property is always passed as true in the Dialog.render function.
As result unrenderLayer function from RenderToLayer.js is not called even if Dialog's open property is changed to false.
Once I passed down the open property to RenderToLayer, newly opened dialog was rendered using new theme.
So if layer is removed from DOM using unrenderLayer and then recreated again using renderLayer - context is in sync with actual theme.

For now I am using monkey patch like this:

MaterialUI.Dialog.prototype.render = function() {
  return React.createElement(MaterialUI.internal.RenderToLayer, { render: this.renderLayer, open: this.props.open, useLayerForClickAway: false })
}

If dialog was opened before theme change with this patch, changes will be reflected only after dialog reopening.

[oliviertassinari]

The core issue seems to be this one: facebook/react#6599.
The workaround proposed by @bezrodnov is pretty brutal. We would basically unmount and mount again.

Could we rather fill the gap by bypassing the context with a property where it's needed?

[puranjayjain]

@oliviertassinari yes I agree with you on that but right now that issue is far from fixed, can we have the workaround in use right now? We could also use props in lieu but I'm not sure exactly what you have in mind. Can you comment on relevant parts of the pr? I'll update it with your suggestions

[oliviertassinari]

@puranjayjain It was simpler for me to submit a fix. You can have a look at #4548.

[puranjayjain]

Thank you so much I tried to solve it the entire day!