Convert request-series viewlet into a Y.View #146
Conversation
the request series viewlet manager instantition code.
| name: file.name, | ||
| size: file.size, | ||
| defaultSeries: env.get('defaultSeries') | ||
| requestSeries: new Y.juju.viewlets.RequestSeries({ |
There was a problem hiding this comment.
Because this is only a single view we don't need to have a shared model
There was a problem hiding this comment.
but what about destroying it later?
There was a problem hiding this comment.
When the viewlet manager is destroyed it cleans up all of it's views and viewlet events https://github.com/juju/juju-gui/blob/develop/app/views/viewlet-manager.js#L691
|
Test PASSed. |
| size: file.size, | ||
| defaultSeries: this.get('env').get('defaultSeries') | ||
| })); | ||
| // So that we can call render multiple times. |
There was a problem hiding this comment.
the comment is that we don't call render multiple times but do you mean _bindUI? How would render get called multiple times? Can the events in bindUI just be normal YUI view events {} object?
There was a problem hiding this comment.
To the first part: just a precaution so that we don't get double events and have to track down some obscure bug.
To the second: See the function details below....which you already did :)
|
👍 just would like to see where the View gets cleaned up. I'd also love to see the View destructor make sure that the viewlet manager is also destroyed as a matter of course so that if the button isn't directly clicked we can still be sure clean up is done. |
Thanks a lot for the review!
|
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge |
__To QA__ Drag a local charm to the canvas, you should be able to deploy it, and cancel the deploy as before.
To QA
Drag a local charm to the canvas, you should be able to deploy it, and cancel the deploy as before.