Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable always on burger menu for responsive navigation menus. #35568

Merged
merged 16 commits into from
Oct 13, 2021
Merged

Enable always on burger menu for responsive navigation menus. #35568

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7f8c105
Enable always-on burger menu in the editor
Oct 12, 2021
2151417
enable always hidden responsive menu
Oct 12, 2021
937b224
only show toggle if menu is responsive
Oct 12, 2021
7f7fa4f
change open button modifier classname
Oct 12, 2021
3f1580a
regenerate fixtures to include new attribute
Oct 12, 2021
b758795
replace isResponsive with overlayMenu attribute
vcanales Oct 13, 2021
5fbdd0b
add submenus option header
vcanales Oct 13, 2021
3b6cf52
update fixtures for new overlayMenu attribute
vcanales Oct 13, 2021
5d3e03b
make texts translatable
vcanales Oct 13, 2021
96f3dff
Tweak inspector
jasmussen Oct 13, 2021
6524981
Add separate heading.
jasmussen Oct 13, 2021
9e6693a
Tweak wording.
jasmussen Oct 13, 2021
facfdf7
Simplify toggles.
jasmussen Oct 13, 2021
c9464cd
add deprecation
ntsekouras Oct 13, 2021
b832b86
fix e2e test
ntsekouras Oct 13, 2021
78f96af
remove redundant aria-label attributes
vcanales Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions packages/block-library/src/navigation/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
"title": "Navigation",
"category": "theme",
"description": "A collection of blocks that allow visitors to get around your site.",
"keywords": [ "menu", "navigation", "links" ],
"keywords": [
"menu",
"navigation",
"links"
],
"textdomain": "default",
"attributes": {
"orientation": {
Expand Down Expand Up @@ -36,13 +40,13 @@
"type": "boolean",
"default": true
},
"openSubmenusOnClick":{
"openSubmenusOnClick": {
"type": "boolean",
"default": false
},
"isResponsive": {
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
"type": "boolean",
"default": false
"overlayMenu": {
"type": "string",
"default": "never"
},
"__unstableLocation": {
"type": "string"
Expand Down Expand Up @@ -77,7 +81,10 @@
"orientation": "orientation"
},
"supports": {
"align": [ "wide", "full" ],
"align": [
"wide",
"full"
],
"anchor": true,
"html": false,
"inserter": true,
Expand Down
55 changes: 43 additions & 12 deletions packages/block-library/src/navigation/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ import {
getColorClassName,
} from '@wordpress/block-editor';
import { useDispatch, withSelect, withDispatch } from '@wordpress/data';
import { PanelBody, ToggleControl, ToolbarGroup } from '@wordpress/components';
import {
PanelBody,
ToggleControl,
__experimentalToggleGroupControl as ToggleGroupControl,
__experimentalToggleGroupControlOption as ToggleGroupControlOption,
ToolbarGroup,
} from '@wordpress/components';
import { compose } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';

Expand Down Expand Up @@ -135,7 +141,7 @@ function Navigation( {
className: classnames( className, {
[ `items-justified-${ attributes.itemsJustification }` ]: attributes.itemsJustification,
'is-vertical': attributes.orientation === 'vertical',
'is-responsive': attributes.isResponsive,
'is-responsive': 'never' !== attributes.overlayMenu,
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
'has-text-color': !! textColor.color || !! textColor?.class,
[ getColorClassName(
'color',
Expand Down Expand Up @@ -272,15 +278,39 @@ function Navigation( {
<InspectorControls>
{ hasSubmenuIndicatorSetting && (
<PanelBody title={ __( 'Display settings' ) }>
<ToggleControl
checked={ attributes.isResponsive }
onChange={ ( value ) => {
setAttributes( {
isResponsive: value,
} );
} }
label={ __( 'Enable responsive menu' ) }
/>
<ToggleGroupControl
label={
<div>
<h4>{ __( 'Overlay Menu' ) }</h4>
<p>
{ __(
'Controls whether the menu collapses into a toggle button opening an overlay for navigation.'
) }
</p>
</div>
}
value={ attributes.overlayMenu }
onChange={ ( value ) =>
setAttributes( { overlayMenu: value } )
}
>
<ToggleGroupControlOption
value="never"
label={ __( 'Never' ) }
aria-label={ __( 'Never' ) }
/>
<ToggleGroupControlOption
value="mobile"
label={ __( 'Mobile' ) }
aria-label={ __( 'Mobile' ) }
/>
<ToggleGroupControlOption
value="always"
label={ __( 'Always' ) }
aria-label={ __( 'Always' ) }
vcanales marked this conversation as resolved.
Show resolved Hide resolved
/>
</ToggleGroupControl>
<h4>{ __( 'Submenus' ) }</h4>
<ToggleControl
checked={ attributes.openSubmenusOnClick }
onChange={ ( value ) => {
Expand Down Expand Up @@ -352,7 +382,8 @@ function Navigation( {
id={ clientId }
onToggle={ setResponsiveMenuVisibility }
isOpen={ isResponsiveMenuOpen }
isResponsive={ attributes.isResponsive }
isResponsive={ 'never' !== attributes.overlayMenu }
isHiddenByDefault={ 'always' === attributes.overlayMenu }
>
<div { ...innerBlocksProps }></div>
</ResponsiveWrapper>
Expand Down
6 changes: 0 additions & 6 deletions packages/block-library/src/navigation/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,12 +397,6 @@ $color-control-label-height: 20px;
}
}
}
.components-button.wp-block-navigation__responsive-container-open {
@include break-small {
display: none;
}
}

// Emulate the fullscreen editing inside the editor.
// Most of this can be removed when the iframe lands.

Expand Down
25 changes: 19 additions & 6 deletions packages/block-library/src/navigation/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ function render_block_core_navigation( $attributes, $content, $block ) {

unset( $attributes['rgbTextColor'], $attributes['rgbBackgroundColor'] );

$should_load_view_script = ! wp_script_is( 'wp-block-navigation-view' ) && ( ! empty( $attributes['isResponsive'] ) || $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] );
$should_load_view_script = ! wp_script_is( 'wp-block-navigation-view' ) && ( ( ! empty( $attributes['overlayMenu'] ) && 'never' !== $attributes['overlayMenu'] ) || $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] );
if ( $should_load_view_script ) {
wp_enqueue_script( 'wp-block-navigation-view' );
}
Expand Down Expand Up @@ -250,7 +250,7 @@ function render_block_core_navigation( $attributes, $content, $block ) {
$font_sizes['css_classes'],
( isset( $attributes['orientation'] ) && 'vertical' === $attributes['orientation'] ) ? array( 'is-vertical' ) : array(),
isset( $attributes['itemsJustification'] ) ? array( 'items-justified-' . $attributes['itemsJustification'] ) : array(),
isset( $attributes['isResponsive'] ) && true === $attributes['isResponsive'] ? array( 'is-responsive' ) : array()
isset( $attributes['overlayMenu'] ) && 'never' !== $attributes['overlayMenu'] ? array( 'is-responsive' ) : array()
);

$inner_blocks_html = '';
Expand Down Expand Up @@ -288,17 +288,28 @@ function render_block_core_navigation( $attributes, $content, $block ) {

// Determine whether or not navigation elements should be wrapped in the markup required to make it responsive,
// return early if they don't.
if ( ! isset( $attributes['isResponsive'] ) || false === $attributes['isResponsive'] ) {
if ( ! isset( $attributes['overlayMenu'] ) || 'never' === $attributes['overlayMenu'] ) {
return sprintf(
'<nav %1$s>%2$s</nav>',
$wrapper_attributes,
$inner_blocks_html
);
}

$is_hidden_by_default = isset( $attributes['overlayMenu'] ) && 'always' === $attributes['overlayMenu'];

$responsive_container_classes = array(
'wp-block-navigation__responsive-container',
$is_hidden_by_default ? 'hidden-by-default' : '',
);
$open_button_classes = array(
'wp-block-navigation__responsive-container-open',
$is_hidden_by_default ? 'always-shown' : '',
);

$responsive_container_markup = sprintf(
'<button aria-expanded="false" aria-haspopup="true" aria-label="%3$s" class="wp-block-navigation__responsive-container-open" data-micromodal-trigger="modal-%1$s"><svg width="24" height="24" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" role="img" aria-hidden="true" focusable="false"><rect x="4" y="7.5" width="16" height="1.5" /><rect x="4" y="15" width="16" height="1.5" /></svg></button>
<div class="wp-block-navigation__responsive-container" id="modal-%1$s" aria-hidden="true">
'<button aria-expanded="false" aria-haspopup="true" aria-label="%3$s" class="%6$s" data-micromodal-trigger="modal-%1$s"><svg width="24" height="24" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" role="img" aria-hidden="true" focusable="false"><rect x="4" y="7.5" width="16" height="1.5" /><rect x="4" y="15" width="16" height="1.5" /></svg></button>
<div class="%5$s" id="modal-%1$s" aria-hidden="true">
<div class="wp-block-navigation__responsive-close" tabindex="-1" data-micromodal-close>
<div class="wp-block-navigation__responsive-dialog" role="dialog" aria-modal="true" aria-labelledby="modal-%1$s-title" >
<button aria-label="%4$s" data-micromodal-close class="wp-block-navigation__responsive-container-close"><svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24" role="img" aria-hidden="true" focusable="false"><path d="M13 11.8l6.1-6.3-1-1-6.1 6.2-6.1-6.2-1 1 6.1 6.3-6.5 6.7 1 1 6.5-6.6 6.5 6.6 1-1z"></path></svg></button>
Expand All @@ -311,7 +322,9 @@ function render_block_core_navigation( $attributes, $content, $block ) {
$modal_unique_id,
$inner_blocks_html,
__( 'Open menu' ), // Open button label.
__( 'Close menu' ) // Close button label.
__( 'Close menu' ), // Close button label.
implode( ' ', $responsive_container_classes ),
implode( ' ', $open_button_classes )
);

return sprintf(
Expand Down
8 changes: 7 additions & 1 deletion packages/block-library/src/navigation/responsive-wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default function ResponsiveWrapper( {
isOpen,
isResponsive,
onToggle,
isHiddenByDefault,
} ) {
if ( ! isResponsive ) {
return children;
Expand All @@ -25,8 +26,13 @@ export default function ResponsiveWrapper( {
'wp-block-navigation__responsive-container',
{
'is-menu-open': isOpen,
'hidden-by-default': isHiddenByDefault,
}
);
const openButtonClasses = classnames(
'wp-block-navigation__responsive-container-open',
{ 'always-shown': isHiddenByDefault }
);

const modalId = `${ id }-modal`;

Expand All @@ -37,7 +43,7 @@ export default function ResponsiveWrapper( {
aria-haspopup="true"
aria-expanded={ isOpen }
aria-label={ __( 'Open menu' ) }
className="wp-block-navigation__responsive-container-open"
className={ openButtonClasses }
onClick={ () => onToggle( true ) }
>
<SVG
Expand Down
28 changes: 16 additions & 12 deletions packages/block-library/src/navigation/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,16 +505,18 @@
}

@include break-small() {
&:not(.is-menu-open) {
display: flex;
flex-direction: row;
width: 100%;
position: relative;
z-index: 2;
background-color: inherit;

.wp-block-navigation__responsive-container-close {
display: none;
&:not(.hidden-by-default) {
&:not(.is-menu-open) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rules here are a bit hard to parse, but I think they make sense. But it would be good if a themer, perhaps @kjellr could take this PR for a spin with a theme that uses the existing responsive feature, to see that it works as it should?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this with a few existing block themes that had the responsive menu set to (what is now) "Mobile" and found no issues. @jeffikus and @scruffian are also giving it a quick spin too just to be sure but I feel pretty solid on this so far.

display: flex;
flex-direction: row;
width: 100%;
position: relative;
z-index: 2;
background-color: inherit;

.wp-block-navigation__responsive-container-close {
display: none;
}
}
}

Expand Down Expand Up @@ -558,8 +560,10 @@
.wp-block-navigation__responsive-container-open {
display: flex;

@include break-small {
display: none;
&:not(.always-shown) {
@include break-small {
display: none;
}
}

// Justify the button.
Expand Down
2 changes: 1 addition & 1 deletion test/integration/fixtures/blocks/core__navigation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"orientation": "horizontal",
"showSubmenuIcon": true,
"openSubmenusOnClick": false,
"isResponsive": false
"overlayMenu": "never"
},
"innerBlocks": [],
"originalContent": ""
Expand Down