Posted by acouch on April 20, 2009 at 4:31pm
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | node system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
This is a patch to add classes to the 'Create Content' node add list.
This would allow the adding of icons to the node add list which would increase usability of that form.
This is a two line addition to the theme_node_add_list function.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| theme_node_add_list.patch | 994 bytes | Idle | Passed: 11305 passes, 0 fails, 0 exceptions | View details | Re-test |
Comments
#1
#2
Setting this to 'needs review'.
#3
Using the 'access_arguments' seems a little odd, although I can see you are limited in what the menu system gives you. I think it will be better to use the 'href' and pull the node/add/ off the front of it.
Also change node-add-list to node-type-list for consistency.
#4
Thanks for the feedback. I implemented your suggestions. The patch is resubmitted.
#5
$class = str_replace("node/add/","",($item['href']));should be
$class = str_replace('node/add/', '', $item['href']);(spacing after commas, removal of extraneous parentheses, use of simpler single quotes)
<dt class="node-type-list-'. $class . '">needs a space before the . (this spacing standard was changed for D7)#6
Thanks for the feedback. I implemented your suggestions. The patch is resubmitted.
#7
Looks good to me. Adding in the classes themers need is a good thing.
#8
Works as advertised, but I wonder why we don't use theme_item() and them_item_list() for a more consistent display of lists of items throughout the admin interface.
#9
hey jpoesen,
good point. i looked into it and theme_item_list() is currently broken but being updated: http://drupal.org/node/256827 . i think it would be good to try and make sure that classes more than just 'first' and 'last' are able to be added and are added for default elements.
i'm also looking at some other theme functions like theme_admin_block_content which themes the admin page but doesn't give classes so elements can have icons added.
#10
A couple things with this line:
+ $class = str_replace('node/add/','',($item['href']));1. There should be spaces after those commas.
2. I don't understand why $item['href'] is in parentheses. Looks like they can be removed.
3. It might be nice to have a comment here so themers understand what those classes are intended for? This might also be unnecessary. I am not a themer, so I defer to experts. :)
Also, should we leave the old "node-type-list" in addition to also adding this new class? It seems like you wouldn't be able to apply a style to all of the titles without duplicating an awful lot of rules?
#11
New patch for D8.
I don't think code comments in here will help themers, as they will see the classes in the resulting markup.