Issue 3097 - Refactored subscriptions list table on adblockplus.org/en/subscriptions

includes/subscriptionList.tmpl

Index: includes/subscriptionList.tmpl
diff --git a/includes/subscriptionList.tmpl b/includes/subscriptionList.tmpl
index 26451f01bb511826bc98abbbe4e5d9b8ea11d18c..44e8188bd75f99731d12868be8063cf4009a03fb 100644
--- a/includes/subscriptionList.tmpl
+++ b/includes/subscriptionList.tmpl
@@ -16,55 +16,63 @@
  #}
 
 {%- macro process_subscription(subscription, parent=None) %}
-  <tr{% if subscription["deprecated"] %} class="deprecated"{% endif %}>
-    {%- if parent and subscription["supplements"] %}
-      <td rowspan="2" class="dummy"></td>
-      <td rowspan="2">
-    {%- else %}
-      <td rowspan="2" colspan="2">
-    {%- endif %}
-      <strong class="subscriptionTitle">{{ subscription["name"] }}</strong><br>
-      {{ subscription["specialization"] }}
-      {%- if subscription["supplements"] %}
-      <br>{{ "supplement for"|translate("supplements_prefix") }} {{ ' / '.join(subscription["supplements"]) }} {{ " "|translate("supplements_suffix") }}
-      {%- endif %}
-    </td>
-    <td>
-      {%- if subscription["maintainer"] %}
-      {{ "by"|translate("maintainer_prefix") }} {{ subscription["maintainer"] }} {{ " "|translate("maintainer_suffix") }}<br>
-      {%- endif %}
-      {%- set is_first = True %}
-      {% for key, default in (('homepage', 'Homepage'), ('forum', 'Forum'), ('contact', 'Contact page'), ('faq', 'FAQ'), ('blog', 'Blog'), ('changelog', 'Changelog'), ('policy', 'Policy')) -%}
-        {%- set url = subscription[key] -%}
-        {%- if url -%}
-          {%- if not is_first %}, {% endif -%}
-          {%- set is_first = False -%}
-          <a href="{{ url }}">{{ default|translate(key) }}</a>
-        {%- endif %}
-      {%- endfor %}
-    </td>
-  </tr>
-  <tr{% if subscription["deprecated"] %} class="deprecated"{% endif %}>
-    <td>
-      {%- if subscription["deprecated"] %}
-      <strong>{{ "Note: This list is not optimized for Adblock Plus and may slow down your browsing experience."|translate("deprecation_warning") }}</strong><br>
-      {%- endif %}
-      {{ "Subscribe:"|translate("subscribe") }}{{ ' ' }}
-      {%- for title, url, complete in subscription["variants"] -%}
-        <a href="abp:subscribe?location={{ url|urlencode }}&amp;title={{ title|urlencode }}
-          {%- if parent and not complete -%}
-            {%- set main_title, main_url, main_complete = parent.variants[0] -%}
-            &amp;requiresLocation={{ main_url|urlencode }}&amp;requiresTitle={{ main_title|urlencode }}
-          {%- endif -%}
-        ">{{ title }}</a>{%- if not loop.last %}, {% endif -%}
-      {%- endfor %}
-    </td>
-  </tr>
-  {%- if not parent -%}
+  <li>
+    <article class="subscription {% if subscription["deprecated"] %}deprecated{% endif %}">
+      <div class="subscription-content">
+        <div class="subscription-summary">
+          <h1>{{ subscription["name"] }}</h1>
+          <p>
+            <span class="sr-only">{{ "Specialization"|translate("specialization") }}:</span>

[Thomas Greiner, 2016/07/15 16:07:53]

Detail: Not sure what "sr-only" means but it's not being used for styling so it
looks like you can remove the `<span>` around the text entirely.

[juliandoucette, 2016/07/19 22:29:17]

My mistake.

"sr" stands for "screen reader".

I forgot to add styles for this class.

+            {{ subscription["specialization"] }}
+          </p>
+          {%- if subscription["supplements"] %}
+          <p>{{ "supplement for"|translate("supplements_prefix") }} {{ ' / '.join(subscription["supplements"]) }} {{ " "|translate("supplements_suffix") }}.</p>

[Thomas Greiner, 2016/07/15 16:07:53]

Detail: Mind splitting this content (and the content from other paragraphs) up
into multiple code lines to make it easier to read? Because I noticed you did
that in the one above and it shouldn't have any impact on how the text is
displayed on the page.

[juliandoucette, 2016/07/19 22:29:16]

Done.

+          {%- endif %}
+        </div>
+      </div>
+      <footer class="subscription-details">
+        <div class="subscription-authors">

[Thomas Greiner, 2016/07/15 16:07:53]

Detail: Since you've decided to use `<footer>` for containing the filter list
meta data, what about using `<address>` instead of this `<div>` here?

See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/address

[juliandoucette, 2016/07/19 22:29:16]

Good idea.

+          {%- if subscription["maintainer"] %}
+          <p>{{ "by"|translate("maintainer_prefix") }} {{ subscription["maintainer"] }} {{ " "|translate("maintainer_suffix") }}</p>
+          {%- endif %}
+          {%- set is_first = True %}
+          <p>
+          {% for key, default in (('homepage', 'Homepage'), ('forum', 'Forum'), ('contact', 'Contact page'), ('faq', 'FAQ'), ('blog', 'Blog'), ('changelog', 'Changelog'), ('policy', 'Policy')) -%}
+          {%- set url = subscription[key] -%}
+            {%- if url -%}
+              {%- if not is_first %}, {% endif -%}
+              {%- set is_first = False -%}
+              <a href="{{ url }}">{{ default|translate(key) }}</a>
+            {%- endif %}
+          {%- endfor %}
+          </p>
+        </div>
+        <div class="subscription-links">
+          {%- if subscription["deprecated"] %}
+          <p><strong>{{ "Note: This list is not optimized for Adblock Plus and may slow down your browsing experience."|translate("deprecation_warning") }}</strong></p>
+          {%- endif %}
+          <p>
+          {{ "Subscribe:"|translate("subscribe") }}{{ ' ' }}
+            {%- for title, url, complete in subscription["variants"] -%}
+            <a href="abp:subscribe?location={{ url|urlencode }}&amp;title={{ title|urlencode }}
+              {%- if parent and not complete -%}
+                {%- set main_title, main_url, main_complete = parent.variants[0] -%}
+                &amp;requiresLocation={{ main_url|urlencode }}&amp;requiresTitle={{ main_title|urlencode }}
+              {%- endif -%}
+            ">{{ title }}</a>{%- if not loop.last %}, {% endif -%}
+            {%- endfor %}
+          </p>
+        </div>
+      </footer>
+    </article>
+    {%- if subscription["supplemented"] -%}
+    <ul class="supplemented-subscriptions-list">
     {%- for supplement in subscription["supplemented"]|subscription_sort -%}
       {{ process_subscription(supplement, subscription) }}
     {%- endfor -%}
-  {%- endif -%}
+    </ul>
+    {%- endif -%}
+  </li>
 {%- endmacro %}
 
 {% macro display_subscriptions(subscriptions) %}
@@ -72,17 +80,17 @@
     {%- if not (subscription["supplements"] and current_type == subscription["type"]) -%}
       {%- if current_type != subscription["type"] -%}
         {%- if current_type %}
-  </table>
+  </ul>
         {%- endif -%}
         {%- set current_type = subscription["type"] %}
   <h2 id="type_{{ current_type }}">{{ get_string("type_" + current_type, "subscriptions") }}</h2>
 
-  <table class="subscriptions">
+  <ul class="subscriptions-list">
       {%- endif -%}
     {%- endif -%}
     {%- if subscription["type"] not in subscription["supplementsType"] -%}
       {{ process_subscription(subscription) }}
     {%- endif -%}
   {%- endfor %}
-  </table>
+  </ul>
 {% endmacro %}