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.

Files: 
CommentFileSizeAuthor
#21 drupal7core-add_variable_to_use_nav_tags_in_theme_item_list-2190803-21.patch2 KBlauriii
PASSED: [[SimpleTest]]: [MySQL] 40,972 pass(es). View
#19 drupal7core-add_variable_to_use_nav_tags_in_theme_item_list-2190803-19.patch2 KBlauriii
FAILED: [[SimpleTest]]: [MySQL] 40,725 pass(es), 1 fail(s), and 0 exception(s). View
#15 drupal7core-add_variable_to_use_nav_tags_in_theme_item_list-2190803-14.patch1.95 KBJaypan
FAILED: [[SimpleTest]]: [MySQL] 40,592 pass(es), 1 fail(s), and 1 exception(s). View

Comments

Jaypan’s picture

Status: Active » Needs review
FileSize
1.76 KB
PASSED: [[SimpleTest]]: [MySQL] 63,360 pass(es). View
Jaypan’s picture

Assigned: Jaypan » Unassigned

Removing my name from the assignment in the hope of getting some attention for this issue.

Stefan Lehmann’s picture

Issue tags: +Novice
Stefan Lehmann’s picture

Tested and it works like a charm!

(Done during the Drupal South 2014 Code Sprint in Wellington)

quietone’s picture

Status: Needs review » Reviewed & tested by the community
Jaypan’s picture

(Done during the Drupal South 2014 Code Sprint in Wellington)

Do you mean you guys had the same idea at that code sprint?

If so, great minds think alike :D

Stefan Lehmann’s picture

Haha .. no .. just wanted to promote the event a bit. The test was done during the code sprint here. :-)

Jaypan’s picture

Ahh gotcha. Thanks for testing!

joelpittet’s picture

Status: Reviewed & tested by the community » Needs work

This 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.

Cottser’s picture

Issue tags: +Twig

Thanks 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)

joelpittet’s picture

My 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.

Jaypan’s picture

Version: 8.x-dev » 7.x-dev
Issue tags: -Twig

I'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.

Jaypan’s picture

Status: Needs work » Needs review
FileSize
1.53 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in includes/theme.inc. View

Drupal 7 patch attached.

Status: Needs review » Needs work
Jaypan’s picture

FileSize
1.95 KB
FAILED: [[SimpleTest]]: [MySQL] 40,592 pass(es), 1 fail(s), and 1 exception(s). View

Fixing bug, and adding forgotten documentation in last patch.

Jaypan’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
lauriii’s picture

Assigned: Unassigned » lauriii
lauriii’s picture

Status: Needs work » Needs review
FileSize
2 KB
FAILED: [[SimpleTest]]: [MySQL] 40,725 pass(es), 1 fail(s), and 0 exception(s). View

Now nav_tags variable is optional in order to fix the failed machine tests.

Status: Needs review » Needs work
lauriii’s picture

Status: Needs work » Needs review
FileSize
2 KB
PASSED: [[SimpleTest]]: [MySQL] 40,972 pass(es). View

Switched 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.

lauriii’s picture

Assigned: lauriii » Unassigned
rootwork’s picture

Status: Needs review » Needs work

This needs a reroll. I tried to do it myself but it was beyond my abilities -- a lot has changed in these functions.

Stefan Lehmann’s picture

Status: Needs work » Needs review
joelpittet’s picture

I'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

saki007ster’s picture

Issue tags: -Novice +Twig

@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.