Merge lp:~jstpierre/loggerhead/css-changes into lp:loggerhead

[Jasper St. Pierre]

[John A Meinel (jameinel)]

I like the idea of the changes. But I did a quick test and the annotation column on the left 'blew up' and consumed pretty much the whole screen.

I don't think it is really usable in that state. If we can find a way to prevent the annotation information from consuming so much width I think this is ready to land.

Another thing that would be nice, but certainly doesn't block this, is if the columns were split, such that selecting text would not give you the annotations. Certainly doesn't have to be done to get this landed.

But I think it would be a usability regression if we landed this as is, because many pages will push the content you want to see off to the right.

review: Needs Fixing

[Jasper St. Pierre (jstpierre)]

"the content you want to see"

Isn't the content you want to see the annotation?

Anyway, HTML is a mess, and I'd like to figure out how to do this properly, but after Googling and talking with people, it looks like max-width is ignored for tables, so I can't use that.

Ideally, I'd really like three columns:

=============================
 Annotation | Line | Content
=============================

 * Annotation should take up the minimum amount of space without wrapping, with a maximum width of 25%., in which case it should wrap.
 * Line should be 20px or so, like it is now.
 * Content should have all of the extra space, if there is any. Not Annotation or Line.

I don't feel this is possible with just HTML or CSS, which is extremely disappointing.

The closest I've gotten is:

td.annotation {
    max-width: 25%;
    white-space: nowrap;
}

But then Content doesn't get all of the extra space (it could be in Annotation).

[Robert Collins (lifeless)]

@jam are your concerned addressed? I think jasper is stalled at the moment.

[John A Meinel (jameinel)]

I don't think this is perfect, but my CSS is far worse than Jasper's. So I don't have anything better to put here, and I think on the whole it is better than the current layout.

review: Approve

=== modified file 'loggerhead/controllers/annotate_ui.py'
--- loggerhead/controllers/annotate_ui.py	2010-12-01 08:30:02 +0000
+++ loggerhead/controllers/annotate_ui.py	2011-04-22 21:35:57 +0000
@@ -16,6 +16,8 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 #
 
+import itertools
+
 from loggerhead.controllers.view_ui import ViewUI
 from loggerhead import util
 
@@ -29,23 +31,37 @@
         
         change_cache = {}
         last_line_revid = None
-        parity = 1
-        for line_revid, text in tree.annotate_iter(file_id):
-            if line_revid == last_line_revid:
-                # remember which lines have a new revno and which don't
-                new_rev = False
-            else:
-                new_rev = True
-                parity ^= 1
+        last_lineno = None
+        message = ""
+
+        revisions = {}
+
+        lineno = 0
+        for (line_revid, text), lineno in zip(tree.annotate_iter(file_id), itertools.count(1)):
+            if line_revid != last_line_revid:
                 last_line_revid = line_revid
-                if line_revid in change_cache:
-                    change = change_cache[line_revid]
-                else:
+
+                change = change_cache.get(line_revid, None)
+                if change is None:
                     change = self._history.get_changes([line_revid])[0]
                     change_cache[line_revid] = change
 
-            yield util.Container(
-                parity=parity, new_rev=new_rev, change=change)
+                message = change.comment.splitlines()[0]
+
+                if last_lineno:
+                    # The revspan is of lines between the last revision and this one.
+                    # We set the one for the previous revision when we're creating the current revision.
+                    revisions[last_lineno].revspan = lineno - last_lineno
+
+                revisions[lineno] = util.Container(change=change, message=message)
+
+                last_lineno = lineno
+                last_line_revid = line_revid
+
+        # We never set a revspan for the last revision during the loop above, so set it here.
+        revisions[last_lineno].revspan = lineno - last_lineno + 1
+
+        return revisions
             
     def get_values(self, path, kwargs, headers):
         values = super(AnnotateUI, self).get_values(path, kwargs, headers)
 
=== modified file 'loggerhead/controllers/view_ui.py'
--- loggerhead/controllers/view_ui.py	2011-03-23 02:37:07 +0000
+++ loggerhead/controllers/view_ui.py	2011-04-22 21:35:57 +0000
@@ -123,10 +123,10 @@
             raise HTTPMovedPermanently(self._branch.context_url(['/files', revno_url, path]))
 
         return {
-            # In AnnotateUI, "annotated" is a generator giving revision
-            # numbers per lines, but the template checks if "annotated" is
-            # true or not before using it, so we have to define it here also.
-            'annotated': False,
+            # In AnnotateUI, "annotated" is a dictionary mapping lines to changes.
+            # We exploit the fact that bool({}) is False when checking whether
+            # we're in "annotated" mode.
+            'annotated': {},
             'revno_url': revno_url,
             'file_id': file_id,
             'file_path': path,
 
=== modified file 'loggerhead/static/css/global.css'
--- loggerhead/static/css/global.css	2011-02-24 00:44:09 +0000
+++ loggerhead/static/css/global.css	2011-04-22 21:35:57 +0000
@@ -152,6 +152,7 @@
     float: left;
     display: inline;
     height: 22px;
+    line-height: 22px;
     padding: 0 0 0 18px;
     background: transparent url(../images/ico_description.gif) no-repeat center left;
     }
@@ -163,7 +164,6 @@
     }
 
 #branch-info {
-    width: 900px;
     padding: 10px;
     margin: 0 0 10px 0;
     color: #666;
 
=== modified file 'loggerhead/static/css/view.css'
--- loggerhead/static/css/view.css	2011-02-18 05:41:17 +0000
+++ loggerhead/static/css/view.css	2011-04-22 21:35:57 +0000
@@ -1,31 +1,30 @@
 /*table*/
-.viewLineTit,
-.viewRevTit,
-.viewContTit {
+#logentries,
+.viewRev,
+.viewLine {
     border: 1px solid #d2d2d2;
-    }
-.viewLine,
-.viewRev,
+}
+.viewRev {
+    border-right: none;
+    white-space: nowrap;
+    width: 1px;
+    padding: .3em .6em;
+}
 .viewComm,
 .viewCont {
     border: none;
     }
-.viewLine,
-.viewLineTit {
+.viewLine {
+    border-style: none solid none none;
+    width: 20px;
+    padding-left: .6em;
     padding-right: .2em;
     }
-.viewLine {
-    width: 37px;
-    }
-.viewContTit,
+.viewLine.first {
+    border-style: solid solid none none;
+}
 .viewCont {
-    width: auto;
-    padding-left: .3em;
-    }
-.viewRevTit,
-.viewRev {
-    width: 70px;
-    text-align: center;
+    padding-left: .4em;
     }
 .viewLine,
 .viewCont {
@@ -38,6 +37,11 @@
     margin: 0;
     }
 .viewLine {
-    text-align: right;
+    text-align: center;
+    }
+.viewLine {
     font-size: 93%;
     }
+.viewAuthors {
+    color: #999999;
+}
 
=== modified file 'loggerhead/templates/inventory.pt'
--- loggerhead/templates/inventory.pt	2011-02-23 03:01:15 +0000
+++ loggerhead/templates/inventory.pt	2011-04-22 21:35:57 +0000
@@ -19,22 +19,18 @@
           </tal:has-link>
       </tal:block>
       <tal:block metal:fill-slot="branchname" tal:content="string:${branch/friendly_name}" />
-      <tal:block metal:fill-slot="heading">
-      <div id="breadcrumbs">
+          <tal:block metal:fill-slot="heading">
+          <div id="breadcrumbs">
           <tal:has-link condition="branch/branch_link">
             <a tal:attributes="href branch/branch_link"
                tal:content="branch/friendly_name">
               nice/branch/name
             </a>
           </tal:has-link>
-          <span tal:condition="not: branch/branch_link">
-            <span metal:use-macro="breadcrumbs/directory" />
-          </span>
-          <tal:block condition="python:change">
-            &#187; Viewing
-            <span metal:use-macro="breadcrumbs/branch" />
-            for revision <span tal:content="change/revno"/>
-          </tal:block>
+          <tal:no-link condition="not: branch/branch_link">
+            <span metal:use-macro="breadcrumbs/directory"></span>
+          </tal:no-link>
+          <span>: <span metal:use-macro="breadcrumbs/branch" /> (revision <tal:revno content="change/revno"></tal:revno>)</span>
         </div>
       </tal:block>
 
 
=== modified file 'loggerhead/templates/view.pt'
--- loggerhead/templates/view.pt	2011-02-23 03:01:15 +0000
+++ loggerhead/templates/view.pt	2011-04-22 21:35:57 +0000
@@ -49,11 +49,11 @@
             </li>
             <li tal:condition="not:annotated">
               <a tal:attributes="href python:url(['/annotate', revno_url, file_path], clear=1)"
-              >view revision numbers per line</a>
+              >view with revision information</a>
             </li>
             <li tal:condition="annotated">
               <a tal:attributes="href python:url(['/view', revno_url, file_path], clear=1)"
-              >view without revision numbers</a>
+              >view without revision information</a>
             </li>
             <li>
                 <a tal:attributes="href python:url(['/revision', change.revno], clear=1)">view revision</a>
@@ -71,25 +71,24 @@
 
       <div class="view">
         <table id="logentries">
-          <tr class="logheader">
-            <th class="viewLineTit">Line</th>
-            <th class="viewRevTit" tal:condition="annotated">Revision</th>
-            <th class="viewContTit">Contents</th>
-          </tr>
-
           <tal:rep tal:repeat="line contents">
-            <tr tal:define="anno python:annotated and annotated.next()"
-                tal:attributes="class python:anno and 'blueRow' + str(anno.parity) or None">
-              <td class="viewLine">
+            <tr>
+              <td tal:condition="python:path('repeat/line/number') in annotated"
+                  tal:define="anno python:annotated.get(path('repeat/line/number'), None)"
+                  tal:attributes="rowspan python:anno.revspan"
+                  class="viewRev">
+                <a tal:content="python:'%s' % (anno.change.revno,)"
+                   tal:attributes="href python:url(['/revision', anno.change.revno], clear=1);
+                                   title python:'%s by %s, on %s (%s)' % (anno.change.revno, ', '.join(util.hide_emails(anno.change.authors)), anno.change.date.strftime('%d %b %Y %H:%M'), util.date_time(anno.change.date))"></a>
+                <span class="viewAuthors" tal:content="python:'by ' + ', '.join(util.hide_emails(anno.change.authors))" />
+                <br />
+
+                <span tal:content="python:anno.message"></span>
+              </td>
+              <td tal:attributes="class python:path('repeat/line/number') in annotated and 'viewLine first' or 'viewLine'">
                 <a tal:attributes="id string:L${repeat/line/number}; href string:#L${repeat/line/number}"
                    tal:content="repeat/line/number">1</a>
               </td>
-              <td class="viewRev" tal:condition="annotated">
-                <a tal:condition="python:anno.new_rev"
-                   tal:content="python:util.trunc(anno.change.revno)"
-                   tal:attributes="href python:url(['/revision', anno.change.revno], clear=1);
-                                   title python:'%s by %s, on %s (%s)' % (anno.change.revno, ', '.join(util.hide_emails(anno.change.authors)), anno.change.date.strftime('%d %b %Y %H:%M'), util.date_time(anno.change.date))"></a>
-              </td>
               <td class="viewCont"><pre tal:content="structure line"></pre></td>
             </tr>
           </tal:rep>