Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.<menu> has made a comeback in HTML5, we can use menu to build menubar/toolbar type menus.
theme_links seems like a reasonable function to use to build a menubar (e.g. toolbar module uses theme_links), however its hard coded to use <ul>, so in the interest of flexibility I propose we could add a type parameter to theme_links e.g.
$type = $variables['type'];
...
$output .= "<$type" . drupal_attributes($attributes) . '>';
...
$output .= "</$type>";
http://api.drupal.org/api/drupal/includes--theme.inc/function/theme_links/8
Tangentially related issue: #1190210: Convert toolbar.tpl.php to HTML5
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 1203908-add_type_theme_links_with_options.patch | 1.87 KB | h3rj4n |
| #6 | 1203908-add_type_theme_links_without_options.patch | 1.78 KB | h3rj4n |
| #3 | add_type_theme_links-1203908-3.patch | 1.53 KB | jessebeach |
| #1 | add_type_theme_links-1203908-1.patch | 1.82 KB | jessebeach |
Issue fork drupal-1203908
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #1
jessebeach commentedI've been wanting to do this for a long, long time. It took me a little while to figure out where the default arguments were being set, but once I found them in common.inc, it was smooth sailing.
Comment #2
jessebeach commentedAssigning this to me.
Comment #3
jessebeach commentedApparently using the array key "#type" threw the contextual links into a tizzy.
https://skitch.com/jesse.beach/feb93/developer-tools-http-drupal.dev
Changed the array key so that the contextual links render correctly.
Comment #4
ericduran commentedI think we might need to check the $list variable needs before just printing it.
Comment #5
ericduran commentedcheck plain should be enough being that the only expected option is a text string.
Comment #6
h3rj4n commentedRewrote the patch to work with the new folder structure.
I think this feature improves HTML5 in Drupal. It might be better that the $list value is limited by a set of options to prevent the li tags of being wrapped out of context. Not shure if this should be checked on this location or that this is something a user should be aware of.
I created two patches. One with the limited options and one without it.
Comment #7
h3rj4n commentedforgot to set it to needs review
Comment #8
jessebeach commented@ericduran, good thought about checking plain. We should be covered though, because the only three values allowed for list are 'menu', 'ul', or 'ol'.
It should be possible to sneak anything else by.
This is really one of those Drupal WTF in the theming layer.
Thanks for updating this @h3rj4n.
I'm setting to RTBC to get this on folks' radars. It shouldn't be controversial.
Comment #9
catchThis needs tests.
Prefer the version that limits to known tags, definitely don't want a check plain here - if you let people put arbitrary HTML via a UI then you're responsible for adding appropriate permissions to that UI.
Comment #10
mgifford#6: 1203908-add_type_theme_links_with_options.patch queued for re-testing.
Comment #11
mgifford6: 1203908-add_type_theme_links_with_options.patch queued for re-testing.
Comment #13
joelpittetI'd rather not do this dynamic tags approach, maybe a template suggestion would be better but not sure. Regardless of my opinion this is not going to be in 8.0.0 so we need to move it to 8.1.x feature if someone wants to continue this approach.