nerdyator / feedjack

Automatically exported from code.google.com/p/feedjack
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

sinx template produces deeply-nested html because of "ifchanged" content #41

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
I've found whole "channel" block faulty in this template, at least for my
feeds-collection.
Looks like it shouldn't be working right at all, and that's probably masked
with the default theme, which doesn't style "channel" blocks.

Right now it looks like this:

{% for item in object_list %}
...
{% ifchanged %}
<!-- {{ item.date_modified|date:"F j, Y" }} -->
<div class="channel">
    ...(feed header)...
{% endifchanged %}

...(entry)...

{% ifchanged %}
<!-- {{ item.date_modified|date:"F j, Y" }} {{ item.feed.link }} -->
<!-- End .channel -->
<br class="clear"/>
</div>
{% endifchanged %}
...
{% endfor %}

Problem is that if two distinct feeds are from one portal, they often have
the same link, so the "channel" block doesn't get closed, producing deep
tag nesting if this feeds' posts mix frequently enough.
For me it also cause major design breakage, but I'm using a bit modified
sinx theme, so it might be not so bad for vanilla one.
Another problem with the whole approach is that the "channel" blocks get
closed in completely wrong order - closing "ifchanged" part gets triggered
at the new "channel" entry, closing just-opened "channel', not the one that
was opened before that. Furthermore, last "channel" don't get closed at all.

The solution is to either use feed.id as the "ifchanged" key, since it's
guaranteed-unique between the feeds, or add feed.link to the tag-opening
key, if such aggregation is intentional (although I doubt that, since whole
thing just looks like a faulty template logic).

I've changed it like this:

{% for item in object_list %}
...
{% with item.date_modified|date:"F j, Y" as date_key %}
{% ifchanged item.feed.id date_key %}
{% if not forloop.first %}
    <!-- End .channel -->
    <br class="clear"/>
</div>
{% endif %}
<div class="channel">
    <h2><a href="{{ item.feed.link }}" title="{{ item.feed.title }}">{{
item.feed.shortname }}</a></h2>
        ...
{% endifchanged %}
{% endwith %}

...(entry)...

{% if forloop.last %}
    <!-- End .channel -->
    <br class="clear"/>
</div>
{% endif %}
...
{% endfor %}

That way, the "ifchanged" content doesn't have to contain some special
html-comment magic and can actually contain any random stuff, yet still be
echoed only where it's intended to. Channels should close in the correct
order and last one should be properly closed as well.
Whole "with ... as date_key" wrapping is necessary because of this django
bug: http://code.djangoproject.com/ticket/5756

Original issue reported on code.google.com by mk.fraggod@gmail.com on 16 May 2010 at 7:04