Merge lp:~rharding/juju-gui/qa-ant-remove-bottom into lp:juju-gui/experimental

[Richard Harding]

Removal of the bottom landscape bar

You will see the bottom bar has been removed from the dom with all connected
commands. I did not move the controls as there was no direction too. Simply
deleted them. Removed the element of the stylesheet that set the Charm
Browser's bottom value and removed the SASS var with the height of the bottom
bar.

https://codereview.appspot.com/14960044/

[Richard Harding (rharding)]

Reviewers: mp+191846_code.launchpad.net,

Message:
Please take a look.

Description:
Removal of the bottom landscape bar

You will see the bottom bar has been removed from the dom with all
connected
commands. I did not move the controls as there was no direction too.
Simply
deleted them. Removed the element of the stylesheet that set the Charm
Browser's bottom value and removed the SASS var with the height of the
bottom
bar.

https://code.launchpad.net/~rharding/juju-gui/qa-ant-remove-bottom/+merge/191846

(do not edit description out of merge proposal)

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

Affected files (+3, -300 lines):
   A [revision details]
   M app/app.js
   M app/modules-debug.js
   D app/templates/landscape-controls.partial
   M app/templates/overview.handlebars
   M app/views/environment.js
   M app/views/topology/bundle.js
   D app/views/topology/landscape.js
   M app/views/topology/service.js
   M app/views/topology/topology.js
   M app/views/utils.js
   M lib/views/browser/main.less
   M lib/views/browser/vars.less
   M test/test_environment_view.js
   M test/test_landscape.js

[Jeff Pihach (hatch)]

The removal looks good thanks for all the work! Before this lands I
would like to have a discussion as to what the long term goal of this
removal and the landscape integration is to see if we should save some
of this code or remove even more.

QA NOT OK
When viewing the browser in 'Browse' mode there is a white bar across
the bottom of the page.

There is a large white gap when viewing search results in 'Browse' mode:

http://localhost:8888/fullscreen/search/?categories=app-servers&text=

Without the footer the url/status indicator in chrome covers part of the
token when hovering over the tokens.

https://codereview.appspot.com/14960044/diff/1/app/views/topology/service.js
File app/views/topology/service.js (left):

https://codereview.appspot.com/14960044/diff/1/app/views/topology/service.js#oldcode183
app/views/topology/service.js:183:
'/juju-ui/assets/images/landscape_restart_round.png';
These assets should probably be removed from the file system.

https://codereview.appspot.com/14960044/

=== modified file 'app/app.js'
--- app/app.js	2013-10-17 15:14:27 +0000
+++ app/app.js	2013-10-18 17:31:13 +0000
@@ -388,10 +388,6 @@
       // Create a client side database to store state.
       this.db = new models.Database();
 
-      // Optional Landscape integration helper.
-      this.landscape = new views.Landscape();
-      this.landscape.set('db', this.db);
-
       // Set up a new modelController instance.
       this.modelController = new juju.ModelController({
         db: this.db,
@@ -673,7 +669,7 @@
       }
       Y.each(
           [this.env, this.db, this.notifications,
-           this.landscape, this.endpointsController],
+           this.endpointsController],
           function(o) {
             if (o && o.destroy) {
               o.detachAll();
@@ -728,9 +724,6 @@
     _dbChangedHandler: function() {
       var active = this.get('activeView');
 
-      // Update Landscape annotations.
-      this.landscape.update();
-
       // Regardless of which view we are rendering,
       // update the env view on db change.
       if (this.views.environment.instance) {
@@ -1095,7 +1088,6 @@
       var options = {
         getModelURL: Y.bind(this.getModelURL, this),
         nsRouter: this.nsRouter,
-        landscape: this.landscape,
         endpointsController: this.endpointsController,
         useDragDropImport: this.get('sandbox'),
         db: this.db,
@@ -1360,7 +1352,6 @@
     'juju-view-environment',
     'juju-view-login',
     'juju-view-onboarding',
-    'juju-landscape',
     'juju-websocket-logging',
     'io',
     'json-parse',

=== modified file 'app/modules-debug.js'
--- app/modules-debug.js	2013-10-10 11:44:44 +0000
+++ app/modules-debug.js	2013-10-18 17:31:13 +0000
@@ -210,10 +210,6 @@
           fullpath: '/juju-ui/views/topology/service.js'
         },
 
-        'juju-topology-landscape': {
-          fullpath: '/juju-ui/views/topology/landscape.js'
-        },
-
         'juju-topology-importexport': {
           fullpath: '/juju-ui/views/topology/importexport.js'
         },

=== removed file 'app/templates/landscape-controls.partial'
--- app/templates/landscape-controls.partial	2013-03-26 21:17:14 +0000
+++ app/templates/landscape-controls.partial	1970-01-01 00:00:00 +0000
@@ -1,17 +0,0 @@
-<div class="landscape-controls">
-    <div class="logo-tab">
-        <a><i class="sprite landscape_logo"></i></a>
-    </div>
-    <div class="machine-control control">
-        <i class="sprite dotted_divider icon-divider"></i>
-        <a><i class="sprite landscape-icon landscape_machine"></i></a>
-    </div>
-    <div class="updates-control control">
-        <i class="sprite dotted_divider icon-divider"></i>
-        <a><i class="sprite landscape-icon landscape_updates"></i></a>
-    </div>
-    <div class="restart-control control">
-        <i class="sprite dotted_divider icon-divider"></i>
-        <a><i class="sprite landscape-icon landscape_restart"></i></a>
-    </div>
-</div>

=== modified file 'app/templates/overview.handlebars'
--- app/templates/overview.handlebars	2013-09-19 16:27:07 +0000
+++ app/templates/overview.handlebars	2013-10-18 17:31:13 +0000
@@ -28,9 +28,6 @@
       </div>
       {{> right-sidebar }}
     </div>
-    <div id="overview-tasks" class="bottom-navbar">
-        {{> landscape-controls }}
-    </div>
     <div id="rmrelation-modal-panel"></div>
     <div id="rmsubrelation-modal-panel"></div>
 </div>

=== modified file 'app/views/environment.js'
--- app/views/environment.js	2013-09-30 16:43:28 +0000
+++ app/views/environment.js	2013-10-18 17:31:13 +0000
@@ -350,7 +350,6 @@
               getInspector: Y.bind(this.getInspector, this),
               setInspector: Y.bind(this.setInspector, this),
               createServiceInspector: Y.bind(this.createServiceInspector, this),
-              landscape: this.get('landscape'),
               getModelURL: this.get('getModelURL'),
               container: container,
               endpointsController: this.get('endpointsController'),
@@ -360,7 +359,6 @@
             topo.addModule(views.PanZoomModule);
             topo.addModule(views.ViewportModule);
             topo.addModule(views.RelationModule);
-            topo.addModule(views.LandscapeModule);
 
             topo.addTarget(this);
             this.topo = topo;

=== modified file 'app/views/topology/bundle.js'
--- app/views/topology/bundle.js	2013-10-10 00:38:45 +0000
+++ app/views/topology/bundle.js	2013-10-18 17:31:13 +0000
@@ -154,8 +154,6 @@
       if (node.empty()) {
         return;
       }
-      var topo = this.get('component'),
-          landscape = topo.get('landscape');
 
       // Apply Position Annotations
       // This is done after the services_boxes

=== removed file 'app/views/topology/landscape.js'
--- app/views/topology/landscape.js	2013-06-05 16:58:11 +0000
+++ app/views/topology/landscape.js	1970-01-01 00:00:00 +0000
@@ -1,73 +0,0 @@
-/*
-This file is part of the Juju GUI, which lets users view and manage Juju
-environments within a graphical interface (https://launchpad.net/juju-gui).
-Copyright (C) 2012-2013 Canonical Ltd.
-
-This program is free software: you can redistribute it and/or modify it under
-the terms of the GNU Affero General Public License version 3, as published by
-the Free Software Foundation.
-
-This program is distributed in the hope that it will be useful, but WITHOUT
-ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
-SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero
-General Public License for more details.
-
-You should have received a copy of the GNU Affero General Public License along
-with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-'use strict';
-
-/**
- * Provide the LandscapeModule class.
- *
- * @module topology
- * @submodule topology.landscape
- */
-
-YUI.add('juju-topology-landscape', function(Y) {
-  var views = Y.namespace('juju.views'),
-      models = Y.namespace('juju.models'),
-      d3ns = Y.namespace('d3');
-
-  /**
-   * Handle Landscape integration within a Topology.
-   *
-   * @class LandscapeModule
-   */
-  var LandscapeModule = Y.Base.create('LandscapeModule', d3ns.Module, [], {
-    /**
-     * Update Landscape links as needed.
-     *
-     * @method update
-     * @return {undefined} Nothing.
-     */
-    update: function() {
-      var topo = this.get('component');
-      var db = topo.get('db');
-      var env = db.environment;
-      var container = this.get('container');
-
-      views.utils.updateLandscapeBottomBar(topo.get('landscape'), env, env,
-          container);
-    }
-  }, {
-    ATTRS: {
-      /**
-        @property {d3ns.Component} component
-      */
-      component: {}
-    }
-
-  });
-  views.LandscapeModule = LandscapeModule;
-}, '0.1.0', {
-  requires: [
-    'node',
-    'event',
-    'd3-components',
-    'juju-models',
-    'juju-env',
-    'juju-view-utils'
-  ]
-});

=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js	2013-10-17 17:01:32 +0000
+++ app/views/topology/service.js	2013-10-18 17:31:13 +0000
@@ -71,7 +71,6 @@
     }
     var self = this,
         topo = this.get('component'),
-        landscape = topo.get('landscape'),
         service_scale = this.service_scale,
         service_scale_width = this.service_scale_width,
         service_scale_height = this.service_scale_height;
@@ -169,55 +168,6 @@
           'x': 64,
           'y': 47 * 0.8});
 
-    // Landscape badge
-    if (landscape) {
-      node.each(function(d) {
-        var landscapeAsset;
-        var securityBadge = landscape.getLandscapeBadge(
-            d.model, 'security', 'round');
-        var rebootBadge = landscape.getLandscapeBadge(
-            d.model, 'reboot', 'round');
-
-        if (securityBadge && rebootBadge) {
-          landscapeAsset =
-              '/juju-ui/assets/images/landscape_restart_round.png';
-        } else if (securityBadge) {
-          landscapeAsset =
-              '/juju-ui/assets/images/landscape_security_round.png';
-        } else if (rebootBadge) {
-          landscapeAsset =
-              '/juju-ui/assets/images/landscape_restart_round.png';
-        }
-        if (landscapeAsset === undefined) {
-          // Remove any existing badge.
-          d3.select(this).select('.landscape-badge').remove();
-        } else {
-          var existing = Y.one(this).one('.landscape-badge'),
-              curr_href, target;
-
-          if (!existing) {
-            existing = d3.select(this).append('image');
-            existing.attr({
-              'class': 'landscape-badge',
-              'width': 32,
-              'height': 32
-            });
-          }
-          existing = d3.select(this).select('.landscape-badge');
-          existing.attr({
-            'x': 13,
-            'y': 79
-          });
-
-          // Only set 'xlink:href' if not already set to the new value,
-          // thus avoiding redundant requests to the server. #1182135
-          curr_href = existing.attr('xlink:href');
-          if (curr_href !== landscapeAsset) {
-            existing.attr({'xlink:href': landscapeAsset});
-          }
-        }
-      });
-    }
     // The following are sizes in pixels of the SVG assets used to
     // render a service, and are used to in calculating the vertical
     // positioning of text down along the service block.

=== modified file 'app/views/topology/topology.js'
--- app/views/topology/topology.js	2013-10-15 22:51:35 +0000
+++ app/views/topology/topology.js	2013-10-18 17:31:13 +0000
@@ -297,7 +297,6 @@
     'juju-topology-relation',
     'juju-topology-panzoom',
     'juju-topology-viewport',
-    'juju-topology-landscape',
     'juju-topology-importexport',
     'juju-topology-utils'
   ]

=== modified file 'app/views/utils.js'
--- app/views/utils.js	2013-10-15 19:33:04 +0000
+++ app/views/utils.js	2013-10-18 17:31:13 +0000
@@ -398,38 +398,6 @@
         });
   };
 
-  utils.updateLandscapeBottomBar = function(landscape, env, model, container) {
-    // Landscape annotations are stored in a unit's annotations, but just on
-    // the object in the case of services/environment.
-    var annotations = model.annotations ? model.annotations : model;
-    var envAnnotations = env.get ? env.get('annotations') : env;
-    var controls = container.one('.landscape-controls').hide();
-    var machine = controls.one('.machine-control').hide();
-    var updates = controls.one('.updates-control').hide();
-    var restart = controls.one('.restart-control').hide();
-
-    if (envAnnotations['landscape-url']) {
-      controls.show();
-      var baseLandscapeURL = landscape.getLandscapeURL(model);
-      if (baseLandscapeURL) {
-        machine.show();
-        machine.one('a').setAttribute('href', baseLandscapeURL);
-
-        if (annotations['landscape-security-upgrades']) {
-          updates.show();
-          updates.one('a').setAttribute('href',
-              landscape.getLandscapeURL(model, 'security'));
-        }
-
-        if (annotations['landscape-needs-reboot']) {
-          restart.show();
-          restart.one('a').setAttribute('href',
-              landscape.getLandscapeURL(model, 'reboot'));
-        }
-      }
-    }
-  };
-
   function _addAlertMessage(container, alertClass, message) {
     var div = container.one('#message-area');
 

=== modified file 'lib/views/browser/main.less'
--- lib/views/browser/main.less	2013-09-24 21:10:50 +0000
+++ lib/views/browser/main.less	2013-10-18 17:31:13 +0000
@@ -42,10 +42,6 @@
     background-color: #fff;
     background-image: none;
   }
-  .bws-view-data,
-  #bws-sidebar {
-    bottom: @canvas-footer-nav-height;
-  }
   #bws-fullscreen,
   #bws-sidebar .bws-content {
     overflow-x: hidden;

=== modified file 'lib/views/browser/vars.less'
--- lib/views/browser/vars.less	2013-08-13 00:35:01 +0000
+++ lib/views/browser/vars.less	2013-10-18 17:31:13 +0000
@@ -15,6 +15,5 @@
 @bws-corner: @border-radius;
 @bws-groove-dark: #c9cbc8;
 @bws-groove-light: #f4f2f3;
-@canvas-footer-nav-height: 50px;
 @tabview-right-padding: 70px;
 @tabview-left-padding: 80px;
\ No newline at end of file

=== modified file 'test/test_environment_view.js'
--- test/test_environment_view.js	2013-10-17 16:54:10 +0000
+++ test/test_environment_view.js	2013-10-18 17:31:13 +0000
@@ -644,45 +644,6 @@
       view.update();
     });
 
-    it('must be able to use Landscape annotations', function() {
-      var landscape = new views.Landscape();
-      var wordpress = db.services.getById('wordpress');
-      landscape.set('db', db);
-      // Mock out enough of the annotations to
-      // allow a full render with landscape data.
-      db.environment.set('annotations', {
-        'landscape-url': 'http://host',
-        'landscape-computers': '/foo',
-        'landscape-reboot-alert-url': '+reboot'
-      });
-      db.environment['landscape-needs-reboot'] = true;
-      db.services.each(function(s) {
-        s.set('annotations', {
-          'landscape-computers': '/service'
-        });
-      });
-      wordpress['landscape-needs-reboot'] = true;
-      wordpress['landscape-security-upgrades'] = false;
-
-      var view = new views.environment({
-        container: container,
-        db: db,
-        env: env,
-        landscape: landscape,
-        store: fakeStore
-      }).render();
-
-      var rebootItem = container.one('.landscape-controls .restart-control');
-      rebootItem.one('a').get('href').should.equal('http://host/foo+reboot');
-
-      // Test that we can match a single badge rendered on the wordpress
-      // service.
-      var wpNode = view.topo.modules.ServiceModule.getServiceNode('wordpress');
-      var hasBadge = d3.select(wpNode).select('.landscape-badge');
-      hasBadge[0][0].should.not.equal(null);
-    });
-
-
     it('must be able to render subordinate relation indicators',
        function() {
          new views.environment({

=== modified file 'test/test_landscape.js'
--- test/test_landscape.js	2013-09-26 22:03:11 +0000
+++ test/test_landscape.js	2013-10-18 17:31:13 +0000
@@ -147,66 +147,4 @@
     mysql['landscape-security-upgrades'].should.equal(false);
   });
 
-  it('should build the bottom-bar properly', function() {
-    var env = db.environment;
-    var mysql = db.services.getById('mysql');
-    var unit = mysql.get('units').item(0);
-    var partial = Y.Handlebars.partials['landscape-controls'];
-    Y.one('body').append('<div id="test-node"></div>');
-    var node = Y.one('#test-node');
-    node.append(partial());
-
-    views.utils.updateLandscapeBottomBar(landscape, env, env, node,
-        'environment');
-
-    // We should have the logo.
-    node.one('.logo-tab i').hasClass('landscape_logo').should.equal(true);
-    // We should have the correct URL for the machines.
-    node.one('.machine-control a').get('href').should
-      .equal('http://landscape.example.com/computers/criteria/environment:test/');
-    // We should have visible controls.
-    node.one('.updates-control').getStyle('display').should.equal('block');
-    node.one('.restart-control').getStyle('display').should.equal('block');
-
-    views.utils.updateLandscapeBottomBar(landscape, env, mysql, node,
-        'service');
-
-    // We should have the correct URL for the machines.
-    node.one('.machine-control a').get('href').should.equal('http://' +
-        'landscape.example.com/computers/criteria/environment:test+service:mysql/');
-    // We should have visible restart but not update controls.
-    node.one('.updates-control').getStyle('display').should.equal('none');
-    node.one('.restart-control').getStyle('display').should.equal('block');
-
-    // We handle missing annotations on a service.
-    mysql.set('annotations', {});
-    landscape.update();
-    views.utils.updateLandscapeBottomBar(landscape, env, mysql, node,
-        'service');
-    node.one('.machine-control').getStyle('display').should.equal('none');
-
-    // We handle normal unit annotations.
-    unit.annotations = {'landscape-computer': '+unit:mysql-0'};
-    landscape.update();
-
-    views.utils.updateLandscapeBottomBar(landscape, env, unit, node,
-        'unit');
-
-    // We should have the correct URL for the machines.
-    node.one('.machine-control a').get('href').should.equal('http://' +
-        'landscape.example.com/computers/criteria/environment:test+unit:mysql-0/');
-    // We should have no visible controls.
-    node.one('.updates-control').getStyle('display').should.equal('none');
-    node.one('.restart-control').getStyle('display').should.equal('none');
-
-    // We handle completely missing annotations on a unit.
-    delete unit.annotations;
-    landscape.update();
-    views.utils.updateLandscapeBottomBar(landscape, env, unit, node,
-        'unit');
-    node.one('.machine-control').getStyle('display').should.equal('none');
-
-    node.remove();
-  });
-
 });