-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
Show and hide dialog using props instead of methods? #1682
Comments
I totally agree with you. I myself had to write a wrapper component only to handle porp changes and call |
+1 for this, doing "monkey patching" of the methods to read the props and to change the state of components manually... p.s. amazing job boys!! thank you so much for your work. |
@galexy / @subjectix PRs welcome :-) |
Here is the monkey patching I'm using, if this can help. 'use strict';
const React = require('react');
const PureRenderMixin = require('react/lib/ReactComponentWithPureRenderMixin');
const CanvasDialog = React.createClass({
propTypes: {
children: React.PropTypes.element.isRequired,
show: React.PropTypes.bool.isRequired,
},
mixins: [
PureRenderMixin,
],
componentDidUpdate: function(prevProps) {
const show = this.props.show;
if (prevProps.show !== show) {
const dialog = this.refs.dialog;
// Prevent nested action trigger
setTimeout(() => {
if (show) {
dialog.show();
} else {
dialog.dismiss();
}
}, 0);
}
},
render: function() {
return React.cloneElement(this.props.children, {
ref: 'dialog',
openImmediately: this.props.show,
});
},
});
module.exports = CanvasDialog; |
I think it would be idiomatic react (and flux) to have props on the dialog that determine if the dialog is visible or not.
Thoughts?
The text was updated successfully, but these errors were encountered: