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 15 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
89 changes: 88 additions & 1 deletion packages/block-library/src/navigation/deprecated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,92 @@ const TYPOGRAPHY_PRESET_DEPRECATION_MAP = {
textTransform: 'var:preset|text-transform|',
};

export default [
const deprecated = [
// Remove `isResponsive` attribute.
{
attributes: {
orientation: {
type: 'string',
default: 'horizontal',
},
textColor: {
type: 'string',
},
customTextColor: {
type: 'string',
},
rgbTextColor: {
type: 'string',
},
backgroundColor: {
type: 'string',
},
customBackgroundColor: {
type: 'string',
},
rgbBackgroundColor: {
type: 'string',
},
itemsJustification: {
type: 'string',
},
showSubmenuIcon: {
type: 'boolean',
default: true,
},
openSubmenusOnClick: {
type: 'boolean',
default: false,
},
isResponsive: {
type: 'boolean',
default: 'false',
},
__unstableLocation: {
type: 'string',
},
overlayBackgroundColor: {
type: 'string',
},
customOverlayBackgroundColor: {
type: 'string',
},
overlayTextColor: {
type: 'string',
},
customOverlayTextColor: {
type: 'string',
},
},
supports: {
align: [ 'wide', 'full' ],
anchor: true,
html: false,
inserter: true,
typography: {
fontSize: true,
lineHeight: true,
__experimentalFontStyle: true,
__experimentalFontWeight: true,
__experimentalTextTransform: true,
__experimentalFontFamily: true,
__experimentalTextDecoration: true,
},
},
isEligible( attributes ) {
return attributes.isResponsive;
},
migrate( attributes ) {
delete attributes.isResponsive;
return {
...attributes,
overlayMenu: 'mobile',
};
},
save() {
return <InnerBlocks.Content />;
},
},
{
attributes: {
orientation: {
Expand Down Expand Up @@ -163,3 +248,5 @@ export default [
},
},
];

export default deprecated;
58 changes: 43 additions & 15 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 @@ -271,24 +277,45 @@ function Navigation( {
{ navigatorModal }
<InspectorControls>
{ hasSubmenuIndicatorSetting && (
<PanelBody title={ __( 'Display settings' ) }>
<ToggleControl
checked={ attributes.isResponsive }
onChange={ ( value ) => {
setAttributes( {
isResponsive: value,
} );
} }
label={ __( 'Enable responsive menu' ) }
/>
<PanelBody title={ __( 'Display' ) }>
<h3>{ __( 'Overlay Menu' ) }</h3>
<ToggleGroupControl
label={ __( 'Configure overlay menu' ) }
value={ attributes.overlayMenu }
help={ __(
'Collapses the navigation options in a menu icon opening an overlay.'
) }
onChange={ ( value ) =>
setAttributes( { overlayMenu: value } )
}
isBlock
hideLabelFromVision
>
<ToggleGroupControlOption
value="never"
label={ __( 'Off' ) }
aria-label={ __( 'Off' ) }
/>
<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>
<h3>{ __( 'Submenus' ) }</h3>
<ToggleControl
checked={ attributes.openSubmenusOnClick }
onChange={ ( value ) => {
setAttributes( {
openSubmenusOnClick: value,
} );
} }
label={ __( 'Open submenus on click' ) }
label={ __( 'Open on click' ) }
/>
{ ! attributes.openSubmenusOnClick && (
<ToggleControl
Expand All @@ -298,7 +325,7 @@ function Navigation( {
showSubmenuIcon: value,
} );
} }
label={ __( 'Show submenu indicator icons' ) }
label={ __( 'Show icons' ) }
/>
) }
</PanelBody>
Expand Down Expand Up @@ -352,7 +379,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
30 changes: 24 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,12 @@ 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'] );
/**
* This is for backwards compatibility after `isResponsive` attribute has been removed.
*/
$has_old_responsive_attribute = ! empty( $attributes['isResponsive'] ) && $attributes['isResponsive'];
$is_responsive_menu = isset( $attributes['overlayMenu'] ) && 'never' !== $attributes['overlayMenu'] || $has_old_responsive_attribute;
$should_load_view_script = ! wp_script_is( 'wp-block-navigation-view' ) && ( $is_responsive_menu || $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] );
if ( $should_load_view_script ) {
wp_enqueue_script( 'wp-block-navigation-view' );
}
Expand Down Expand Up @@ -250,7 +255,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()
$is_responsive_menu ? array( 'is-responsive' ) : array()
);

$inner_blocks_html = '';
Expand Down Expand Up @@ -288,17 +293,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 ( ! $is_responsive_menu ) {
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 +327,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
Loading