Blockbase: Changed the trigger to render social icons

[pbking]

Changes proposed in this Pull Request:

This changes the trigger to add/style social media icons from the Block attribute __unstableSocialLinks to the class social-links

This was done because when the templates that contain navigation blocks with an unstableSocialLinks attribute when saved that attribute is removed, thus social links are no longer rendered.

A class is used instead because it will persist across edits.

The logic used to determine if the social bits should be rendered has been updated to ensure that:

• It's on a navigation block with the __unstableLocation attribute (which IS sanctioned by Gutenberg and that was already the case for all template instances)
• That no custom menus have been added (via the FSE).
• That the block has the social-links class.

Additionally instead of getting the target menu location previously defined via the __unstableSocialLinks attribute the location of social is assumed. (All instances in the templates were using that location.)

Lastly all of the template parts/patterns that were leveraging the __unstableSocialLinks attribute have been changed to use the class social-links instead.

To test ensure the following scenarios work as expected:

A site using Blockbase (or Blockbase child) that has [primary|social] location(s) assigned (via customizer) and have no other modifications to their header continue to show the primary menu and/or social menu as expected.

With the change in place modify the header and make changes to any part of the header EXCEPT the navigation block. Note that the navigation continues to render as expected. (This is the bug noted in #6077)

Now modify the navigation block in the FSE (creating a menu from scratch, or assigning it an existing menu from, etc). Note that the social navigation items are no longer being shown now that the menu has been edited via the FSE.

Related issue(s):

Fixes #6077

[scruffian]

I'm not sure about this. The aim of FSE is that users will see the same in the frontend and the editor. It's not possible to add social links to the navigation using the customizer, which is why this was introduced, but once a user starts working in the Site Editor they can add their own social links to the navigation block, so there's no need for this feature.

If we are going to change how this works then I think my preference would be to just remove the feature entirely.

[pbking]

I've been on the fence about that. I don't think we can remove the feature entirely, that would change the navigation of existing installations and I'm keen to ensure that as the theme progresses we don't break anything that's already happening.

I'm keen to remove the feature, but I'm not sure that can be done without breaking said existing installations. I think the best scenario we can work toward is:

• Any menus that exist today can continue to work as designed.
• Update the customizer to direct the users to the FSE to make any further changes to their navigation instead of allowing it in the Customizer. (A separate issue)

The downside to keeping the social bits but not making this change is that users that already have navigation that includes social will lose that and will have to rebuild their navigation if they make any changes to the header template. I'm not sure how likely that is or if it's something we should worry about but that's the scenario I'm thinking about.

I'm also not sure if the navigation bits of the Customizer CAN be removed if we keep the navigation targets assigned so maybe that's not even an option.

[jffng]

With the change in place modify the header and make changes to any part of the header EXCEPT the navigation block. Note that the navigation continues to render as expected. (This is the bug noted in #6077)

Now modify the navigation block in the FSE (creating a menu from scratch, or assigning it an existing menu from, etc). Note that the social navigation items are no longer being shown now that the menu has been edited via the FSE.

I tested and verified the above scenarios with this PR.

[jffng]

If we are keen to remove this feature but not break existing installations, could we do something like the following?

Suggested change

	if ( defined( 'IS_GUTENBERG_PLUGIN' ) ) {
	$nav_menus = get_nav_menu_locations();
	if ( defined( 'IS_GUTENBERG_PLUGIN' ) && ! empty( $nav_menus[ 'social' ] ) ) {

This way, users who have already defined menus will continue to be able to edit them via the customizer. For fresh sites with no menus defined yet, they will have to go to the site editor to edit their navigation.

[pbking]

I was actually tinkering with exactly that! However I found that the 'get_nav_menu_locations' returns the menus that are defined in the code that follows. The 'locations' are what the theme provides. I was not able to discern what to call to determine if the user had any menus defined.

I think that's a good idea for the reasons you state, though I still think that checking for Gutenberg is still important. If that's not installed leveraging the 'classic menus' still wouldn't be supported.

[jffng]

I was thinking that we would only register the menu locations if there are already menus assigned to that location, which get_nav_menu_locations returns just that.

I was not able to discern what to call to determine if the user had any menus defined.

If we wanted to make the condition for if the user had any menus defined, regardless of whether they are assigned to a location, we could use https://developer.wordpress.org/reference/functions/wp_get_nav_menus/

[jffng]

To clarify my review — this PR fixes the issue in my testing (you can edit the header without losing your social menu) and removing the feature entirely will change / break existing sites, so I think it makes sense to ship this and decide if / how we want to retire the feature in a follow up.

[pbking]

I'm going to move this into the "Blockbase 3.0" bucket. We can't remove the feature for reasons noted above and this would fix the issue at hand. There don't seem to be any obvious side-effects.

[pbking]

Closing: Work merged in #6167