If I do
$variables = array(
'items' => array(
'Item one',
'Item two',
'Item three',
),
);
print theme('item_list', $variables);
$variables = array(
'items' => array(
'one' => 'Item one',
'two' => 'Item two',
'three' => 'Item three',
),
);
print theme('item_list', $variables);
The result will be:
<div class="item-list">
<ul>
<li class="first">Item one</li>
<li>Item two</li>
<li class="last">Item three</li>
</ul>
</div>
<div class="item-list">
<ul>
<li class="first">Item one</li>
<li class="first">Item two</li>
<li class="first">Item three</li>
</ul>
</div>
When using associative arrays, every item has the "first" class, and the "last" class is missing on the last one.
The documentation says nothing about associative arrays on the items parameter, so I think this is a bug that must be fixed. Otherwise we must fix the docs to clarify that only indexed arrays are allowed.
PS: I think this issue doesn't affect Drupal 8 due to the refactorings done at #256827: Various bugs in theme_item_list()
Comments
Comment #1
danillonunes CreditAttribution: danillonunes commentedDidn't do the tests locally, but I hope there's no test expecting this bugged behaviour.
Comment #2
danillonunes CreditAttribution: danillonunes commentedComment #3
danillonunes CreditAttribution: danillonunes commentedI also improved the tests to cover this case. This patch has just the test change, so now it must no pass.
Comment #4
danillonunes CreditAttribution: danillonunes commentedAnd this patch is #1 and #3 merged, aka the patch that must be commited if everything is ok.
Comment #5
danillonunes CreditAttribution: danillonunes commentedActually, the last test is redundant now and can be removed.
Comment #6
wundo CreditAttribution: wundo commentedI've tested locally is works perfectly.
Next time could you please name the patch file like this: [description]-[issue-number]-[comment-number].patch? http://drupal.org/node/1054616
Comment #7
danillonunes CreditAttribution: danillonunes commentedWell, so item 1 is the first and item *total number of items* is the last. I don't have to do a weird thing like
$i = -1;
at start just so I could use a weirdif ($i == $num_items - 1)
at the end.Comment #8
webchickHm. Not sure about this one. Assigning to David.
Comment #9
webchickOh, no I'm not. :) We need a 8.x patch first.
Comment #10
danillonunes CreditAttribution: danillonunes commented@webchick I'm reverting the changes because 8.x was already fixed by #256827: Various bugs in theme_item_list() :)
Comment #11
webchickAh, sorry! Missed that. David it is then. :) We don't normally change theme functions in stable releases. This particular case looks fairly harmless, but David is really good at sniffing out "harmless" from actual harmless. ;D
Comment #12
David_Rothstein CreditAttribution: David_Rothstein commentedLooks good to me.
However, the code in Drupal 8 behaves a bit differently from this, and as far as I can see nothing like the test additions in this patch were ever added to Drupal 8 (so this functionality doesn't actually seem to be tested there). That's not so good; I think the tests should be forward-ported.
But in the meantime, committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/ff1d0fc
Comment #13
star-szrIf we're just adding tests it's eligible for RC.