Snap details to react #218
Conversation
www/src/js/views/snap-details.js
Outdated
| var Installer = require('../components/installer.js'); | ||
|
|
||
| function SnapSize(props) { | ||
| var model = props.model; |
There was a problem hiding this comment.
For the functional components.. it's not necessary, but a bit more explicit/nicer to destructure the props into arguments you actually use. So SnapSize({model}). After a bit of reading; I think it's safe to do <SnapSize {...model.attributes} /> and SnapSize({size}). Then the first two lines disappear.
There was a problem hiding this comment.
yes makes total sense, and keeps things tight
www/src/js/views/snap-details.js
Outdated
|
|
||
| module.exports = React.createBackboneClass({ | ||
| componentWillMount: function() { | ||
| var model = this.props.model; |
www/src/js/views/snap-details.js
Outdated
| module.exports = React.createBackboneClass({ | ||
| componentWillMount: function() { | ||
| var model = this.props.model; | ||
| if (! model) { |
There was a problem hiding this comment.
well ... even if trapped above, ... I tend to be very paranoid in js ... there are not real guarantees at the component level that we can put to force a non null model ... which makes me uneasy about removing it?
There was a problem hiding this comment.
I simplified a bit the code for this
www/src/js/views/snap-details.js
Outdated
| return; | ||
| } | ||
|
|
||
| var self = this; |
www/src/js/views/snap-details.js
Outdated
| }, | ||
|
|
||
| onDownloadProgressChanged: function() { | ||
| var model = this.props.model; |
There was a problem hiding this comment.
same pattern, I would simplify at least with something like :
if (this.props.model)
this.setState(...)
www/src/js/views/snap-details.js
Outdated
| }, | ||
|
|
||
| render: function() { | ||
| var model = this.props.model; |
No description provided.