Skip to content

Responsive Navigation #30047

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

Merged
vcanales merged 35 commits into from
May 12, 2021
Merged

Responsive Navigation #30047

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
6d9f03b
Implement responsive navigation menu
Apr 9, 2021
885fcb5
only hide closing button if menu is closed
Apr 9, 2021
fa50591
make sure frontend.js is only being loaded once
Apr 9, 2021
f433a9a
Fix issues with open modal.
jasmussen Apr 9, 2021
7729c3f
make sure scripts are only loaded once
vcanales Apr 15, 2021
ab29f51
Add a focus z index so focus isn't cropped.
jasmussen Apr 16, 2021
6ea6ed1
Fix ariaHidden
jasmussen Apr 16, 2021
f40546d
Fix color issue.
jasmussen Apr 16, 2021
0c2a4dc
Fix safari issue.
jasmussen Apr 16, 2021
3df9544
only enqueue frontend script if needed
vcanales Apr 16, 2021
a7dac30
use isResponsive instead of responsiveNavigation
vcanales Apr 16, 2021
2eb3b68
Fix modal id
vcanales Apr 29, 2021
82f85e1
Update package-lock, move deps to correct place
vcanales Apr 29, 2021
f80f54a
Fix aria-hidden label in the editor
vcanales Apr 30, 2021
d22e43d
Set responsiveness off by default
vcanales May 3, 2021
3808578
Add missing aria attributes
vcanales May 3, 2021
637bb09
update e2e fixture
vcanales May 4, 2021
2cd59a7
remove unnecessary context declaration
vcanales May 4, 2021
ccd361f
remove file existence check
vcanales May 4, 2021
dadce1c
Tests responsiveness on preview page
vcanales May 7, 2021
a59dff6
Refactor server side render of nav menu
vcanales May 7, 2021
986c192
Make sure toggle button labels are translatable
vcanales May 7, 2021
bc9bb9c
Only render open button if menu is closed
vcanales May 7, 2021
0d12f7c
Remove duplicate CSS from rebase.
jasmussen May 5, 2021
a12aeea
Fix focus cropping issue.
jasmussen May 5, 2021
7065648
Simplify overlay style.
jasmussen May 5, 2021
5da3bfd
Fix for page list.
jasmussen May 5, 2021
1b9885f
Remove a few todos.
jasmussen May 5, 2021
dd97947
Fix overlay on desktop breakpoints style, and focus styles.
jasmussen May 6, 2021
dc9d315
Small transparency fix.
jasmussen May 10, 2021
516e7a6
keep parameter order consistency
vcanales May 11, 2021
10adcaa
Address feedback
vcanales May 11, 2021
4b0ecda
Update packages/block-library/src/navigation/frontend.js
vcanales May 11, 2021
b2990cd
remove no-longer needed css rules
vcanales May 11, 2021
2769bd2
edit redundant callback definition
vcanales May 11, 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
Prev Previous commit
Next Next commit
use isResponsive instead of responsiveNavigation
  • Loading branch information
@vcanales
vcanales committed May 3, 2021
commit a7dac30d0299219bdbc5f539020200cbab3b847a
9 changes: 6 additions & 3 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 @@ -35,7 +35,7 @@
"type": "boolean",
"default": true
},
"responsiveNavigation": {
"isResponsive": {
"type": "boolean",
"default": true
}
Expand All @@ -50,10 +50,13 @@
"showSubmenuIcon": "showSubmenuIcon",
"style": "style",
"orientation": "orientation",
"responsiveNavigation": "responsiveNavigation"
"isResponsive": "isResponsive"
},
"supports": {
"align": [ "wide", "full" ],
"align": [
"wide",
"full"
],
"anchor": true,
"html": false,
"inserter": true,
Expand Down
8 changes: 4 additions & 4 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 @@ -69,7 +69,7 @@ function Navigation( {
className: classnames( className, {
[ `items-justified-${ attributes.itemsJustification }` ]: attributes.itemsJustification,
'is-vertical': attributes.orientation === 'vertical',
'is-responsive': attributes.responsiveNavigation,
'is-responsive': attributes.isResponsive,
} ),
} );

Expand Down Expand Up @@ -154,10 +154,10 @@ function Navigation( {
label={ __( 'Show submenu indicator icons' ) }
/>
<ToggleControl
checked={ attributes.responsiveNavigation }
checked={ attributes.isResponsive }
onChange={ ( value ) => {
setAttributes( {
responsiveNavigation: value,
isResponsive: value,
} );
} }
label={ __( 'Enable responsive menu' ) }
Expand All @@ -172,7 +172,7 @@ function Navigation( {
setResponsiveMenuVisibility( value )
}
isOpen={ isResponsiveMenuOpen }
isResponsive={ attributes.responsiveNavigation }
isResponsive={ attributes.isResponsive }
>
<ul { ...innerBlocksProps }></ul>
</ResponsiveWrapper>
Expand Down
6 changes: 3 additions & 3 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 @@ -122,7 +122,7 @@ function render_block_core_navigation( $attributes, $content, $block ) {
unset( $attributes['rgbTextColor'], $attributes['rgbBackgroundColor'] );

$script_path = __DIR__ . '/navigation/frontend.js';
$should_load_frontend_script = $attributes['responsiveNavigation'] && ! wp_script_is( 'core_block_navigation_load_frontend_scripts' ) && file_exists( $script_path );
$should_load_frontend_script = $attributes['isResponsive'] && ! wp_script_is( 'core_block_navigation_load_frontend_scripts' ) && file_exists( $script_path );

if ( $should_load_frontend_script ) {
wp_enqueue_script(
Expand All @@ -145,7 +145,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['responsiveNavigation'] ) && true === $attributes['responsiveNavigation'] ? array( 'is-responsive' ) : array()
isset( $attributes['isResponsive'] ) && true === $attributes['isResponsive'] ? array( 'is-responsive' ) : array()
);

$inner_blocks_html = '';
Expand All @@ -165,7 +165,7 @@ function render_block_core_navigation( $attributes, $content, $block ) {
$modal_unique_id = uniqid();

// Determine whether or not navigation elements should be wrapped in the markup required to make it responsive.
$responsive_container_markup = isset( $attributes['responsiveNavigation'] ) && true === $attributes['responsiveNavigation'] ? sprintf(
$responsive_container_markup = isset( $attributes['isResponsive'] ) && true === $attributes['isResponsive'] ? sprintf(
'<button class="wp-block-navigation__responsive-container-open" data-micromodal-trigger="modal-%2$s" aria-label="Open menu"><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>
Copy link
Member

@gziolo gziolo Apr 16, 2021

Choose a reason for hiding this comment

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

The button misses some a11y properties, see Reakit:

Screen Shot 2021-04-16 at 10 31 46

It looks like we should add aria-haspopup and aria-expanded. @diegohaz can you confirm?

Copy link
Member

@diegohaz diegohaz Apr 16, 2021

Choose a reason for hiding this comment

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

Those attributes are not required for modal triggers. But they help a lot as screen readers will announce the button as a popup button and whether it's expanded or not.

<div class="wp-block-navigation__responsive-container" id="modal-%2$s" aria-hidden="true">
<div class="wp-block-navigation__responsive-close" tabindex="-1" data-micromodal-close>
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/fixtures/blocks/core__navigation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"isValid": true,
"attributes": {
"showSubmenuIcon": true,
"responsiveNavigation": true
"isResponsive": true
},
"innerBlocks": [],
"originalContent": ""
Expand Down