Global Header menu: Use icons for the mobile menu toggle buttons

[marko-srb]

Can we change the 'Menu' in mobile to the cheveron? As in design...

Screenshot for reference.

It's good to consider the same edit/look for Plugins:
https://github.com/WordPress/wporg-mu-plugins/

[ryelle]

The "Submit a site" link is part of the navigation, so it is also hidden in the mobile dropdown - I don't think we can (or should) separate it out like that.

It's good to consider the same edit/look for Plugins:

Are you saying that this should be done across all redesigned sites?

[marko-srb]

Disregard the link (can/should go under the arrow).

Oh, let's do it for Showcase only. I thought it was only present on the plugins (live sites) at the moment.

—

But generally I think yes, all redesigned sites should have this use-case with cheveron. @jasmussen can help me out, but I believe all redesigned mobile versions should go with cheveron instead of text "Menu".
That could be a separate issue, for sure, with further discussion.

Reference.

When @fcoveram is back from vacation we can discuss more. Although if I recall it was approved design...

[ryelle]

It is present on live (redesigned) sites, see https://wordpress.org/about/requirements/, https://wordpress.org/download/counter/, https://wordpress.org/documentation/ etc.

But generally I think yes, all redesigned sites should have this use-case with cheveron

I don't want us to make this change twice, and I don't think it makes sense for one site (showcase) to diverge without it being very intentional. So I'd recommend either changing everywhere, or holding off entirely if it needs more discussion.

For what it's worth, "Menu" as the label was proposed/decided in this discussion starting here.

[jasmussen]

I missed that initial conversation around replacing the chevron with the word "Menu", and the close with the "X". I'd argue there's nuance there around legibility, especially with long titles. So I agree, we should go back to having the chevron to open, and an X to close. CC: @fcoveram for when he has a chance to catch up.

Apologies for the back and forth there.

[ryelle]

I've moved the issue to wporg-mu-plugins so this can be implemented on all redesigned sites. Unfortunately I think there will still need to be a change in every child theme to update the navigation block's hasIcon, combined with the CSS and workarounds to get the navigation block to show a custom icon (the only core icons are the two hamburger variants).

I'm unclear about the timing here, though. Is this required for the showcase launch? Should it wait for @fcoveram's feedback too?

For now, I've left it in the showcase project, but as blocked.

[marko-srb]

Ideally it is done for Showcase before the launch, and discussed with @fcoveram before we do it on all other places. If that's not the option, then the whole thing can wait.

Thanks.

[ryelle]

Okay, I'm going to take it out of the Showcase project then.

[ndiego]

For reference, there is a discussion in Core about allowing custom icons: WordPress/gutenberg#37930.

That said, in the meantime, we can use the WP_HTML_Tag_Processor. The following code should work but probably could be cleaned up a bit, and the icons might need some design modifications.

You will need to set the Navigation block to show the three-bar hamburger icon on mobile. The two-bar icon does not use path and, therefore won't work. Then, this code will convert the icon to chevrons.

function customize_navigation_block_icon( $block_content, $block ) {

    if ( $block['blockName'] === 'core/navigation' ) {

        $tag_processor = new WP_HTML_Tag_Processor( $block_content );

        if ( 
            $tag_processor->next_tag( array( 
                'tag_name' => 'div', 
                'class_name' => 'wp-block-wporg-local-navigation-bar' 
            ) )
        ) {
            if ( 
                $tag_processor->next_tag( array( 
                    'tag_name' => 'button', 
                    'class_name' => 'wp-block-navigation__responsive-container-open' 
                ) ) &&
                $tag_processor->next_tag( 'path' )
            ) {
                $tag_processor->set_attribute( 'd', 'M17.5 11.6L12 16l-5.5-4.4.9-1.2L12 14l4.5-3.6 1 1.2z' );
            }
    
            if ( 
                $tag_processor->next_tag( array( 
                    'tag_name' => 'button', 
                    'class_name' => 'wp-block-navigation__responsive-container-close' 
                ) ) &&
                $tag_processor->next_tag( 'path' )
            ) {
                $tag_processor->set_attribute( 'd', 'M6.5 12.4L12 8l5.5 4.4-.9 1.2L12 10l-4.5 3.6-1-1.2z' );
            }
    
            return $tag_processor->get_updated_html();
        }
    }

    return $block_content;
}
add_filter('render_block', 'customize_navigation_block_icon', 10, 2);

[jasmussen]

Just wanted to clarify that from design, let's definitely go with the icons, not the labels here. I understand if this can be blocked or challenged technically, in which case it isn't the most urgent in the world, but just noting for the record so I can unblock it from design.

[ryelle]

It's not technically difficult (@ndiego's solution looks good, or CSS like I said), I was just saying that it's not a "flip a switch" change. It was blocked on the design confirmation that this should happen for all sites, so I've moved it to "to do" now.

[fcoveram]

This is definitely my mistake. Sorry for missing the "Menu" label decision and not updating the component. I second @jasmussen's point on going with the icon.

I will update the component to reflect the change in all mockups. Thanks and sorry for the back and forth.

[ndiego]

@renintw and @adamwoodnz, this is not a requirement for the Showcase v2 launch, but this should now be unblocked. We will move forward with the icon instead of the "Menu". Let us know if you need anything else. Thanks!

[adamwoodnz]

Developer is updated, I'll roll out the rest today.

[adamwoodnz]

✅ Rolled out to Showcase, Documentation and Main too.