Merge lp:~gary/juju-gui/styling-environment-menu into lp:juju-gui/experimental

Proposed by Gary Poster
Status: Merged
Merged at revision: 882
Proposed branch: lp:~gary/juju-gui/styling-environment-menu
Merge into: lp:juju-gui/experimental
Diff against target: 208 lines (+55/-34)
2 files modified
app/views/topology/service.js (+28/-11)
lib/views/stylesheet.less (+27/-23)
To merge this branch: bzr merge lp:~gary/juju-gui/styling-environment-menu
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+175973@code.launchpad.net

Description of the change

Update service menus and enable for inspector

One of the last big issues remaining in the UX design to make the inspector usable was to make a disoverable way of creating relations. Luca and Ant worked together to revamp the existing popup menu for the purpose. This branch takes their design work and makes it work for both inspector and non-inspector uses. In order to have it work well, I had to update some of the calculations that determined where the menu was drawn.

https://codereview.appspot.com/11620043/

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :
Download full text (9.5 KiB)

Reviewers: mp+175973_code.launchpad.net,

Message:
Please take a look.

Description:
Update service menus and enable for inspector

One of the last big issues remaining in the UX design to make the
inspector usable was to make a disoverable way of creating relations.
Luca and Ant worked together to revamp the existing popup menu for the
purpose. This branch takes their design work and makes it work for both
inspector and non-inspector uses. In order to have it work well, I had
to update some of the calculations that determined where the menu was
drawn.

https://code.launchpad.net/~gary/juju-gui/styling-environment-menu/+merge/175973

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/assets/images/icons/icon_noshadow_destroy.png
   M app/assets/images/icons/icon_noshadow_relation.png
   M app/assets/images/icons/icon_noshadow_view.png
   M app/assets/images/landscape_restart_menu.png
   M app/assets/images/landscape_security_menu.png
   M app/views/topology/service.js
   M lib/views/stylesheet.less

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/assets/images/icons/icon_noshadow_destroy.png
=== modified file 'app/assets/images/icons/icon_noshadow_destroy.png'
Binary files app/assets/images/icons/icon_noshadow_destroy.png 2012-09-20
15:53:46 +0000 and app/assets/images/icons/icon_noshadow_destroy.png
2013-07-19 16:03:37 +0000 differ

Index: app/assets/images/icons/icon_noshadow_relation.png
=== modified file 'app/assets/images/icons/icon_noshadow_relation.png'
Binary files app/assets/images/icons/icon_noshadow_relation.png 2012-09-20
15:53:46 +0000 and app/assets/images/icons/icon_noshadow_relation.png
2013-07-19 16:03:37 +0000 differ

Index: app/assets/images/icons/icon_noshadow_view.png
=== modified file 'app/assets/images/icons/icon_noshadow_view.png'
Binary files app/assets/images/icons/icon_noshadow_view.png 2012-09-20
15:53:46 +0000 and app/assets/images/icons/icon_noshadow_view.png
2013-07-20 01:15:05 +0000 differ

Index: app/assets/images/landscape_restart_menu.png
=== modified file 'app/assets/images/landscape_restart_menu.png'
Binary files app/assets/images/landscape_restart_menu.png 2013-02-27
14:49:14 +0000 and app/assets/images/landscape_restart_menu.png 2013-07-20
01:15:05 +0000 differ

Index: app/assets/images/landscape_security_menu.png
=== modified file 'app/assets/images/landscape_security_menu.png'
Binary files app/assets/images/landscape_security_menu.png 2013-03-26
21:13:04 +0000 and app/assets/images/landscape_security_menu.png 2013-07-20
01:15:05 +0000 differ

Index: app/views/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-07-16 19:10:36 +0000
+++ app/views/topology/service.js 2013-07-20 01:19:53 +0000
@@ -1196,12 +1196,14 @@
            z = topo...

Read more...

Revision history for this message
Brad Crittenden (bac) wrote :

LGTM with a few nits. QA was fine. Thanks!

https://codereview.appspot.com/11620043/diff/1/app/assets/images/icons/icon_noshadow_destroy.png
File app/assets/images/icons/icon_noshadow_destroy.png (right):

https://codereview.appspot.com/11620043/diff/1/app/assets/images/icons/icon_noshadow_destroy.png#newcode6
app/assets/images/icons/icon_noshadow_destroy.png:6:
ð%—^éµß÷¾{w¼“@¹\î/Yl÷Øþ°}cëv:±Èw%âg¼¼`»ƒÓ@(äݖ

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Update service menus and enable for inspector

One of the last big issues remaining in the UX design to make the
inspector usable was to make a disoverable way of creating relations.
Luca and Ant worked together to revamp the existing popup menu for the
purpose. This branch takes their design work and makes it work for both
inspector and non-inspector uses. In order to have it work well, I had
to update some of the calculations that determined where the menu was
drawn.

R=benji, bac
CC=
https://codereview.appspot.com/11620043

https://codereview.appspot.com/11620043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/assets/images/icons/icon_noshadow_destroy.png'
2Binary files app/assets/images/icons/icon_noshadow_destroy.png 2012-09-20 15:53:46 +0000 and app/assets/images/icons/icon_noshadow_destroy.png 2013-07-20 01:26:26 +0000 differ
3=== modified file 'app/assets/images/icons/icon_noshadow_relation.png'
4Binary files app/assets/images/icons/icon_noshadow_relation.png 2012-09-20 15:53:46 +0000 and app/assets/images/icons/icon_noshadow_relation.png 2013-07-20 01:26:26 +0000 differ
5=== modified file 'app/assets/images/icons/icon_noshadow_view.png'
6Binary files app/assets/images/icons/icon_noshadow_view.png 2012-09-20 15:53:46 +0000 and app/assets/images/icons/icon_noshadow_view.png 2013-07-20 01:26:26 +0000 differ
7=== modified file 'app/assets/images/landscape_restart_menu.png'
8Binary files app/assets/images/landscape_restart_menu.png 2013-02-27 14:49:14 +0000 and app/assets/images/landscape_restart_menu.png 2013-07-20 01:26:26 +0000 differ
9=== modified file 'app/assets/images/landscape_security_menu.png'
10Binary files app/assets/images/landscape_security_menu.png 2013-03-26 21:13:04 +0000 and app/assets/images/landscape_security_menu.png 2013-07-20 01:26:26 +0000 differ
11=== modified file 'app/views/topology/service.js'
12--- app/views/topology/service.js 2013-07-16 19:10:36 +0000
13+++ app/views/topology/service.js 2013-07-20 01:26:26 +0000
14@@ -1196,12 +1196,14 @@
15 z = topo.get('scale');
16
17 if (service && cp) {
18- var cp_width = cp.getDOMNode().getClientRects()[0].width,
19- menu_left = (service.x * z + service.w * z / 2 <
20- topo.get('width') * z / 2),
21- service_center = service.relativeCenter;
22+ var cpRect = cp.getDOMNode().getClientRects()[0],
23+ cpWidth = cpRect.width,
24+ service_center = service.relativeCenter,
25+ menuLeft = (service.x * z + tr[0] + service_center[0] * z <
26+ topo.get('width') / 2),
27+ cpHeight = cpRect.height;
28
29- if (menu_left) {
30+ if (menuLeft) {
31 cp.removeClass('left')
32 .addClass('right');
33 } else {
34@@ -1210,15 +1212,17 @@
35 }
36 // Set the position of the div in the following way:
37 // top: aligned to the scaled/panned service minus the
38- // location of the tip of the arrow (68px down the menu,
39- // via css) such that the arrow always points at the service.
40+ // location of the tip of the arrow such that the arrow always
41+ // points at the service.
42 // left: aligned to the scaled/panned service; if the
43 // service is left of the midline, display it to the
44 // right, and vice versa.
45 cp.setStyles({
46- 'top': service.y * z + tr[1] + (service_center[1] * z) - 68,
47+ 'top': (
48+ service.y * z + tr[1] +
49+ (service_center[1] * z) - (cpHeight / 2)),
50 'left': service.x * z +
51- (menu_left ? service.w * z : -(cp_width)) + tr[0]
52+ (menuLeft ? service.w * z + 16 : -(cpWidth) - 16) + tr[0]
53 });
54 }
55 },
56@@ -1254,16 +1258,20 @@
57 var landscape = topo.get('landscape');
58 var landscapeReboot = serviceMenu.one('.landscape-reboot').hide();
59 var landscapeSecurity = serviceMenu.one('.landscape-security').hide();
60+ var triangle = serviceMenu.one('.triangle');
61 var securityURL, rebootURL;
62 var flags = window.flags;
63
64 if (flags.serviceInspector) {
65 this.show_service(service);
66- return;
67+ }
68+
69+ if (service.get('pending')) {
70+ return true;
71 }
72
73 // Update landscape links and show/hide as needed.
74- if (landscape) {
75+ if (landscape && !flags.serviceInspector) {
76 rebootURL = landscape.getLandscapeURL(service, 'reboot');
77 securityURL = landscape.getLandscapeURL(service, 'security');
78
79@@ -1275,11 +1283,20 @@
80 }
81 }
82
83+ // The view option should not be used with the inspector.
84+ if (flags.serviceInspector) {
85+ serviceMenu.one('.view-service').hide();
86+ }
87+
88 if (box && !serviceMenu.hasClass('active')) {
89 topo.set('active_service', box);
90 topo.set('active_context', box.node);
91 serviceMenu.addClass('active');
92
93+ var menuHeight = serviceMenu.getDOMNode().getClientRects()[0].height;
94+ var triHeight = 18;
95+ triangle.setStyle('top', ((menuHeight - triHeight) / 2) + 'px');
96+
97 // Disable the 'Build Relation' link if the charm has not yet loaded.
98 var addRelation = serviceMenu.one('.add-relation');
99 if (this.allowBuildRelation(topo, service)) {
100
101=== modified file 'lib/views/stylesheet.less'
102--- lib/views/stylesheet.less 2013-07-19 17:30:13 +0000
103+++ lib/views/stylesheet.less 2013-07-20 01:26:26 +0000
104@@ -28,6 +28,8 @@
105 @navbar-divider-colour: #333;
106 @text-colour: #505050;
107 @border-radius: 3px;
108+@enviroment-menu-background-color: #ffffff;
109+@enviroment-menu-hover-background: #f2f2f2;
110
111 // Imports need to be after vars.
112 @import "typography.less";
113@@ -460,12 +462,12 @@
114 }
115 .environment-menu {
116 @border_radius: 20px;
117- @background_color: #282421;
118- .create-border-radius(@border_radius);
119- .create-box-shadow(-2px 4px 4px 0 rgba(0, 0, 0, 0.5));
120+ @background_color: @enviroment-menu-background-color;
121+ .create-border-radius(@border-radius);
122+ .create-box-shadow(0 0 2px rgba(0, 0, 0, 0.3));
123 display: none;
124 background-color: @background_color;
125- color: #fdf6e3;
126+ color: @text-colour;
127 top: 0;
128 left: 0;
129 position: absolute;
130@@ -475,18 +477,21 @@
131 }
132 .triangle {
133 position: absolute;
134- top: 62px;
135- width: 12px;
136- line-height: 21px;
137- background-repeat: no-repeat;
138+ top: 22px;
139+ width: 0px;
140+ height: 0px;
141+ border-style: solid;
142+ }
143+ &.right .triangle {
144+ left: -16px;
145+ border-width: 10px 17.3px 10px 0;
146+ border-color: transparent #ffffff transparent transparent;
147 }
148 &.left .triangle {
149- right: -12px;
150- background-image: url(/juju-ui/assets/images/icons/icon_shadow_triangle_right.png);
151- }
152- &.right .triangle {
153- left: -12px;
154- background-image: url(/juju-ui/assets/images/icons/icon_shadow_triangle.png);
155+ right: -16px;
156+ border-width: 10px 0 10px 17.3px;
157+ border-color: transparent transparent transparent #ffffff;
158+
159 }
160 .menu-title {
161 margin: 10px;
162@@ -496,22 +501,21 @@
163 float: right;
164 }
165 ul {
166- margin: 10px;
167+ margin: 0;
168 list-style-type: none;
169
170 li {
171- .create-border-radius(@border_radius);
172- background-position: 5px center;
173+ background-position: 10px center;
174 background-repeat: no-repeat;
175- border-bottom: 1px #5f594f solid;
176 cursor: pointer;
177 line-height: 32px;
178- padding: 5px 5px 5px 5px;
179+ padding: 0 15px;
180+ white-space: nowrap;
181
182 a {
183 text-decoration: none;
184- font-size: 75%;
185- color: #fdf6e3;
186+ font-size: 13px;
187+ color: @text-colour;
188 }
189 &.view-service {
190 background-image: url(/juju-ui/assets/images/icons/icon_noshadow_view.png);
191@@ -535,7 +539,7 @@
192 border-bottom: none;
193 }
194 &:hover {
195- background-color: lighten(@background_color, 10%);
196+ background-color: @enviroment-menu-hover-background;
197 }
198 &.disabled {
199 color: gray;
200@@ -548,7 +552,7 @@
201 }
202 }
203 &#service-menu ul li {
204- padding-left: 42px;
205+ padding-left: 36px;
206 }
207
208 }

Subscribers

People subscribed via source and target branches