Issue 41 in feedjack: sinx template produces deeply-nested html because of "ifchanged" content

[codesite...@google.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 41 by mk.fraggod: sinx template produces deeply-nested html
because of "ifchanged" content
http://code.google.com/p/feedjack/issues/detail?id=41

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


--
You received this message because you are subscribed to the Google Groups "Feedjack Updates" group.
To post to this group, send email to feedjack...@googlegroups.com.
To unsubscribe from this group, send email to feedjack-updat...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/feedjack-updates?hl=en.