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

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

 <!--
 Copyright 2014 The Chromium Authors. All rights reserved.
 Use of this source code is governed by a BSD-style license that can be
 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>
     <style>
       :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>
   <script>
     Polymer({
       commits: null,
       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 [];
         return Object.keys(commits).sort();
       },
 
       _commits: function(repository) {
         var commits = [];
         if (!this.first || !this.last)
           return commits;
 
         var first = Number(this.first[repository]);
         var last = Number(this.last[repository]);
         if (first == last)
           return commits;
 
         if (first > last) {
           console.warn('Revision range is backwards, which is invalid:', first, last);
           return commits;
         }
 
         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>