Merge lp:~huwshimi/launchpad/table-headings-728187 into lp:launchpad

[Huw Wilkins]

[r=benji][bug=728187] On mirror pages: fixed table headings to be consistent with other tables in LP, removed redundant inline styles and replaced some with external CSS and fixed bottom table lines length.

Fixed table headings to be consistent with other tables in LP (bug #728187). Because this has multiple table headers I couldn't just reuse the existing classes.

Also removed redundant inline styles and replaced some with external CSS.

Fixed bottom table lines length.

A screenshot of all these fixes is here: https://launchpadlibrarian.net/74192351/table_headers.png

[Benji York (benji)]

This branch looks good.

One thought: if it works, it'd be a slight improvement (IMHO) to use
colspan="5" for the blank row instead of having five table cells. In
fact it might be even nicer to give the last row a custom class and
style that row such that the spacing is achieved without having a blank
row.

review: Approve (code)

[Huw Wilkins (huwshimi)]

I have modified this to remove the empty cells and added a css classes to manage height and style. I could not remove the row altogether as we still require the surrounding borders (I guess technically this is possible, but only by adding more HTML/CSS).

[Benji York (benji)]

I like it.

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-06-28 19:24:39 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-07-01 10:34:30 +0000
@@ -518,6 +518,7 @@
 th, td {
     vertical-align: baseline;
     }
+.head th,
 thead th, tr.thead th {
     text-align: left;
     vertical-align: bottom;
@@ -624,6 +625,7 @@
 table.listing th {
     font-weight: bold;
     }
+table.listing .head, table.listing .head th,
 table.listing thead, table.listing thead th, table.listing tfoot tr,
 table.listing tr.thead th {
     border: 1px solid #d2d2d2;
@@ -658,6 +660,12 @@
     border: 1px #d2d2d2;
     border-style: dotted none none none;
     }
+table.listing .section-break td {
+    border-width: 1px 0 0 0;
+    border-style: solid;
+    border-color: #d2d2d2;
+    height: 1em;
+    }
 table.listing .note td {
     border-style: none;
     }

=== modified file 'lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt'
--- lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt	2010-12-18 00:54:04 +0000
+++ lib/lp/registry/stories/distributionmirror/xx-distribution-mirrors.txt	2011-07-01 10:34:30 +0000
@@ -12,13 +12,13 @@
     ...     country = extract_text(header.find('th'))
     ...     mirrors = []
     ...     for tr in header.findNextSiblings('tr'):
-    ...         if 'highlight' in str(tr.attrs):
+    ...         if 'head' in str(tr.attrs):
     ...             print "%s: %s" % (country, mirrors)
     ...             country = extract_text(tr.find('th'))
     ...             if country == 'Total':
     ...                 break
     ...             mirrors = []
-    ...         elif 'lesser' in str(tr.attrs):
+    ...         elif 'section-break' in str(tr.attrs):
     ...             # This is an empty row to visually separate the mirrors
     ...             # from different countries, so we'll just skip it.
     ...             pass

=== modified file 'lib/lp/registry/templates/distributionmirror-macros.pt'
--- lib/lp/registry/templates/distributionmirror-macros.pt	2010-12-16 20:40:07 +0000
+++ lib/lp/registry/templates/distributionmirror-macros.pt	2011-07-01 10:34:30 +0000
@@ -16,16 +16,14 @@
   <table class="listing" id="mirrors_list">
   <tbody>
     <tal:country_and_mirrors repeat="country_and_mirrors mirrors_by_country">
-      <tr class="highlight">
-        <th colspan="2" style="text-align: left"
+      <tr class="head">
+        <th colspan="2" 
             tal:content="country_and_mirrors/country" />
-        <th style="text-align: left"
-            tal:content="country_and_mirrors/throughput"/>
-        <th style="text-align: left" tal:condition="show_mirror_type">
+        <th tal:content="country_and_mirrors/throughput"/>
+        <th tal:condition="show_mirror_type">
           Type
         </th>
-        <th style="text-align: left"
-            tal:define="mirror_count country_and_mirrors/number">
+        <th tal:define="mirror_count country_and_mirrors/number">
           <tal:count replace="mirror_count" />
           <span tal:condition="python:mirror_count == 1">mirror</span>
           <span tal:condition="python:mirror_count != 1">mirrors</span>
@@ -59,20 +57,19 @@
         Include a blank row after the last entry of a country to provide
         vertical spacing to separate the next country.
       </tal:comment>
-      <tr class="lesser">
-        <td>&nbsp;</td>
+      <tr class="section-break">
+        <td colspan="5" />
       </tr>
 
     </tal:country_and_mirrors>
-    <tr class="highlight">
-      <th colspan="5" style="text-align: left; font-weight: bold;">Total</th>
+    <tr class="head">
+      <th colspan="5" >Total</th>
     </tr>
     <tr>
       <td colspan="2" />
-      <td style="text-align: left; font-weight: bold;"
-          tal:content="total_throughput" />
+      <td tal:content="total_throughput" />
       <td tal:condition="show_mirror_type"></td>
-      <td style="text-align: left; font-weight: bold;">
+      <td>
           <tal:count replace="total_mirror_count" />
           <span tal:condition="python:total_mirror_count == 1">mirror</span>
           <span tal:condition="python:total_mirror_count != 1">mirrors</span>