When there are no content items under the current filter at /admin/content/node, the "select all" checkbox at the top should not be shown. There are no items to select, so select all doesn't make any sense. See patch. This probably applies to HEAD also.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | node.module-select-all_4.patch | 1.32 KB | lyricnz |
| #10 | node.module-select-all_3.patch | 1.18 KB | Freso |
| #5 | node.module-select-all_2.patch | 1.17 KB | lyricnz |
| #3 | node.module-select-all_1.patch | 1.13 KB | lyricnz |
| #2 | node.module-select-all_0.patch | 1.09 KB | lyricnz |
Comments
Comment #1
lyricnz commentedMoving to head, more likely to get applied there :) then backported to 5.1
Comment #2
lyricnz commentedTested in 6.x. Fix typo.
Comment #3
lyricnz commentedReroll and apply from drupal root.
Comment #4
pwolanin commentedcode looks reasonable - but a little hard to read the inline conditional. Also - should the header cell get some content? a space or nbsp?- is it valid HTML to have it totally empty?
Comment #5
lyricnz commentedIt's definitely valid to have empty TD/TH cells, and it's actually rendered differently to a non-empty cell. This especially shows if you have a border defined for your table. Not sure that matters here.
Anyway, I've extracted the inline ?: into variable. Patch attached.
Comment #6
pwolanin commentedComment #7
gábor hojtsyWhy do we display the table at all, if there are no elements?
Comment #8
lyricnz commentedGotta put the "there's no content" message somewhere, I guess, but good point.
I figured we should leave the table there, because as users fiddle with their filters, it might be confusing to see the table appear and disappear.
Comment #9
gábor hojtsyThe possible confusion about displaying the table or not displaying it sometimes seems to be a valid point for me.
Comment #10
Freso commentedThe patch doesn't apply:
So I re-rolled against latest HEAD and tested it, with the conclusion that the patch works as advertised. Marking RTBC.
Comment #11
gábor hojtsyIt would be good to document in a comment, why isset($form['title']) && is_array($form['title']) leads to $has_posts. It looks like an interesting conditional. Otherwise all seem to look fine.
Comment #13
lyricnz commentedAgree, Gábor. We didn't create that condition, just moved it from further down. How about:
// If there are rows in this form, then $form['title'] contains a list of the title form elements.
Comment #14
lyricnz commentedFWIW the reason the patch didn't apply was that d6 has split node.module. I'll reroll a backport for d5 this is commited to d6.
Comment #15
Freso commentedlyricnz: As Gábor hasn't commented either way with regards to the comment, I think it'd be safe to make a new patch/re-roll the patch with the comment and mark this as "needs review".
Comment #16
lyricnz commentedAdded comment describing $has_posts.
Comment #17
gábor hojtsyLooks better, thanks, committed.
Comment #18
(not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.