WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#29890 assigned enhancement

Make menu descriptions available to be displayed on the front-end

Reported by: obenland Owned by: helen
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: has-patch twenty-fifteen
Focuses: template Cc:

Description (last modified by obenland)

When the refreshed menu UI was introduced in r14248, it came with the possibility to add a description to each menu item. When that description textarea is displayed, it's accompanied by the help text:

The description will be displayed in the menu if the current theme supports it.

Yet there is currently no way for themes (that I'm aware of) to actually display these descriptions. Let's change that!

Added incentive would be one less callback in #29799 Twenty Fifteen.

Attachments (6)

29890.diff (1.7 KB) - added by obenland 3 years ago.
29890.2.diff (1.7 KB) - added by obenland 3 years ago.
29890.3.diff (1.9 KB) - added by obenland 3 years ago.
29890.4.diff (5.7 KB) - added by DrewAPicture 3 years ago.
'item_descriptions'
29890.5.diff (5.7 KB) - added by DrewAPicture 3 years ago.
'show_descriptions'
29890.6.diff (5.7 KB) - added by obenland 3 years ago.
Use div instead of span

Download all attachments as: .zip

Change History (39)

@obenland
3 years ago

#1 @obenland
3 years ago

I"m not sure which argument would be the most appropriate, description or show_description or display_description. Thoughts?

#2 @obenland
3 years ago

  • Summary changed from Make menu descriptions available to be displayed to Make menu descriptions available to be displayed on the front-end

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.


3 years ago

@obenland
3 years ago

#4 @cainm
3 years ago

I think it'd be better to have the description inside the anchor tag (and thus before the link_after output). That would follow the general UI of each 'block' within a navigation element being a clickable group. (i.e. email clients)

#5 @iamtakashi
3 years ago

Yes. I'm with cainm. Larger clickable area will be helpful for touch devices too.

@obenland
3 years ago

#6 @obenland
3 years ago

That makes sense. 29890.3.diff puts it inside the anchor tag.

@DrewAPicture
3 years ago

'item_descriptions'

#7 follow-up: @DrewAPicture
3 years ago

In 29890.4.diff I went with 'item_descriptions' instead of 'show_description'. This is because the latter implies a single menu description whereas the former implies individual item descriptions.

#8 in reply to: ↑ 7 ; follow-up: @cainm
3 years ago

Replying to DrewAPicture:

In 29890.4.diff I went with 'item_descriptions' instead of 'show_description'. This is because the latter implies a single menu description whereas the former implies individual item descriptions.

I think @obenland was following the precedent set with functions like wp_list_pages, where 'show_date' is used.

#9 in reply to: ↑ 8 @DrewAPicture
3 years ago

Replying to cainm:

Replying to DrewAPicture:

In 29890.4.diff I went with 'item_descriptions' instead of 'show_description'. This is because the latter implies a single menu description whereas the former implies individual item descriptions.

I think @obenland was following the precedent set with functions like wp_list_pages, where 'show_date' is used.

Yep, I was just talking to him on Skype :)

In 29890.5.diff I went instead with 'show_descriptions'. That at least implies the intent.

Argument naming can be hard. The general rule of thumb is to consider the purpose of the function you're adding arguments for. In this case, using the singular 'show_description' implies that you're showing a description for the entire menu. Using the plural 'show_descriptions' implies there are more than one of them, and from that we can indirectly infer that this argument applies to the multiple menu items within the menu this function generates.

#10 @obenland
3 years ago

Hm, looks like I ended up not submitting a comment I had written.

In 29890.2.diff:

  • Added a leading whitespace to the span element to avoid title and description running into each other.
  • Renamed the wp_nav_menu() argument to show_description to make it clearer it expects a boolean.

I do like Drew's suggestion of show_descriptions, to make it clearer it's about multiple item descriptions, and not a possible menu description.

#11 @obenland
3 years ago

  • Description modified (diff)

@DrewAPicture
3 years ago

'show_descriptions'

#12 @obenland
3 years ago

  • Keywords twenty-fifteen added

Tagging with twenty-fifteen to keep track of it.

#13 @toscho
3 years ago

I am not sure about the span. If the description contains markup (<p>), the span creates invalid HTML. Block elements are allowed in a elements now, so a div might be better.

#14 follow-up: @celloexpressions
3 years ago

The menus system goes to horrific lengths to support menu item descriptions (and there's some history with them always storing something like the original post's excerpt there by default, which caused even bigger issues), so I'm really surprised there isn't a good way to implement them in themes currently.

+1 for 29890.5.diff. While I think descriptions support html, they really shouldn't as there are enough problems with trying to save too much data in menu items anyway. I'm fine with <span> for now, then we can change it if needed once we see what people might want to do with them once it's in Twenty Fifteen in trunk.

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.


3 years ago

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.


3 years ago

@obenland
3 years ago

Use div instead of span

#17 @celloexpressions
3 years ago

HTML is currently stripped, see #26880. I'm strongly in favor of wontfixing that ticket, so that would mean that span would work fine. But div works too, of course.

Last edited 3 years ago by celloexpressions (previous) (diff)

#18 in reply to: ↑ 14 ; follow-up: @obenland
3 years ago

Replying to celloexpressions:

there's some history with [menu items] always storing something like the original post's excerpt there by default, which caused even bigger issues

This was in reference to #16799, which was fixed with r18733 and r18734.

There is a possibility that sites with menu items that were created between 3.0 and 3.1 have those old descriptions show up, when they switch to a theme that has show_descriptions enabled.

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by obenland. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by obenland. View the logs.


3 years ago

#22 @helen
3 years ago

I talked myself and obenland in circles in SF on this - basically, I think there's an issue of doing this on display as opposed to on registration. I think registration still makes sense - a menu location's design can support showing a description. There's no great way for a user to do anything about their menu descriptions even when they use the customizer to pre-flight the theme switch - if this is set on registration, perhaps we could allow the user to decide whether or not to show descriptions for that menu location.

Even on registration, since individual menus are decoupled from display areas (which are what get registered), we can't really make a smart decision about showing the description field when editing a menu. I think this may not be a big deal, since suddenly showing those old pre-populated descriptions on a theme change is an existing problem, but it kind of stinks. Perhaps a pointer is still warranted either way.

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


3 years ago

#24 @johnbillion
3 years ago

  • Owner set to helen
  • Status changed from new to assigned

Need to make a decision on this.

#25 in reply to: ↑ 18 @philiparthurmoore
3 years ago

Replying to obenland:

There is a possibility that sites with menu items that were created between 3.0 and 3.1 have those old descriptions show up, when they switch to a theme that has show_descriptions enabled.

This happened on my site last week when I updated the site's active theme to use menu descriptions in the same manner that they are handled in Twenty Fifteen. I really wish I had recorded what happened, but a very large post content dump was output for the menu description on an old menu that I had created a while back. I had to manually erase them and enter new descriptions.

Other than the above issue I am generally in large favor of this proposal. If the description field is available we should be able to make use of it in themes.

#26 follow-up: @philiparthurmoore
3 years ago

Yep, exactly this:

			if ( empty( $menu_item->description ) ) {
				/**
				 * Filter a navigation menu item's description.
				 *
				 * @since 3.0.0
				 *
				 * @param string $description The menu item description.
				 */
				$menu_item->description = apply_filters( 'nav_menu_description', wp_trim_words( $menu_item->post_content, 200 ) );
			}

That's inside of wp_setup_nav_menu_item.

I'm really not sure of how best to handle this, but looking at your latest patch in https://core.trac.wordpress.org/attachment/ticket/29890/29890.6.diff I'm curious why there's no output filter via nav_menu_description available to Walker_Nav_Menu. How would you feel about using nav_menu_description in your patch to allow theme developers to filter that output for whatever reason?

For example, if I know that a user is on an old version of WordPress and has a ton of trimmed post content stored in his menu descriptions, I could modify that output via nav_menu_description from within the theme for front-end display.

This ticket was mentioned in Slack in #core-themes by obenland. View the logs.


3 years ago

#28 in reply to: ↑ 26 @obenland
3 years ago

Replying to philiparthurmoore:

For example, if I know that a user is on an old version of WordPress and has a ton of trimmed post content stored in his menu descriptions, I could modify that output via nav_menu_description from within the theme for front-end display.

It's unfortunately not that easy, I'm afraid. We can't make assumptions based on the WordPress version. If a user has menu items that were created during that time, the description contains the post content, even today, even when their install is current, like in your case.

Short of adding a user option to toggle the display of menu item descriptions, there is really not a whole lot we can do to mitigate the risk of running into a situation where they have long descriptions show up. The only comforting thought is, that it's only an issue with old menu items, it'll only show up on theme change, and it's something that users will probably catch when they preview their site with the new theme.

#29 @philiparthurmoore
3 years ago

It's a good point that you make, for sure. Either way I really like that this will be available to themes.

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by obenland. View the logs.


3 years ago

#32 @obenland
3 years ago

  • Milestone changed from 4.1 to Future Release

Punted, based on chat linked to above.

#33 @obenland
3 years ago

#32599 was marked as a duplicate.

Note: See TracTickets for help on using tickets.