juliancwirko / react-s-alert

Alerts / Notifications for React with rich configuration options
https://www.npmjs.com/package/react-s-alert
MIT License
649 stars 75 forks source link

context not passed correctly to contentTemplate #37

Closed ron23 closed 6 years ago

ron23 commented 7 years ago

I'm using material-ui (http://www.material-ui.com) which is using context to pass theme around to components. I'm also using the contentTemplate and I pass a react component to it. The bug is that this react Component that i'm passing doesn't have access to the context of the app therefore I can't use any material-ui's component in it.

tiny ex:


class myContentTemplate extends React.Component {
static contextTypes = {
 myVal: PropTypes.object
}

 render() {
  this.context.myVal // This will be undefined.
 }
}
juliancwirko commented 7 years ago

Hi, I don't use material-ui at all, so it is hard for me to tell what could be wrong, but I'll try to play with it asap.

EmmaSimon commented 7 years ago

This looks like it means it also won't work with react-router. Either through Link components or withRouter.

EmmaSimon commented 7 years ago

I've figured out a few workarounds for react-router and i18next. For router, you can access history by passing it through customFields, then do whatever with it in the alert template. You can also use Link by passing context through customFields, which may also work with other situations. It's probably overcomplicated for router since you can just push to history, but you need to add childContextTypes to the alert template, and contextTypes to the place where you're calling Alert.whatever('', { context: this.context }). Then in the alert template, getChildContext returns the context from customFields.

For react-i18next (@PeterMerkert), translate is a wrapper, and won't work with the above method because the context is only on the inner alert template component, not the hoc, where i18next needs the context. But luckily, you can pass in your i18n instance into the translate hoc through options, rather than making it rely on context. You must also set wait to false here if it isn't already. Making it wait will mess up react-s-alert when it tries to calculate the height of the alert.

ignatevdev commented 6 years ago

I have tried to understand the root of this problem and actually it took me quite a while to figure out, but I have eventually found the reason of why the context is not passed properly.

If you actually try to log your context, you will see that at first it is undefined, but after a short while you will receive your data. The reason of why this happens is because of how react-s-alert calculates the offset for the next element in the stack.

In lib/s-alert-parts/s-alert-data-prep.js you can see, that it mounts the component inside the body, measures the height and then removes it. Obviously, no context will ever be passed to such component rendered in isolation, thus you get undefined.

I have found a very simple fix to this problem, at least in React 16. You can mount these alerts using ReactDOM.unstable_renderSubtreeIntoContainer instead of ReactDOM.render. unstable_renderSubtreeIntoContainer despite of it's daunting name works absolutely fine and in fact is the only way to mount a component inside DOM and preserve it's context.

I will submit a PR with an option to use unstable_renderSubtreeIntoContainer instead of render, because some people might not want to use an "unstable" feature.

ignatevdev commented 6 years ago

Should be fixed after #58 is merged

juliancwirko commented 6 years ago

merged, thanks