Merge lp:~makyo/juju-gui/ambiguous-relations into lp:juju-gui/experimental

Proposed by Madison Scott-Clary
Status: Merged
Approved by: Kapil Thangavelu
Approved revision: 200
Merged at revision: 199
Proposed branch: lp:~makyo/juju-gui/ambiguous-relations
Merge into: lp:juju-gui/experimental
Diff against target: 346 lines (+153/-34)
8 files modified
app/templates/ambiguousRelationList.handlebars (+12/-0)
app/templates/overview.handlebars (+6/-2)
app/views/environment.js (+84/-15)
app/views/utils.js (+11/-0)
lib/views/stylesheet.less (+2/-2)
test/test_application_notifications.js (+5/-1)
test/test_charm_configuration.js (+2/-1)
test/test_environment_view.js (+31/-13)
To merge this branch: bzr merge lp:~makyo/juju-gui/ambiguous-relations
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+130616@code.launchpad.net

Description of the change

Implement ambiguous relation menu for adding rels

Displays a menu of choices when adding a relation between two services would result in ambiguity, otherwise simply adds a relation. Also, a bit of minor test clean-up in charm config, due to floating point inequality.

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

To post a comment you must log in.
Revision history for this message
Madison Scott-Clary (makyo) wrote :

Reviewers: mp+130616_code.launchpad.net,

Message:
Please take a look.

Description:
Implement ambiguous relation menu for adding rels

Displays a menu of choices when adding a relation between two services
would result in ambiguity, otherwise simply adds a relation. Also, a
bit of minor test clean-up in charm config, due to floating point
inequality.

https://code.launchpad.net/~makyo/juju-gui/ambiguous-relations/+merge/130616

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6736051/

Affected files:
   A [revision details]
   M app/templates/overview.handlebars
   M app/views/environment.js
   M lib/views/stylesheet.less
   M test/test_application_notifications.js
   M test/test_charm_configuration.js
   M test/test_environment_view.js

Revision history for this message
Thiago Veronezi (tveronezi) wrote :

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#newcode1449
app/views/environment.js:1449: var menu =
container.one('#ambiguous-relation-menu'),
If you are using ids you could simply use Y.one, right? Or you could use
a class instead.

https://codereview.appspot.com/6736051/diff/1/app/views/environment.js#newcode1463
app/views/environment.js:1463:
endpoints[m.id].forEach(function(endpoint) {
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?

https://codereview.appspot.com/6736051/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6736051/diff/1/lib/views/stylesheet.less#newcode145
lib/views/stylesheet.less:145: #service-menu, #ambiguous-relation-menu {
You could remove the id and use both classes in your menu. Like...

<div class="service-menu ambiguous-relation-menu">
   <div class="triangle">&nbsp;</div>
     <ul/>
   </div>
</div>

So you can remove this new css declaration and do something like...

var menu = container.one('.ambiguous-relation-menu')

...in environment.js

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

Revision history for this message
Madison Scott-Clary (makyo) wrote :

Thanks for the review, Thiago. See below for notes.

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#newcode1449
app/views/environment.js:1449: var menu =
container.one('#ambiguous-relation-menu'),
On 2012/10/19 18:09:29, thiago wrote:
> If you are using ids you could simply use Y.one, right? Or you could
use a class
> instead.

I don't think that using a class would buy me anything here. The
ambiguous relation menu is unique, and referred to by a unique id.
Convention in the past has been to access nodes through container.

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?

Done. I initially couldn't figure out how I'd do it with Handlebars
given that the click action references each individual endpoint set, but
I did it with node data.

https://codereview.appspot.com/6736051/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6736051/diff/1/lib/views/stylesheet.less#newcode145
lib/views/stylesheet.less:145: #service-menu, #ambiguous-relation-menu {
On 2012/10/19 18:09:29, thiago wrote:
> You could remove the id and use both classes in your menu. Like...

> <div class="service-menu ambiguous-relation-menu">
> <div class="triangle">&nbsp;</div>
> <ul/>
> </div>
> </div>

> So you can remove this new css declaration and do something like...

> var menu = container.one('.ambiguous-relation-menu')

> ...in environment.js

Done. Both #service-menu and #ambiguous-relation-menu are unique and
must be referred to that way because of the way they're controlled in
the view. However, I have added a 'environment-menu' class to each, and
then used that as the selector here.

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

198. By Madison Scott-Clary

Implement ambiguous relation menu for adding relations

199. By Madison Scott-Clary

Implement ambiguous relation menu for adding relations

200. By Madison Scott-Clary

Implement ambiguous relation menu for adding relations

Revision history for this message
Madison Scott-Clary (makyo) wrote :
Revision history for this message
Benjamin Saller (bcsaller) wrote :
Download full text (3.3 KiB)

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/envi...

Read more...

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

Looks pretty good. some minors below. could you a new card for the menu
scroll and positioning (ie. if too close to an edge, display on opposite
side).

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);
i'd suggest renaming this method, just to avoid the test parlance
outside of unit testing. maybe... addRelationEndpointCheck

https://codereview.appspot.com/6736051/diff/3002/app/views/environment.js#newcode1476
app/views/environment.js:1476: // Add a cancel item.
The cancel item is better at the end, preferably styled as a dialog
cancel button or with red/differentiating text.

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

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

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.
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.

https://codereview.appspot.com/6736051/diff/3002/test/test_environment_view.js
File test/test_environment_view.js (right):

https://codereview.appspot.com/6736051/diff/3002/test/test_environment_view.js#newcode309
test/test_environment_view.js:309: add_rel =
container.one('#service-menu .add-relation'),
indent looks a little off here.

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

Revision history for this message
Madison Scott-Clary (makyo) wrote :

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/

Revision history for this message
Madison Scott-Clary (makyo) wrote :

*** Submitted:

Implement ambiguous relation menu for adding rels

Displays a menu of choices when adding a relation between two services
would result in ambiguity, otherwise simply adds a relation. Also, a
bit of minor test clean-up in charm config, due to floating point
inequality.

R=thiago, benjamin.saller, hazmat
CC=
https://codereview.appspot.com/6736051

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

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

There's still some minors in the this branch fwiw.. Relation cancel text
should read 'Cancel' not 'None'. The pending relation line should continue
to be displayed while the pending display menu is active. The relation
pairs shoulds shown should be sorted (deterministic order), and there's no
need to indent the pairs.

-k

On Sat, Oct 20, 2012 at 8:21 PM, Matthew Scott
<email address hidden>wrote:

> *** Submitted:
>
> Implement ambiguous relation menu for adding rels
>
> Displays a menu of choices when adding a relation between two services
> would result in ambiguity, otherwise simply adds a relation. Also, a
> bit of minor test clean-up in charm config, due to floating point
> inequality.
>
> R=thiago, benjamin.saller, hazmat
> CC=
> https://codereview.appspot.com/6736051
>
>
> 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.
>

Revision history for this message
Madison Scott-Clary (makyo) wrote :

These should be separate cards if they did not make it into the review. I'm shut down for the night, I'll see about making them tomorrow prior to travel.

~Matthew Scott

PGP key: http://drab-makyo.com/pgp/

(Sent from my phone, responses may be delayed; forgive any typing errors)

On Oct 21, 2012, at 6:13 PM, Kapil Thangavelu <email address hidden> wrote:

> There's still some minors in the this branch fwiw.. Relation cancel text
> should read 'Cancel' not 'None'. The pending relation line should continue
> to be displayed while the pending display menu is active. The relation
> pairs shoulds shown should be sorted (deterministic order), and there's no
> need to indent the pairs.
>
> -k
>
>
> On Sat, Oct 20, 2012 at 8:21 PM, Matthew Scott
> <email address hidden>wrote:
>
>> *** Submitted:
>>
>> Implement ambiguous relation menu for adding rels
>>
>> Displays a menu of choices when adding a relation between two services
>> would result in ambiguity, otherwise simply adds a relation. Also, a
>> bit of minor test clean-up in charm config, due to floating point
>> inequality.
>>
>> R=thiago, benjamin.saller, hazmat
>> CC=
>> https://codereview.appspot.com/6736051
>>
>>
>> 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.
>
> --
> https://code.launchpad.net/~makyo/juju-gui/ambiguous-relations/+merge/130616
> You are the owner of lp:~makyo/juju-gui/ambiguous-relations.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'app/templates/ambiguousRelationList.handlebars'
2--- app/templates/ambiguousRelationList.handlebars 1970-01-01 00:00:00 +0000
3+++ app/templates/ambiguousRelationList.handlebars 2012-10-19 19:19:23 +0000
4@@ -0,0 +1,12 @@
5+<ul>
6+ <li class="cancel">None</li>
7+ {{#relationslist endpoints}}
8+ <li
9+ data-startservice="{{start.service}}"
10+ data-startname="{{start.name}}"
11+ data-endservice="{{end.service}}"
12+ data-endname="{{end.name}}">
13+ {{start.service}}:{{start.name}} &rarr; {{end.service}}:{{end.name}}
14+ </li>
15+ {{/relationslist}}
16+</ul>
17
18=== modified file 'app/templates/overview.handlebars'
19--- app/templates/overview.handlebars 2012-10-11 10:49:21 +0000
20+++ app/templates/overview.handlebars 2012-10-19 19:19:23 +0000
21@@ -1,7 +1,7 @@
22 <div id="overview">
23 <div id="canvas" class="crosshatch-background">
24- <div id="service-menu">
25- <div id="service-menu-triangle">&nbsp;</div>
26+ <div class="environment-menu" id="service-menu">
27+ <div class="triangle">&nbsp;</div>
28 <ul>
29 <li class="view-service">View</li>
30 <li class="add-relation">Build Relation</li>
31@@ -9,6 +9,10 @@
32 <li class="destroy-service">Destroy</li>
33 </ul>
34 </div>
35+ <div class="environment-menu" id="ambiguous-relation-menu">
36+ <div class="triangle">&nbsp;</div>
37+ <ul/>
38+ </div>
39 </div>
40 <div id="overview-tasks">
41 <div class="controls yui3-skin-sam">
42
43=== modified file 'app/views/environment.js'
44--- app/views/environment.js 2012-10-18 17:52:44 +0000
45+++ app/views/environment.js 2012-10-19 19:19:23 +0000
46@@ -1016,7 +1016,7 @@
47 if (curr_action === 'show_service') {
48 this.set('currentServiceClickAction', 'addRelationStart');
49 } else if (curr_action === 'addRelationStart' ||
50- curr_action === 'addRelationEnd') {
51+ curr_action === 'ambiguousAddRelationTest') {
52 this.set('currentServiceClickAction', 'toggleControlPanel');
53 } // Otherwise do nothing.
54 },
55@@ -1073,7 +1073,7 @@
56 // If we landed on a rect, add relation, otherwise, cancel.
57 if (rect) {
58 self.service_click_actions
59- .addRelationEnd(endpoint, self, rect);
60+ .ambiguousAddRelationTest(endpoint, self, rect);
61 } else {
62 // TODO clean up, abstract
63 self.cancelRelationBuild();
64@@ -1395,15 +1395,16 @@
65 var db = view.get('db'),
66 app = view.get('app'),
67 service = view.serviceForBox(m),
68+ endpoints = models.getEndpoints(
69+ service, app.serviceEndpoints, db),
70
71 /* Transform endpoints into a list of
72 * relatable services (to the service in m)
73 */
74 possible_relations = Y.Array.map(
75 Y.Array.flatten(Y.Object.values(
76- models.getEndpoints(
77- service, app.serviceEndpoints, db))),
78- function(ep) {return ep.service;}),
79+ endpoints)),
80+ function(ep) {return ep.service;}),
81 invalidRelationTargets = {};
82
83 // Iterate services and invert the possibles list.
84@@ -1430,15 +1431,86 @@
85
86 // Store start service in attrs.
87 view.set('addRelationStart_service', m);
88+ // Store possible endpoints.
89+ view.set('addRelationStart_possibleEndpoints', endpoints);
90 // Set click action.
91- view.set('currentServiceClickAction', 'addRelationEnd');
92+ view.set('currentServiceClickAction', 'ambiguousAddRelationTest');
93+ },
94+
95+ /*
96+ * Test if the pending relation is ambiguous. Display a menu if so,
97+ * create the relation if not.
98+ */
99+ ambiguousAddRelationTest: function(m, view, context) {
100+ var endpoints = view.get('addRelationStart_possibleEndpoints'),
101+ container = view.get('container');
102+ if (endpoints[m.id].length > 1) {
103+ // Display menu with available endpoints.
104+ var menu = container.one('#ambiguous-relation-menu'),
105+ list = menu.one('ul');
106+ list.empty();
107+ menu.one('ul').remove(true);
108+
109+ menu.append(Templates
110+ .ambiguousRelationList({endpoints: endpoints[m.id]}));
111+
112+ // For each endpoint choice, bind an an event to 'click' to
113+ // add the specified relation.
114+ menu.all('li').on('click', function(evt) {
115+ if (evt.currentTarget.hasClass('cancel')) {
116+ return;
117+ }
118+ var el = evt.currentTarget,
119+ endpoints_item = [
120+ [el.getData('startservice'), {
121+ name: el.getData('startname'),
122+ role: 'server' }],
123+ [el.getData('endservice'), {
124+ name: el.getData('endname'),
125+ role: 'client' }]];
126+ menu.removeClass('active');
127+ view.service_click_actions
128+ .addRelationEnd(endpoints_item, view, context);
129+ });
130+
131+ // Add a cancel item.
132+ menu.one('.cancel').on('click', function(evt) {
133+ menu.removeClass('active');
134+ view.cancelRelationBuild();
135+ });
136+
137+ // Display the menu at the service endpoint.
138+ var tr = view.zoom.translate(),
139+ z = view.zoom.scale();
140+ menu.setStyle('top', m.y * z + tr[1]);
141+ menu.setStyle('left', m.x * z + m.w * z + tr[0]);
142+ menu.addClass('active');
143+ } else {
144+ // Create a relation with the only available endpoint.
145+ var ep = endpoints[m.id][0],
146+ endpoints_item = [
147+ [ep[0].service, {
148+ name: ep[0].name,
149+ role: 'server' }],
150+ [ep[1].service, {
151+ name: ep[1].name,
152+ role: 'client' }]];
153+ view.service_click_actions
154+ .addRelationEnd(endpoints_item, view, context);
155+ }
156 },
157
158 /*
159 * Fired when clicking the second service is clicked in the
160 * add relation flow.
161+ *
162+ * :param endpoints: array of two endpoints, each in the form
163+ * ['service name', {
164+ * name: 'endpoint type',
165+ * role: 'client or server'
166+ * }]
167 */
168- addRelationEnd: function(m, view, context) {
169+ addRelationEnd: function(endpoints, view, context) {
170 // Redisplay all services
171 view.cancelRelationBuild();
172
173@@ -1447,9 +1519,9 @@
174 env = view.get('env'),
175 db = view.get('db'),
176 source = view.get('addRelationStart_service'),
177- relation_id = 'pending:' + source.id + m.id;
178+ relation_id = 'pending:' + endpoints[0][0] + endpoints[1][0];
179
180- if (m.id === source.id) {
181+ if (endpoints[0][0] === endpoints[1][0]) {
182 view.set('currentServiceClickAction', 'toggleControlPanel');
183 return;
184 }
185@@ -1459,10 +1531,7 @@
186 db.relations.create({
187 relation_id: relation_id,
188 display_name: 'pending',
189- endpoints: [
190- [source.id, {name: 'pending', role: 'server'}],
191- [m.id, {name: 'pending', role: 'client'}]
192- ],
193+ endpoints: endpoints,
194 pending: true
195 });
196
197@@ -1473,8 +1542,8 @@
198 // Fire event to add relation in juju.
199 // This needs to specify interface in the future.
200 env.add_relation(
201- source.id,
202- m.id,
203+ endpoints[0][0] + ':' + endpoints[0][1].name,
204+ endpoints[1][0] + ':' + endpoints[1][1].name,
205 Y.bind(this._addRelationCallback, this, view, relation_id)
206 );
207 view.set('currentServiceClickAction', 'toggleControlPanel');
208
209=== modified file 'app/views/utils.js'
210--- app/views/utils.js 2012-10-18 20:19:26 +0000
211+++ app/views/utils.js 2012-10-19 19:19:23 +0000
212@@ -852,6 +852,17 @@
213 Y.Markdown.toHTML(text));
214 });
215
216+ /*
217+ * Build a list of relation types given a list of endpoints.
218+ */
219+ Y.Handlebars.registerHelper('relationslist', function(endpoints, options) {
220+ var out = '';
221+ endpoints.forEach(function(ep) {
222+ out += options.fn({start: ep[0], end: ep[1]});
223+ });
224+ return out;
225+ });
226+
227 }, '0.1.0', {
228 requires: ['base-build',
229 'handlebars',
230
231=== modified file 'lib/views/stylesheet.less'
232--- lib/views/stylesheet.less 2012-10-18 17:50:22 +0000
233+++ lib/views/stylesheet.less 2012-10-19 19:19:23 +0000
234@@ -142,7 +142,7 @@
235 }
236
237
238-#service-menu {
239+.environment-menu {
240 @border_radius: 20px;
241 @background_color: #282421;
242
243@@ -166,7 +166,7 @@
244 display: block;
245 }
246
247- #service-menu-triangle {
248+ .triangle {
249 position: absolute;
250 top: 62px;
251 left: -12px;
252
253=== modified file 'test/test_application_notifications.js'
254--- test/test_application_notifications.js 2012-10-15 18:18:52 +0000
255+++ test/test_application_notifications.js 2012-10-19 19:19:23 +0000
256@@ -310,7 +310,11 @@
257 };
258
259 views.environment.prototype.service_click_actions.addRelationEnd
260- .apply(view, [{id: 1}, view]);
261+ .apply(view, [
262+ [
263+ ['s1', {name: 'n', role: 'client'}],
264+ ['s2', {name: 'n', role: 'server'}]],
265+ view]);
266
267 assertNotificationNumber('1');
268
269
270=== modified file 'test/test_charm_configuration.js'
271--- test/test_charm_configuration.js 2012-10-18 00:27:33 +0000
272+++ test/test_charm_configuration.js 2012-10-19 19:19:23 +0000
273@@ -232,7 +232,8 @@
274 // The simulate module does not support firing scroll events so we call
275 // the associated method directly.
276 view._moveTooltip();
277- tooltip.get('boundingBox').getY().should.equal(originalY - 10);
278+ Math.floor(tooltip.get('boundingBox').getY())
279+ .should.equal(Math.floor(originalY - 10));
280 });
281
282 it('must hide the tooltip when its field scrolls away', function() {
283
284=== modified file 'test/test_environment_view.js'
285--- test/test_environment_view.js 2012-10-15 10:07:49 +0000
286+++ test/test_environment_view.js 2012-10-19 19:19:23 +0000
287@@ -299,28 +299,46 @@
288
289 it('must be able to add a relation from the control panel',
290 function() {
291+ var view = new views.environment({
292+ container: container,
293+ app: {serviceEndpoints: {}},
294+ db: db,
295+ env: env
296+ }).render();
297+ var service = container.one('.service'),
298+ add_rel = container.one('#service-menu .add-relation'),
299+ after_evt;
300+
301 // Mock endpoints
302 var existing = models.getEndpoints;
303 models.getEndpoints = function() {
304- return {requires: [],
305- provides: []};
306+ var endpoints = {},
307+ serviceName = service.one('.name')
308+ .getDOMNode().firstChild.nodeValue,
309+ nextServiceName = service.next().one('.name')
310+ .getDOMNode().firstChild.nodeValue;
311+ endpoints[nextServiceName] = [
312+ [
313+ {
314+ service: serviceName,
315+ name: 'relName',
316+ type: 'relType'
317+ },
318+ {
319+ service: nextServiceName,
320+ name: 'relName',
321+ type: 'relType'
322+ }
323+ ]
324+ ];
325+ return endpoints;
326 };
327
328- var view = new views.environment({
329- container: container,
330- app: {serviceEndpoints: {}},
331- db: db,
332- env: env
333- }).render();
334- var service = container.one('.service'),
335- add_rel = container.one('#service-menu .add-relation'),
336- after_evt;
337-
338 service.simulate('click');
339 add_rel.simulate('click');
340 container.all('.selectable-service')
341 .size()
342- .should.equal(1);
343+ .should.equal(2);
344 service.next().simulate('click');
345 container.all('.selectable-service').size()
346 .should.equal(0);

Subscribers

People subscribed via source and target branches