[GOM] Improve the display of the roll information

Tools/GardeningServer/scripts/ui.js

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 333 matching lines @@
             $(this).removeClass('processing');
     },
     newId: function() {
         var id = 'status-content-' + ++this._currentId;
         this._unfinishedIds[id] = 1;
         return id;
     }
 });
 
 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: '));
 
         var latestRevision = model.latestRevisionWithNoBuildersInFlight();
         var latestRevisions = model.latestRevisionByBuilder();
 
         // Get the list of builders sorted with the most recent one first.
         var builders = Object.keys(latestRevisions);
         builders.sort(function (a, b) { return parseInt(latestRevisions[b]) - parseInt(latestRevisions[a]);});
@@ skipping 37 matching lines @@
             if (!revisionsNode.open) {
                 $(revisionsPopUp).removeClass("active");
             }
         });
 
         var totRevision = model.latestRevision();
         theSpan.appendChild(document.createTextNode(', trunk is at '));
         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();
         });
     }
 });
 
 })();