Code review comment for lp://staging/~gary/juju-gui/styling-environment-menu

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

  }