← Back to team overview

yellow team mailing list archive

Re: Implement ambiguous relation menu for adding rels (issue 6736051)

 

Cards created in FrontPage.


https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js#newcode1076
app/views/environment.js:1076: .ambiguousAddRelationTest(endpoint, self,
rect);
On 2012/10/19 21:48:58, hazmat wrote:
> i'd suggest renaming this method, just to avoid the test parlance
outside of
> unit testing. maybe... addRelationEndpointCheck

Done.

https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js#newcode1476
app/views/environment.js:1476: // Add a cancel item.
On 2012/10/19 21:48:58, hazmat wrote:
> The cancel item is better at the end, preferably styled as a dialog
cancel
> button or with red/differentiating text.

Done.

https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js#newcode1477
app/views/environment.js:1477: menu.one('.cancel').on('click',
function(evt) {
On 2012/10/19 21:48:58, hazmat wrote:
> The menu should also cancel on click outside, the same way the invalid
target
> fadeout works.

Done.

https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js#newcode1483
app/views/environment.js:1483: var tr = view.zoom.translate(),
On 2012/10/19 21:48:58, hazmat wrote:
> like the service menu this should also track roughly with the service
container
> on zoom/pan (the menu shouldn't resize though on zoom).

Done.

https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js#newcode1489
app/views/environment.js:1489: // Create a relation with the only
available endpoint.
On 2012/10/19 21:48:58, hazmat wrote:
> i think this is better structured with the second clause upfront with
a return
> at the end of the block, and then the first block can become dedented.

Done.

https://codereview.appspot.com/6736051/

-- 
https://code.launchpad.net/~makyo/juju-gui/ambiguous-relations/+merge/130616
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/ambiguous-relations into lp:juju-gui.


References