[GOM] Improve the display of the roll information

Tools/GardeningServer/scripts/ui.js

Index: Tools/GardeningServer/scripts/ui.js
diff --git a/Tools/GardeningServer/scripts/ui.js b/Tools/GardeningServer/scripts/ui.js
index 19e3d6d35038e420d25b626e1ab49e9444c7af22..a10e422f1fea9b5bc5923511417954bd57459819 100644
--- a/Tools/GardeningServer/scripts/ui.js
+++ b/Tools/GardeningServer/scripts/ui.js
@@ -351,6 +351,23 @@ ui.StatusArea = base.extends('div',  {
 });
 
 ui.revisionDetails = base.extends('span', {
+    updateUIIfDone: function() {
+        if (this.roll === undefined || this.lastRolledRevision === undefined)
+            return;
+
+        this.appendChild(document.createElement("br"));

[ojan, 2014/06/04 19:46:25]

Meh. I prefer it on one line to conserve vertical space. But I don't feel
strongly enough about that to not lgtm.

[Julien - ping for review, 2014/06/05 00:33:51]

You kinda did :-)

When doing a design, it's important to put associated information close together
(rule of proximity) but also somehow separate it from non-related information
(rule of contrast). This <br> is meant to make roll-related information (last
rolled revision and current roll) stand aside from bot related information
(trunk revision and last revision processed by the bots).

As it stands we have a really long line that's mixing 2 types of information and
makes it hard to parse IMO.

[Julien - ping for review, 2014/06/05 00:45:29]

I am open to better suggestions to separate the 2 types of information but I
definitely think that bundling them together hurts the usability of GOM.

+        this.appendChild(document.createTextNode('Last roll is to '));
+        this.appendChild(ui.createLinkNode(trac.changesetURL(this.lastRolledRevision), this.lastRolledRevision));
+        this.appendChild(document.createTextNode(', current autoroll '));
+        if (this.roll) {
+            var linkText = "" + this.roll.fromRevision + ":" + this.roll.toRevision;
+            this.appendChild(ui.createLinkNode(this.roll.url, linkText));
+            if (this.roll.isStopped)
+                this.appendChild(document.createTextNode(' (STOPPED) '));
+        } else {
+            this.appendChild(document.createTextNode(' None'));
+        }
+    },
     init: function() {
         var theSpan = this;
         theSpan.appendChild(document.createTextNode('Latest revision processed by every bot: '));
@@ -408,20 +425,13 @@ ui.revisionDetails = base.extends('span', {
         theSpan.appendChild(ui.createLinkNode(trac.changesetURL(totRevision), totRevision));
 
         checkout.lastBlinkRollRevision().then(function(revision) {
-            theSpan.appendChild(document.createTextNode(', last roll is to '));
-            theSpan.appendChild(ui.createLinkNode(trac.changesetURL(revision), revision));
+            theSpan.lastRolledRevision = revision;
+            theSpan.updateUIIfDone();
         }, function() {});
 
         rollbot.fetchCurrentRoll().then(function(roll) {
-            theSpan.appendChild(document.createTextNode(', current autoroll '));
-            if (roll) {
-                var linkText = "" + roll.fromRevision + ":" + roll.toRevision;
-                theSpan.appendChild(ui.createLinkNode(roll.url, linkText));
-                if (roll.isStopped)
-                    theSpan.appendChild(document.createTextNode(' (STOPPED) '));
-            } else {
-                theSpan.appendChild(document.createTextNode(' None'));
-            }
+            theSpan.roll = roll;
+            theSpan.updateUIIfDone();
         });
     }
 });