theme_item_list() wraps the list in <div/> tags. However this function is used to generate pagers and menus, and with these lists, in HTML5 it's preferable to wrap the list in <nav/> tags. Of course we don't want to do this by default, as it will cause earlier versions of IE to fail, but we should offer it to users who want to make use of the functionality without having to override theme_item_list().
I'm attaching a patch that adds a new variable to theme_item_list(): $variables['nav_tags']. This is a boolean that when set to TRUE will output nav tags as a wrapper to the item list rather than div tags.
Comments
Comment #1
jaypanComment #2
jaypanRemoving my name from the assignment in the hope of getting some attention for this issue.
Comment #3
stefan lehmannComment #4
stefan lehmannTested and it works like a charm!
(Done during the Drupal South 2014 Code Sprint in Wellington)
Comment #5
quietone commentedComment #6
jaypanDo you mean you guys had the same idea at that code sprint?
If so, great minds think alike :D
Comment #7
stefan lehmannHaha .. no .. just wanted to promote the event a bit. The test was done during the code sprint here. :-)
Comment #8
jaypanAhh gotcha. Thanks for testing!
Comment #9
joelpittetThis likely should be a template suggestion or something. It's being converted to a twig template over here #1939062: Convert theme_item_list() to Twig.
Hopefully it can be just twig template suggestion and remove the condition from the preprocess. If not this needs coding standards cleanup for the whitespace around the if statement, and xpath tests to make sure the desired markup makes the page.
Comment #10
star-szrThanks for this!
The patch no longer applies because this was recently converted to a Twig template, see #1939062: Convert theme_item_list() to Twig. I'm a bit wary of item-list.html.twig getting overly abstract but we could consider making the div that gets output in that template into a variable like wrapper_tag, similar to how we do list_type. The disadvantage is being able to grep the codebase to find the correct template, but I think overall that is curbed by being able to turn on twig_debug in settings.php and view source/inspect to see where markup is coming from.
(crossposted with @joelpittet but it's late so just clicking Save :P, discuss amongst yourselves)
Comment #11
joelpittetMy thought on the suggestions would be:
item-list--nav.html.twig would have the nav tags in it.
'item_list__nav'would be the #theme value. As Cottser suggests, discuss amongst yourselves on best approach.Comment #12
jaypanI'm switching this to D7, as it appears that a D8 patch is unnecessary. I'll post a patch in the next day or two.
Comment #13
jaypanDrupal 7 patch attached.
Comment #15
jaypanFixing bug, and adding forgotten documentation in last patch.
Comment #16
jaypanComment #18
lauriiiComment #19
lauriiiNow nav_tags variable is optional in order to fix the failed machine tests.
Comment #21
lauriiiSwitched the positions of div and nav options in the if. In the last patch the nav was set to element if returned false so everything was opposite.
Comment #22
lauriiiComment #23
rootworkThis needs a reroll. I tried to do it myself but it was beyond my abilities -- a lot has changed in these functions.
Comment #24
stefan lehmann21: drupal7core-add_variable_to_use_nav_tags_in_theme_item_list-2190803-21.patch queued for re-testing.
Comment #25
joelpittetI'd like to divert your attention to this solution that seems more awesome saucy: #1777332: Replace theme_menu_link() and menu-tree.html.twig with a single Twig template
Comment #26
saki007ster@joelpittet So what is it that you propose here. This issue could be a good example of how the twig should be used.
I am still not sure of the solution, also dont feel this issue to be novice anymore.