Merge into trunk : styling-environment-menu : Code : juju-gui

Status:	Merged
Merged at revision:	882
Proposed branch:	lp://staging/~gary/juju-gui/styling-environment-menu
Merge into:	lp://staging/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://staging/~gary/juju-gui/styling-environment-menu
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2013-07-20	Pending
Review via email: mp+175973@code.staging.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/

Revision history for this message

Gary Poster (gary) wrote on 2013-07-20:

#

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

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: gary.poster@canonical.com-20130719182637-sph1dliluy3xylxo
+New revision: gary.poster@canonical.com-20130720011953-7d75eftons2nk2wx

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.get('scale');

if (service && cp) {
-        var cp_width = cp.getDOMNode().getClientRects()[0].width,
-            menu_left = (service.x * z + service.w * z / 2 <
-                         topo.get('width') * z / 2),
-            service_center = service.relativeCenter;
+        var cpRect = cp.getDOMNode().getClientRects()[0],
+            cpWidth = cpRect.width,
+            service_center = service.relativeCenter,
+            menuLeft = (service.x * z + tr[0] + service_center[0] * z <
+                        topo.get('width') / 2),
+            cpHeight = cpRect.height;

-        if (menu_left) {
+        if (menuLeft) {
            cp.removeClass('left')
              .addClass('right');
          } else {
@@ -1210,15 +1212,17 @@
          }
          // Set the position of the div in the following way:
          // top: aligned to the scaled/panned service minus the
-        //   location of the tip of the arrow (68px down the menu,
-        //   via css) such that the arrow always points at the service.
+        //   location of the tip of the arrow such that the arrow always
+        //   points at the service.
          // left: aligned to the scaled/panned service; if the
          //   service is left of the midline, display it to the
          //   right, and vice versa.
          cp.setStyles({
-          'top': service.y * z + tr[1] + (service_center[1] * z) - 68,
+          'top': (
+              service.y * z + tr[1] +
+              (service_center[1] * z) - (cpHeight / 2)),
            'left': service.x * z +
-              (menu_left ? service.w * z : -(cp_width)) + tr[0]
+              (menuLeft ? service.w * z + 16 : -(cpWidth) - 16) + tr[0]
          });
        }
      },
@@ -1254,16 +1258,20 @@
        var landscape = topo.get('landscape');
        var landscapeReboot = serviceMenu.one('.landscape-reboot').hide();
        var landscapeSecurity =  
serviceMenu.one('.landscape-security').hide();
+      var triangle = serviceMenu.one('.triangle');
        var securityURL, rebootURL;
        var flags = window.flags;

if (flags.serviceInspector) {
          this.show_service(service);
-        return;
+      }
+
+      if (service.get('pending')) {
+        return true;
        }

// Update landscape links and show/hide as needed.
-      if (landscape) {
+      if (landscape && !flags.serviceInspector) {
          rebootURL = landscape.getLandscapeURL(service, 'reboot');
          securityURL = landscape.getLandscapeURL(service, 'security');

@@ -1275,11 +1283,20 @@
          }
        }

+      // The view option should not be used with the inspector.
+      if (flags.serviceInspector) {
+        serviceMenu.one('.view-service').hide();
+      }
+
        if (box && !serviceMenu.hasClass('active')) {
          topo.set('active_service', box);
          topo.set('active_context', box.node);
          serviceMenu.addClass('active');

+        var menuHeight =  
serviceMenu.getDOMNode().getClientRects()[0].height;
+        var triHeight = 18;
+        triangle.setStyle('top', ((menuHeight - triHeight) / 2) + 'px');
+
          // Disable the 'Build Relation' link if the charm has not yet  
loaded.
          var addRelation = serviceMenu.one('.add-relation');
          if (this.allowBuildRelation(topo, service)) {

Index: lib/views/stylesheet.less
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-07-19 17:30:13 +0000
+++ lib/views/stylesheet.less	2013-07-20 01:15:05 +0000
@@ -28,6 +28,8 @@
  @navbar-divider-colour: #333;
  @text-colour: #505050;
  @border-radius: 3px;
+@enviroment-menu-background-color: #ffffff;
+@enviroment-menu-hover-background: #f2f2f2;

// Imports need to be after vars.
  @import "typography.less";
@@ -460,12 +462,12 @@
  }
  .environment-menu {
      @border_radius: 20px;
-    @background_color: #282421;
-    .create-border-radius(@border_radius);
-    .create-box-shadow(-2px 4px 4px 0 rgba(0, 0, 0, 0.5));
+    @background_color: @enviroment-menu-background-color;
+    .create-border-radius(@border-radius);
+    .create-box-shadow(0 0 2px rgba(0, 0, 0, 0.3));
      display: none;
      background-color: @background_color;
-    color: #fdf6e3;
+    color: @text-colour;
      top: 0;
      left: 0;
      position: absolute;
@@ -475,18 +477,21 @@
      }
      .triangle {
          position: absolute;
-        top: 62px;
-        width: 12px;
-        line-height: 21px;
-        background-repeat: no-repeat;
+        top: 22px;
+        width: 0px;
+        height: 0px;
+        border-style: solid;
+    }
+    &.right .triangle {
+        left: -16px;
+        border-width: 10px 17.3px 10px 0;
+        border-color: transparent #ffffff transparent transparent;
      }
      &.left .triangle {
-        right: -12px;
-        background-image:  
url(/juju-ui/assets/images/icons/icon_shadow_triangle_right.png);
-    }
-    &.right .triangle {
-        left: -12px;
-        background-image:  
url(/juju-ui/assets/images/icons/icon_shadow_triangle.png);
+        right: -16px;
+        border-width: 10px 0 10px 17.3px;
+        border-color: transparent transparent transparent #ffffff;
+
      }
      .menu-title {
          margin: 10px;
@@ -496,22 +501,21 @@
          float: right;
      }
      ul {
-        margin: 10px;
+        margin: 0;
          list-style-type: none;

li {
-            .create-border-radius(@border_radius);
-            background-position: 5px center;
+            background-position: 10px center;
              background-repeat: no-repeat;
-            border-bottom: 1px #5f594f solid;
              cursor: pointer;
              line-height: 32px;
-            padding: 5px 5px 5px 5px;
+            padding: 0 15px;
+            white-space: nowrap;

a {
                text-decoration: none;
-              font-size: 75%;
-              color: #fdf6e3;
+              font-size: 13px;
+              color: @text-colour;
              }
              &.view-service {
                  background-image:  
url(/juju-ui/assets/images/icons/icon_noshadow_view.png);
@@ -535,7 +539,7 @@
                  border-bottom: none;
              }
              &:hover {
-                background-color: lighten(@background_color, 10%);
+                background-color: @enviroment-menu-hover-background;
              }
              &.disabled {
                  color: gray;
@@ -548,7 +552,7 @@
          }
      }
      &#service-menu ul li {
-        padding-left: 42px;
+        padding-left: 36px;
      }

}

Revision history for this message

Brad Crittenden (bac) wrote on 2013-07-22:

#

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 on 2013-07-22:

#

*** 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/

juju-gui

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

Commit message

Description of the change

Preview Diff

Subscribers