Merge into trunk : inspector-visual-review-bugs : Code : juju-gui

Status:	Merged
Merged at revision:	1058
Proposed branch:	lp://staging/~gary/juju-gui/inspector-visual-review-bugs
Merge into:	lp://staging/juju-gui/experimental
Diff against target:	283 lines (+34/-49) 9 files modified app/templates/ghost-config-viewlet.handlebars (+4/-4) app/templates/inspector-header.handlebars (+3/-1) app/templates/service-configuration.handlebars (+3/-1) app/templates/service-configuration.partial (+0/-6) app/templates/service-expose.partial (+3/-3) app/views/ghost-inspector.js (+2/-7) app/views/inspector.js (+2/-2) lib/views/juju-inspector.less (+15/-23) test/test_ghost_inspector.js (+2/-2)
To merge this branch:	bzr merge lp://staging/~gary/juju-gui/inspector-visual-review-bugs
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2013-09-18	Pending
Review via email: mp+186310@code.staging.launchpad.net

Description of the change

Visual updates from inspector review

Changes from Luca's notes:

General
- The close button is the wrong size. Please see here for the correct size:
http://a.flatsi.es/projects/z9t3pp/images/63hmzx
- When clicking "Destroy service" the popover confirmation feels a bit sluggish,
can we speed this up?
Post-deployment
- The name validation checkmark and cross are not aligned correctly.

Other changes:

Fixed "Use default configuration?" position.
Fixed default configuration settings text colour to match visuals.
Fixed ghost "Import config file..." only active if "Use default configuration?"
is toggled off (bug #1226905)

(landed for Huw)

https://codereview.appspot.com/13457057/

Revision history for this message

Gary Poster (gary) wrote on 2013-09-18:

#

Download full text (12.9 KiB)

Reviewers: mp+186310_code.launchpad.net,

Message:
Please take a look.

Description:
Visual updates from inspector review

Changes from Luca's notes:

General
- The close button is the wrong size. Please see here for the correct
size:
http://a.flatsi.es/projects/z9t3pp/images/63hmzx
- When clicking "Destroy service" the popover confirmation feels a bit
sluggish,
can we speed this up?
Post-deployment
- The name validation checkmark and cross are not aligned correctly.

Other changes:

Fixed "Use default configuration?" position.
Fixed default configuration settings text colour to match visuals.
Fixed ghost "Import config file..." only active if "Use default
configuration?"
is toggled off (bug #1226905)

(landed for Huw)

https://code.launchpad.net/~gary/juju-gui/inspector-visual-review-bugs/+merge/186310

(do not edit description out of merge proposal)

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

Affected files (+33, -46 lines):
   A [revision details]
   M app/assets/images/form-validation-cross.png
   M app/assets/images/form-validation-tick.png
   A app/assets/images/inspector-close.jpg
   M app/templates/ghost-config-viewlet.handlebars
   M app/templates/inspector-header.handlebars
   M app/templates/service-configuration.handlebars
   M app/templates/service-configuration.partial
   M app/templates/service-expose.partial
   M app/views/ghost-inspector.js
   M app/views/inspector.js
   M lib/views/juju-inspector.less
   M test/test_ghost_inspector.js

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/templates/ghost-config-viewlet.handlebars
=== modified file 'app/templates/ghost-config-viewlet.handlebars'
--- app/templates/ghost-config-viewlet.handlebars 2013-09-17 14:30:35 +0000
+++ app/templates/ghost-config-viewlet.handlebars 2013-09-18 05:03:17 +0000
@@ -27,12 +27,12 @@
      <i class="sprite inspector_configuration"></i>
      Configuration
    </h2>
- <div class="view-content join-top service-configuration use-defaults">
+ <div class="view-content join-top service-configuration">
      <div class="config-file settings-wrapper">
        <input type="file" id="config-file">
        <div class="fakebutton">Import config file...</div>
      </div>
- <div class="toggle settings-wrapper">
+ <div class="toggle single-line settings-wrapper">
        <label>Use default configuration?</label>
        <div class="toggle-switch">
          <input type="checkbox" class="hidden-checkbox"
id="use-default-toggle" checked>
@@ -40,11 +40,11 @@
            <div class="handle"></div>
          </label>
        </div>
- <div class="textvalue" style="display: none;">true</div>
+ <div class="textvalue"></div>
        
        <div class="clearall"></div>
      </div>
- <form class="charm-settings">
+ <form class="charm-settings use-defaults">
        <div class="service-config">
       ...

Reviewers: mp+186310_code.launchpad.net,

Message:
Please take a look.

Description:
Visual updates from inspector review

Changes from Luca's notes:

General
- The close button is the wrong size. Please see here for the correct
size:
http://a.flatsi.es/projects/z9t3pp/images/63hmzx
- When clicking "Destroy service" the popover confirmation feels a bit
sluggish,
can we speed this up?
Post-deployment
- The name validation checkmark and cross are not aligned correctly.

Other changes:

Fixed "Use default configuration?" position.
Fixed default configuration settings text colour to match visuals.
Fixed ghost "Import config file..." only active if "Use default
configuration?"
is toggled off (bug #1226905)

(landed for Huw)

https://code.launchpad.net/~gary/juju-gui/inspector-visual-review-bugs/+merge/186310

(do not edit description out of merge proposal)

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

Affected files (+33, -46 lines):
   A [revision details]
   M app/assets/images/form-validation-cross.png
   M app/assets/images/form-validation-tick.png
   A app/assets/images/inspector-close.jpg
   M app/templates/ghost-config-viewlet.handlebars
   M app/templates/inspector-header.handlebars
   M app/templates/service-configuration.handlebars
   M app/templates/service-configuration.partial
   M app/templates/service-expose.partial
   M app/views/ghost-inspector.js
   M app/views/inspector.js
   M lib/views/juju-inspector.less
   M test/test_ghost_inspector.js

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-20130917211701-glculimoege77702
+New revision: gary.poster@canonical.com-20130918124524-zt1i58p0xhfkbv21

Index: app/templates/ghost-config-viewlet.handlebars
=== modified file 'app/templates/ghost-config-viewlet.handlebars'
--- app/templates/ghost-config-viewlet.handlebars	2013-09-17 14:30:35 +0000
+++ app/templates/ghost-config-viewlet.handlebars	2013-09-18 05:03:17 +0000
@@ -27,12 +27,12 @@
      <i class="sprite inspector_configuration"></i>
      Configuration
    </h2>
-  <div class="view-content join-top service-configuration use-defaults">
+  <div class="view-content join-top service-configuration">
      <div class="config-file settings-wrapper">
        <input type="file" id="config-file">
        <div class="fakebutton">Import config file...</div>
      </div>
-    <div class="toggle settings-wrapper">
+    <div class="toggle single-line settings-wrapper">
        <label>Use default configuration?</label>
        <div class="toggle-switch">
          <input type="checkbox" class="hidden-checkbox"  
id="use-default-toggle" checked>
@@ -40,11 +40,11 @@
            <div class="handle"></div>
          </label>
        </div>
-      <div class="textvalue" style="display: none;">true</div>
+      <div class="textvalue"></div>
        
        <div class="clearall"></div>
      </div>
-    <form class="charm-settings">
+    <form class="charm-settings use-defaults">
        <div class="service-config">
          {{> service-configuration}}
        </div>

Index: app/templates/inspector-header.handlebars
=== modified file 'app/templates/inspector-header.handlebars'
--- app/templates/inspector-header.handlebars	2013-09-16 19:32:22 +0000
+++ app/templates/inspector-header.handlebars	2013-09-18 12:45:24 +0000
@@ -16,6 +16,8 @@
          <div class="charm-url" data-charmid="{{charmUrl}}">Charm  
details</div>
        {{/unless}}
      </div>
-    <div href="" class="close">&#x2A2F;</div>
+    <div class="close">
+        <i class="sprite inspector-close"></i>
+    </div>
    </div>
  </header>

Index: app/assets/images/form-validation-cross.png
=== modified file 'app/assets/images/form-validation-cross.png'
Binary files app/assets/images/form-validation-cross.png	2013-09-16  
19:32:22 +0000 and app/assets/images/form-validation-cross.png	2013-09-18  
00:59:16 +0000 differ

Index: app/assets/images/form-validation-tick.png
=== modified file 'app/assets/images/form-validation-tick.png'
Binary files app/assets/images/form-validation-tick.png	2013-09-16 19:32:22  
+0000 and app/assets/images/form-validation-tick.png	2013-09-18 00:59:16  
+0000 differ

Index: app/assets/images/inspector-close.jpg
=== added file 'app/assets/images/inspector-close.jpg'
Binary files app/assets/images/inspector-close.jpg	1970-01-01 00:00:00  
+0000 and app/assets/images/inspector-close.jpg	2013-09-18 00:39:02 +0000  
differ

Index: app/templates/service-configuration.handlebars
=== modified file 'app/templates/service-configuration.handlebars'
--- app/templates/service-configuration.handlebars	2013-09-17 14:30:35 +0000
+++ app/templates/service-configuration.handlebars	2013-09-18 05:03:17 +0000
@@ -5,7 +5,9 @@
        <input type="file">
        <div class="fakebutton">Import config file...</div>
      </div>
-    {{> service-configuration}}
+    <div class="view-content charm-settings">
+      {{> service-configuration}}
+    </div>
    </div>
    {{#unless ghost}}
      <div class="controls configuration-buttons closed">

Index: app/templates/service-configuration.partial
=== modified file 'app/templates/service-configuration.partial'
--- app/templates/service-configuration.partial	2013-09-12 12:36:25 +0000
+++ app/templates/service-configuration.partial	2013-09-18 05:03:17 +0000
@@ -1,7 +1,4 @@
  {{! Expecting the generated data from extractServiceSettings utility. }}
-{{#unless ghost}}
-  <div class="view-content">
-{{/unless}}
  {{#settings}}
      <div class="{{#if isBool}}toggle{{/if}} settings-wrapper">
      {{#if isBool}}
@@ -57,6 +54,3 @@
      </div>
    </div>
  {{/settings}}
-{{#unless ghost}}
-  </div>
-{{/unless}}

Index: app/templates/service-expose.partial
=== modified file 'app/templates/service-expose.partial'
--- app/templates/service-expose.partial	2013-09-12 19:36:19 +0000
+++ app/templates/service-expose.partial	2013-09-18 02:04:35 +0000
@@ -1,7 +1,7 @@
  <div class="overview-footer-item expose">
-  <div class="toggle expose">
+  <div class="toggle single-line">
      <label for="expose-toggle">Expose this service?</label>
-    <div class="toggle-switch toggle-expose" data-bind="exposed">
+    <div class="toggle-switch" data-bind="exposed">
          <input type="checkbox"
                 name="expose-toggle"
                 class="expose-toggle hidden"
@@ -12,4 +12,4 @@
          </label>
      </div>
    </div>
-</div>
\ No newline at end of file
+</div>

Index: app/views/ghost-inspector.js
=== modified file 'app/views/ghost-inspector.js'
--- app/views/ghost-inspector.js	2013-09-17 00:55:55 +0000
+++ app/views/ghost-inspector.js	2013-09-18 05:03:17 +0000
@@ -202,7 +202,7 @@

var container = this.viewletManager.get('container'),
            ghostConfigNode = container.one(
-              '.service-configuration');
+              '.service-configuration .charm-settings');

var textareas = ghostConfigNode.all('textarea'),
            inputs = ghostConfigNode.all('input');
@@ -210,12 +210,7 @@
        if (useDefaults) {
          ghostConfigNode.addClass('use-defaults');
          textareas.setAttribute('disabled');
-        inputs.each(function(input) {
-          // Without this check you will disable the toggle button too
-          if (input.get('id') !== 'use-default-toggle') {
-            input.setAttribute('disabled');
-          }
-        });
+        inputs.setAttribute('disabled');

var viewlet = this.viewletManager.viewlets.ghostConfig,
              viewletContainer = viewlet.container;

Index: app/views/inspector.js
=== modified file 'app/views/inspector.js'
--- app/views/inspector.js	2013-09-16 11:15:46 +0000
+++ app/views/inspector.js	2013-09-18 05:18:01 +0000
@@ -704,7 +704,7 @@
              }));
        }
        var container = this.viewletManager.get('container');
-      container.all('.settings-wrapper').hide();
+      container.all('.charm-settings, .settings-wrapper.toggle').hide();
      },

/**
@@ -718,7 +718,7 @@
        var container = this.viewletManager.get('container');
        this.viewletManager.configFileContent = null;
        container.one('.fakebutton').setHTML('Import config file...');
-      container.all('.settings-wrapper').show();
+      container.all('.charm-settings, .settings-wrapper.toggle').show();
        // Replace the file input node.  There does not appear to be any way
        // to reset the element, so the only option is this rather crude
        // replacement.  It actually works well in practice.

Index: lib/views/juju-inspector.less
=== modified file 'lib/views/juju-inspector.less'
--- lib/views/juju-inspector.less	2013-09-17 21:01:36 +0000
+++ lib/views/juju-inspector.less	2013-09-18 05:03:17 +0000
@@ -220,32 +220,31 @@
              }
          }
          .toggle {
-
              span {
                  display: block;
                  vertical-align: top;
                  line-height: 30px;
              }
-
              .textvalue {
                float: left;
                height: 26px;
                padding-top: 5px;
                text-transform: capitalize;
              }
-
-            &.expose {
+            &.single-line {
                  label {
                      display: inline-block;
+                    margin-bottom: 0;
                      line-height: 34px;
                  }
+                .textvalue {
+                    display: none;
+                }
              }
-
              .toggle-switch {
                  display: inline-block;
                  vertical-align: top;
                  float: right;
-                padding-left: 15px;

label {
                      .create-border-radius(@border-radius);
@@ -484,7 +483,7 @@
                  /* Fixes the bottom of the service name getting cutoff  
(IE10) */
                  height: auto;
              }
-            & .use-defaults {
+            .use-defaults {
                  textarea,
                  input[type=text] {
                      background-color: @disabled-input-background-color;
@@ -492,10 +491,8 @@
                      cursor: not-allowed;
                      color: @disabled-input-text-color;
                  }
-                .fakebutton {
-                    color: @disabled-input-background-color;
-                    border-color: @disabled-input-background-color;
-                    cursor: default;
+                .settings-description {
+                    color: #54504d;
                  }
              }
              .service-config {
@@ -606,16 +603,13 @@
                          text-decoration: underline;
                      }
                      .close {
-                        /* required to overwrite bootstrap styles */
-                        text-shadow: none;
-                        color: #a0a0a0;
-                        font-weight: 200;
+                        /* Required to overwrite bootstrap styles */
                          opacity: 1;
-                        font-size: 17px;
-                        line-height: 17px;
-                        /* not overwriting */
-                        vertical-align: top;
-                        margin-top: -6px;
+                        display: block;
+
+                        .sprite {
+                            vertical-align: top;
+                        }
                      }
                      & > div {
                          display: inline-block;
@@ -709,8 +703,6 @@
                  }
              }
              .settings-wrapper {
-                position: relative;
-
                  span.resolver .conflicted-env {
                    display: inline-block;
                    min-width: 25px;
@@ -829,7 +821,7 @@
          .destroy-service-prompt {
              overflow: hidden;
              max-height: 200px;
-            .transition(.25s, max-height .3s);
+            .transition(max-height .2s);
              position: absolute;
              bottom: 0;
              left: 0;

Index: test/test_ghost_inspector.js
=== modified file 'test/test_ghost_inspector.js'
--- test/test_ghost_inspector.js	2013-09-17 13:27:43 +0000
+++ test/test_ghost_inspector.js	2013-09-18 05:18:01 +0000
@@ -308,7 +308,7 @@

inspector.onFileLoaded('a.yaml', {target: {result: fileContents}});
      inspector.viewletManager.configFileContent.should.equal(fileContents);
-    var settings = container.all('.settings-wrapper');
+    var settings =  
container.all('.charm-settings, .settings-wrapper.toggle');
      settings.each(function(node) {
        node.getStyle('display').should.equal('none');
      });
@@ -320,7 +320,7 @@

inspector.onFileLoaded('a.yaml', {target: {result: fileContents}});
      inspector.viewletManager.configFileContent.should.equal(fileContents);
-    var settings = container.all('.settings-wrapper');
+    var settings =  
container.all('.charm-settings, .settings-wrapper.toggle');
      settings.each(function(node) {
        node.getStyle('display').should.equal('none');
      });

Revision history for this message

Gary Poster (gary) wrote on 2013-09-18:

#

*** Submitted:

Visual updates from inspector review

Changes from Luca's notes:

General
- The close button is the wrong size. Please see here for the correct
size:
http://a.flatsi.es/projects/z9t3pp/images/63hmzx
- When clicking "Destroy service" the popover confirmation feels a bit
sluggish,
can we speed this up?
Post-deployment
- The name validation checkmark and cross are not aligned correctly.

Other changes:

Fixed "Use default configuration?" position.
Fixed default configuration settings text colour to match visuals.
Fixed ghost "Import config file..." only active if "Use default
configuration?"
is toggled off (bug #1226905)

(landed for Huw)

R=
CC=
https://codereview.appspot.com/13457057

https://codereview.appspot.com/13457057/

juju-gui

Merge lp://staging/~gary/juju-gui/inspector-visual-review-bugs into lp://staging/juju-gui/experimental

Commit message

Description of the change

Preview Diff

Subscribers