Collapse commit list by default in sheriff-o-matic.

Tools/GardeningServer/ui/ct-commit-list.html

Index: Tools/GardeningServer/ui/ct-commit-list.html
diff --git a/Tools/GardeningServer/ui/ct-commit-list.html b/Tools/GardeningServer/ui/ct-commit-list.html
index 6ff19411001d97a75351606affa88d8304cde383..740540585199f21454c7fdb60e791c99d24a7569 100644
--- a/Tools/GardeningServer/ui/ct-commit-list.html
+++ b/Tools/GardeningServer/ui/ct-commit-list.html
@@ -5,6 +5,7 @@ found in the LICENSE file.
 -->
 
 <link rel="import" href="ct-commit.html">
+<link rel="import" href="../bower_components/paper-button/paper-button.html">
 
 <polymer-element name="ct-commit-list" attributes="first last commits" noscript>
   <template>
@@ -12,10 +13,36 @@ found in the LICENSE file.
       :host {
         display: block;
       }
+      paper-button[repository] {
+        margin: .5em;
+        text-transform: lowercase;
+      }
+      repository-info {
+        display: block;
+      }
+      commit-list {
+        display: none;
+        padding-left: 1em;
+        padding-right: 1em;
+      }
+      commit-list[visible="true"] {
+        display: block;
+      }
     </style>
     <template repeat="{{ repository in commits.commits | _repositories }}">
-      <template repeat="{{ commit in repository | _commits }}">
-        <ct-commit data="{{ commit }}"></ct-commit>
+      <template if="{{ !_repositoryEmpty(repository) }}">
+        <repository-info>

[ojan, 2014/08/06 23:45:55]

It's kinda weird to define new elements that are not actual components. Just use
<div class="repository-info">.

[dsinclair, 2014/08/12 01:11:19]

Done.

+          <paper-button raisedbutton role="button"

[ojan, 2014/08/06 23:45:55]

I'm sensitive to taking up too much screen real estate for these buttons. Maybe
use a core-icon like in https://codereview.chromium.org/410483002? Then, when
the thing is closed show a XXXX:YYYY style range (and have an onclick on that)
and when it's open, don't show that and just show the commit list. We'd still
show the drop-down button to the left of the commit list in both cases, it just
wouldn't take up so much space.

[dsinclair, 2014/08/12 01:11:20]

I dropped the button and went with text and an arrow. I found it really weird
when I'd click on the text and it would disappear down to just the arrow, so
I've left the text displaying for now. In general, the amount of space in the
collapsed view is a lot less then the current system so I think it's better.

+              label="{{ repository }} {{ _range(repository) }}"
+              on-click="{{ _toggleRepository }} "
+              repository="{{ repository + _range(repository) }}"></paper-button>
+
+          <commit-list visible="{{ repositoryVisible[repository + _range(repository)] }}">

[ojan, 2014/08/06 23:45:55]

Use <template if> instead of attribute + display.

[dsinclair, 2014/08/12 01:11:19]

Done.

+            <template repeat="{{ commit in repository | _commits }}">
+              <ct-commit data="{{ commit }}"></ct-commit>
+            </template>
+          </commit-list>
+        </repository-info>
       </template>
     </template>
   </template>
@@ -25,6 +52,13 @@ found in the LICENSE file.
       first: null,
       last: null,
 
+      // FIXME: This is a hack. This hash ends up being shared between all of the
+      //   elements of this type. We name the repos above with name and commit
+      //   information to tell them apart. If we don't make this 'global' we
+      //   end up losing the expansion information when we refresh the failure
+      //   list.

[ojan, 2014/08/06 23:45:55]

TL;DR: I think it's worth the effort to fix the underlying issue here.

It's a big-ish project, but I think the right solution for this is to fix the
fact that we blow away the whole model on each iteration. Instead we should
actually update the model. I think once
https://codereview.chromium.org/448503003 lands, you'll be able to do this since
we'll have CTFailureGroup objects in the model each with an almost unique key.
The key isn't totally unique yet, but we'll have to make it be unique for all
the features that depend on this, so lets just assume that it is for now. :)

That way we can add/update/remove the CTFailureGroups as appropriate instead of
blowing away the whole list each time. In addition to making it so we don't have
to go through strange contortions to avoid putting things on the model, it will
mean that we don't blow away and recreate the whole DOM each 30 seconds. :)

[dsinclair, 2014/08/12 01:11:19]

Acknowledged.

+      repositoryVisible: {},
+
       _repositories: function(commits) {
         if (!commits)
           return [];
@@ -48,6 +82,22 @@ found in the LICENSE file.
 
         return this.commits.range(repository, first + 1, last);
       },
+
+      _repositoryEmpty: function(repository) {
+        return this._commits(repository).length === 0;
+      },
+
+      _range: function(repository) {
+        var commits = this._commits(repository);
+        if (commits.length === 0)
+          return "";
+        return commits[0].revision + " : " + commits[commits.length - 1].revision;

[ojan, 2014/08/06 23:45:55]

Nit: sugarjs has first() and last() on array.

[dsinclair, 2014/08/12 01:11:19]

Done.

+      },
+
+      _toggleRepository: function(event, detail, sender, target) {
+        var repoName = sender.getAttribute('repository');
+        this.repositoryVisible[repoName] = !this.repositoryVisible[repoName];
+      }
     });
   </script>
 </polymer-element>