WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 4 months ago

Last modified 4 months ago

#39350 closed enhancement (invalid)

Twenty Seventeen: Customise location of the Social Links Menu

Reported by: pento Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: Bundled Theme Keywords: needs-patch dev-feedback
Focuses: Cc:

Description

It's currently only possible to have the Social Links menu in the footer, which isn't ideal for infinite scroll sites. I'd like to be able to move it to a custom menu midget in the sidebar, but that changes the menu to display link text instead of social icons.

Attachments (1)

39350.1.patch (3.3 KB) - added by sami.keijonen 9 months ago.

Download all attachments as: .zip

Change History (15)

#1 @sami.keijonen
9 months ago

My first reaction is that we might need custom widget for this. But I really haven't looked any further.

If we need custom widget would that be ok in default theme?

#2 @davidakennedy
9 months ago

Thanks for the request @pento!

This is an interesting request, and something I'd consider. It can be accomplished with a child theme.

This doesn't work out of the box because the markup is different. Plus, the social navigation uses other special functions to display the SVG icons. So it's harder to pull this off, as it is now.

As @sami.keijonen mentions, a custom widget might be needed to do this. I wouldn't be for adding a custom widget for this. But maybe there's a way to do this more easily and with less code... I'd be open to ideas.

Also, I'd want to hear from @melchoyce about any necessary tweaks to the icon styles, like sizing, to make sure they would look good in the sidebar area.

#3 @sami.keijonen
9 months ago

I'd like to see this possible without custom widget. It would be nice feature for themes that want to support it. But I need to take closer look is that possible without changes in core.

There is a filter widget_nav_menu_args which can be a starting point.

function prefix_widget_social_menu( $nav_menu_args, $nav_menu, $args, $instance ) {
	if ( ! has_nav_menu( 'social' ) ) {
		return;
	}
	
	$nav_menu_args['theme_location'] = 'social';
	$nav_menu_args['link_before']    = '<span class="screen-reader-text">';
	$nav_menu_args['link_after']     = '</span>' . twentyseventeen_get_svg( array( 'icon' => 'chain' ) );
	$nav_menu_args['menu_class']     = 'social-links-menu';
	$nav_menu_args['depth']          = 1;
	return $nav_menu_args;
}
add_filter( 'widget_nav_menu_args', 'prefix_widget_social_menu', 10, 4 );

But that messes with all widget nav menus. How can we check that menu assigned to widget nav menu is the same as assigned to social menu location?

#4 @sami.keijonen
9 months ago

How about like this:

function prefix_widget_social_menu( $nav_menu_args, $nav_menu, $args, $instance ) {
	$menu_location = 'social';
	$menu_name     = wp_get_nav_menu_object( get_nav_menu_locations( $menu_location )[ $menu_location ] )->name;

	if ( $nav_menu->name !== $menu_name || ! has_nav_menu( 'social' ) ) {
		return $nav_menu_args;
	}

	$nav_menu_args['theme_location'] = 'social';
	$nav_menu_args['link_before']    = '<span class="screen-reader-text">';
	$nav_menu_args['link_after']     = '</span>' . twentyseventeen_get_svg( array( 'icon' => 'chain' ) );
	$nav_menu_args['menu_class']     = 'social-links-menu';
	$nav_menu_args['depth']          = 1;

	return $nav_menu_args;
}
add_filter( 'widget_nav_menu_args', 'prefix_widget_social_menu', 10, 4 );

#5 @sami.keijonen
9 months ago

Here is first patch what I had in mind. I'd like to see similar styles than in the footer.

Note that I haven't styled any of the dark or custom color schemes.

#6 @melchoyce
9 months ago

While I like the idea, I don't think this should apply to all custom widget menus, and I'm definitely not into the idea of making a custom widget just for this theme. Honestly, this seems like child theme territory to me. Is there a way we could make it easier for a child theme to accomplish? Or, could we add some CSS classes folks could add to their menu items that would turn the menu items into social icons?

#7 follow-up: @sami.keijonen
9 months ago

@melchoyce: Check my patch.

  • No need for custom widget.
  • Works only for the same widget menu what you assign to social menu.
  • Styling would be similar than in the footer.
Last edited 9 months ago by sami.keijonen (previous) (diff)

#8 in reply to: ↑ 7 @melchoyce
9 months ago

Replying to sami.keijonen:

@melchoyce: Check my patch.

  • No need for custom widget.
  • Works only for the same widget menu what you assign to social menu.
  • Styling would be similar than in the footer.

Cool, will try to get my test site running again. If you're adding code blocks, can you also add a description and screenshots in the future to make it easier to review? I'm not a developer and can't understand the code you posted, so I wasn't able to tell what it did.

#9 @sami.keijonen
9 months ago

Here is image where I have set the same menu in widget than in social menu in the footer.

https://www.dropbox.com/s/qd6cfdy5qf9u8uf/twenty17-social-icons-widget.png?dl=0

#10 @sami.keijonen
4 months ago

@davidakennedy and @melchoyce: What do you think about this feature in Twenty17?

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


4 months ago

#12 @davidakennedy
4 months ago

@sami.keijonen Thanks for the work on this! I appreciate the creative thinking and coding.

Your currently proposed way is better than a custom widget... but it could be confusing to people. Right now, in order to make the menu appear in the widget area, you have to have it assigned to the Social Links Menu spot. Unless I'm missing something. Which means the menu will appear twice on sites. Which some people may not want. :)

All that said, I don't want to add more complexity to the theme, even if it's for a good feature like this. Not sure there's a way around that here. I'm a no on this in its current form, and probably leaning toward no overall. Happy to listen to other ideas or iterations.

#13 @sami.keijonen
4 months ago

  • Resolution set to invalid
  • Status changed from new to closed

No worries. Anyways here is the code for child themes (and for my own themes:)

#14 @netweb
4 months ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.