Issue 2823 - Display new subscription types on subscriptions page

includes/subscriptionList.tmpl

 {#
  # This file is part of the Adblock Plus website,
  # Copyright (C) 2006-2015 Eyeo GmbH
  #
  # Adblock Plus is free software: you can redistribute it and/or modify
  # it under the terms of the GNU General Public License version 3 as
  # published by the Free Software Foundation.
  #
  # Adblock Plus is distributed in the hope that it will be useful,
  # but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 49 matching lines @@
       {%- endfor %}
     </td>
   </tr>
   {%- if not parent -%}
     {%- for supplement in subscription["supplemented"]|subscription_sort -%}
       {{ process_subscription(supplement, subscription) }}
     {%- endfor -%}
   {%- endif -%}
 {%- endmacro %}
 
 {% macro display_subscriptions(subscriptions) %}

[kzar, 2015/09/09 12:23:35]

The logic in this macro is getting increasingly hard to grok. The duplication is
getting worse as well. "type_" + current_type keeps showing up, the <h2> tags
are duplicated which sucks more the more complicated they get and the the
conditional logic is getting very confusing.

I don't have any concrete suggestions but can you see a way to tidy this all up?

[Thomas Greiner, 2015/09/15 13:21:05]

I agree, that this entire template is quite convoluted and confusing. A page
redesign in the future might help. However, for now I tried to clean it up a bit
to at least avoid duplication.

-  {%- set current_type = subscriptions[0]["type"] -%}
-  <h2 id="type_{{ current_type }}">{{ get_string("type_" + current_type, "subscriptions") }}</h2>
-
-  <table class="subscriptions">
   {%- for subscription in subscriptions|subscription_sort -%}
-    {%- if not subscription["supplements"] or current_type != subscription["type"] -%}
+    {%- if not (subscription["supplements"] and current_type == subscription["type"]) -%}
       {%- if current_type != subscription["type"] -%}
+        {%- if current_type %}
+  </table>
+        {%- endif -%}
         {%- set current_type = subscription["type"] %}
-  </table>
-
   <h2 id="type_{{ current_type }}">{{ get_string("type_" + current_type, "subscriptions") }}</h2>
 
   <table class="subscriptions">
       {%- endif -%}
     {%- endif -%}
     {%- if subscription["type"] not in subscription["supplementsType"] -%}
       {{ process_subscription(subscription) }}
     {%- endif -%}
   {%- endfor %}
   </table>
 {% endmacro %}