Swarming UI: Bold requested dimensions in bot dimensions

appengine/swarming/ui/res/imp/taskpage/task-page.html

Index: appengine/swarming/ui/res/imp/taskpage/task-page.html
diff --git a/appengine/swarming/ui/res/imp/taskpage/task-page.html b/appengine/swarming/ui/res/imp/taskpage/task-page.html
index bed9c40613bd7e69c9f8e4090ed1388a14391c43..52984fbab84feb03dbcce32d04dfce2da4730a7d 100644
--- a/appengine/swarming/ui/res/imp/taskpage/task-page.html
+++ b/appengine/swarming/ui/res/imp/taskpage/task-page.html
@@ -63,6 +63,10 @@
         word-break: break-all;
       }
 
+      .bolded {
+        font-weight: bold;
+      }
+
       .expand {
         min-width: 3em;
         vertical-align: middle;
@@ -128,6 +132,10 @@
       .italic {
         font-style: italic;
       }
+
+      span~span:before {

[Ryan Tseng, 2017/06/06 00:27:55]

This should be restricted to certain classes.  ie span.dim~span.dim

[cwpayton, 2017/06/06 16:04:08]

Done.

+        content: " | ";
+      }
     </style>
 
     <url-param name="id"
@@ -342,7 +350,7 @@
                 </tr>
                 <template is="dom-repeat" items="{{_request.properties.dimensions}}" as="dimension">
                   <tr>
-                    <td><b>[[dimension.key]]:</b> [[_alias(dimension)]]</td>
+                    <td><b>[[dimension.key]]:</b> [[_aliasRequest(dimension)]]</td>
                   </tr>
                 </template>
                 <tr>
@@ -483,7 +491,15 @@
                 </tr>
                 <template is="dom-repeat" items="[[_result.bot_dimensions]]" as="dimension">
                   <tr>
-                    <td><b>[[dimension.key]]:</b> [[_alias(dimension)]]</td>
+                    <td bgcolor$="[[_highlight(_request.properties.dimensions, dimension.key)]]">

[kjlubick, 2017/06/06 12:26:54]

Super minor nit: you only need the $= for certain attributes like href, hidden,
etc. Otherwise, just use = (aka a property binding)

https://www.polymer-project.org/1.0/docs/devguide/data-binding#native-binding

[cwpayton, 2017/06/06 16:04:08]

Turns out bgcolor needs a $= instead of a simple =. I made the change and the
highlight stopped working, so I switched it back to $=.

+                      <b>[[dimension.key]]:</b>
+                      <template 
+                        is="dom-repeat" 
+                        items="[[_aliasResult(_request.properties.dimensions, dimension)]]" 
+                        as="value">
+                        <span class$="[[value.class]]">[[value.name]]</span>
+                      </template>
+                    </td>
                   </tr>
                 </template>
 
@@ -736,19 +752,50 @@
       observers: [
         "_similarLoad(_request,_result,_auth_headers)"],
 
-      _alias: function(dim) {
-        var values = dim.value;
+      _aliasRequest: function(request) {
+        var values = request.value;
         if (!Array.isArray(values)) {
           values = [values];
         }
-        if (swarming.alias.has(dim.key)) {
-          values.forEach(function(v, i){
-            values[i] = swarming.alias.apply(v, dim.key);
+        if (swarming.alias.has(request.key)) {
+          values.forEach(function(v, i) {
+            values[i] = swarming.alias.apply(v, request.key);
           });
         }
         return values.join(" | ");
       },
 
+      _aliasResult: function(request, result) {

[kjlubick, 2017/06/06 12:26:54]

Please rename request -> request_dims and result -> dim or similar

Elsewhere request means the entire request object and result means the result
object;  we would prefer to not confuse the reader.

[cwpayton, 2017/06/06 16:04:08]

Done.

+        // request is the entire _request.properties.dimensions array, but we are

[Sergey Berezin, 2017/06/06 00:14:05]

nit: please reformat to fit 80 chars.

[cwpayton, 2017/06/06 16:04:08]

Done.

+        // only passed in one key at a time from _result.bot_dimensions as the
+        // result variable. Therefore we need to loop through the key-value pairs
+        // of request to determine whether the current bot dimension was requested.
+        var dimensions = [];
+        var target = null;
+        request.forEach(function(dim, i) {
+          if (dim.key == result.key) target = i;

[kjlubick, 2017/06/06 12:26:54]

Use triple equals unless there is a good reason not to (and then document what
that good reason is).

Format the if clause onto its own line, with {}

[cwpayton, 2017/06/06 16:04:08]

Done.

+        });
+        var values = result.value;
+        if (!Array.isArray(values)) {

[kjlubick, 2017/06/06 12:26:54]

Just curious, is this logic just for safety or is the dimension's value
occasionally not an array?

[cwpayton, 2017/06/06 16:04:08]

This check was written by someone else before me, so I assume they ran into a
problem where if result only had one value, that value was simply passed in
as-is instead of inside of an array, so I left it in.

+          values = [values];
+        }
+        values.forEach(function(v, i) {
+          // Adjust the proper alias, if necessary
+          if (swarming.alias.has(result.key)){
+            values[i] = swarming.alias.apply(v, result.key);
+          }
+          // Set class attribute to bolded if the value was included in the
+          // requested dimensions
+          var bold = "";
+          if (target != null && request[target].value == v) bold = "bolded"

[kjlubick, 2017/06/06 12:26:54]

Format the if clause onto its own line, with {}

[cwpayton, 2017/06/06 16:04:08]

Done.

+          dimensions.push({"name": values[i], "class": bold});
+        });
+
+        // Return an array of objects that map each value to its corresponding
+        // class name based on whether it should be bolded.
+        return dimensions;

[kjlubick, 2017/06/06 12:26:54]

Minor naming suggestion (take it or leave it), dimensions -> boldMap

[cwpayton, 2017/06/06 16:04:08]

Done.

+      },
+
       _bytes: function(sizeInBytes) {
         return sk.human.bytes(sizeInBytes);
       },
@@ -875,6 +922,14 @@
         return result && result.cipd_pins && result.cipd_pins.packages;
       },
 
+      _highlight: function(request, key) {
+        var highlight = false;
+        request.forEach(function (dim) {
+          if (dim.key == key) highlight = true;

[kjlubick, 2017/06/06 12:26:54]

Format the if clause onto its own line, with {}

[cwpayton, 2017/06/06 16:04:08]

Done.

+        });
+        return highlight ? "Yellow" : "White";
+      },
+
       _internalClass: function(failure) {
         if (failure) {
           return "exception";