Thanks for this. A number of things, some of which will be good talks
for next week. For now I think updating the HTML generation is a good
start and cards for the ux cases mentioned below.

One, we've chosen different paths for self.state and self.set('state',
...). We
should agree to a pattern and stick with it. My feeling is that instance
vars
were fine as we didn't depend on change events from Attribute
manipulation. We
now have enough state on the env graph that I'd like to create
ViewModels
around various operations and bind them to a Scene object. We're
scribbling
scale/zoom/transform, click actions, current actions etc. If we don't
contain
this better its only going to keep getting harder to develop.


UX:

I'd title the menu, it may not be clear to users what is happening w/o
some
'Add Relation...' menu entry as they may not be familiar with ambiguous
relations and their drag action just turned into something else.

Its now possible to have two relation between the same services. These
will
draw in the same visual space and remove relation will need a way to
control
the selection of which relation is being addressed. It would be a very
special
case to detect if there is more than one relation sharing the same
visual
space, I think a better solution is to develop a way that we can show
both
relations exist which is not possible now. We need a card around
addressing
this. The layout changes we plan to make must take this into account.
Additionally w/o the label disambiguation this case still won't be clear
in
many cases.



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

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1463
app/views/environment.js:1463:
endpoints[m.id].forEach(function(endpoint) {
On 2012/10/19 18:09:29, thiago wrote:
> You could use handlebars to build the list for you. For example:

> //remove old list, if any
> menu.all('ul').remove();
> .
> .
> .
> //create new <ul><li></li>...<ul>  list
> var list = Templates.overviewRelationMenuList({
>    endPoints: endpoints
> });

> list.all('li').on('click', ...

> menu.append(list);
> .
> .
> .

> This way you remove some html elements from this js file. wdyt?

Feeding the data to a template is the correct thing to do here. Single
element  DOM node creation is ok, but we avoid generating structures
like this.

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1488
app/views/environment.js:1488: menu.addClass('active');
setStyles can do these in one call. It does seem like we should be
adding support for scale/translate to Box and be using that for
calculations like this. Even though this is an HTML element we should
still be able to wrap it in a box

box.scaledPosition(scale, translate) -> {x, y, w, h} as with .pos today

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1495
app/views/environment.js:1495: role: 'server' }],
I think you're right and we sort the endpoints, but we need to document
were taking advantage of this here, we shouldn't get client/server
wrong.

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1525
app/views/environment.js:1525: if (endpoints[0][0] === endpoints[1][0])
{
This is a better check, /me duhs.

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