Implement basics for ServiceUnit token/view. #239
Conversation
|
Test FAILed. |
|
Test PASSed. |
| var container, utils, models, views, view, id, title, Y; | ||
|
|
||
| before(function(done) { | ||
| Y = YUI(GlobalConfig).use(['juju-views', |
There was a problem hiding this comment.
Why load in all of the views for this one suite? You should just load in the files you need and are testing.
|
Thanks for this branch! I'd be interested to know if the patch solves the test failure issue. Please let me know if you have any questions about the comments above. |
|
Test PASSed. |
| <div class="icons"> | ||
| <a href class="move"><i class="sprite token-move"></i></a> | ||
| </div> | ||
| <div class="machines" style="display: none;"> |
There was a problem hiding this comment.
I'd expect this to be dynamic html. This is ok here, but let's add an XXX to remind us to refactor this out. This machine list will change as you interact with machine view and would need to be rerendered as you clicked on the button.
|
I'd like to chat on this branch. I feel like there's a lot of overlap with Huw's work and getting outside the scope of being an unplaced unit token. |
|
Test PASSed. |
|
Test PASSed. |
| machines: this.get('machines').filterByParent(null), | ||
| containers: [] // XXX Need to find query for getting containers | ||
| }).render(); | ||
| unitList.append(node); |
There was a problem hiding this comment.
this should only happen after hte loop is done. Create a list and append them in one swoop. Hitting the dom for each node created will slow things down.
|
Couple of small bits of feedback on the latest set and then 👍 to land. Let's make sure we have cards on the board to update a lot of the missing bits in here. |
|
Test PASSed. |
|
Thanks all for bearing with me through the branch. |
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge |
Right now primarily a code review as this isn't wired into anything yet.
Right now primarily a code review as this isn't wired into anything yet.