Skip to content

Commit

Permalink
Fix Navigation accessibility issues (#36292)
Browse files Browse the repository at this point in the history 
* Fix display contents accessibility issue in Navigation

* Remove aria-hidden from navigation wrapper.

* Fix content justification in overlay nav.

* Fix for alignment inside overlay menu.

* Move text align rule.

* Revert "Fix content justification in overlay nav."

This reverts commit 5170bb5bb3b9ead314a1ecec68768f2fe2511804.

* Fix php lint errors.

Co-authored-by: jasmussen <joen@automattic.com>
  • Loading branch information
@tellthemachines @jasmussen @andrewserong
2 people authored and andrewserong committed Nov 10, 2021
1 parent 369a056 commit 0ee7f3b
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 35 deletions.
18 changes: 15 additions & 3 deletions lib/block-supports/layout.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,26 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
*/
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "justify-content: {$justify_content_options[ $layout['justifyContent'] ]};";
// --justification-setting allows children to inherit the value regardless or row or column direction.
$style .= "--justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
$style .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= '--layout-direction: row;';
$style .= "--layout-wrap: $flex_wrap;";
$style .= "--layout-justify: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= '--layout-align: center;';
}
}
} else {
$style .= 'flex-direction: column;';
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "align-items: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= "--justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
$style .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= '--layout-direction: column;';
$style .= '--layout-justify: initial;';
$style .= "--layout-align: {$justify_content_options[ $layout['justifyContent'] ]};";
}
}
}
$style .= '}';
Expand Down
32 changes: 25 additions & 7 deletions packages/block-editor/src/layouts/flex.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ export default {
);
},
save: function FlexLayoutStyle( { selector, layout } ) {
const { orientation = 'horizontal' } = layout;
const {
orientation = 'horizontal',
setCascadingProperties = false,
} = layout;
const blockGapSupport = useSetting( 'spacing.blockGap' );
const hasBlockGapStylesSupport = blockGapSupport !== null;
const justifyContent =
Expand All @@ -94,21 +97,36 @@ export default {
const flexWrap = flexWrapOptions.includes( layout.flexWrap )
? layout.flexWrap
: 'wrap';
// --justification-setting allows children to inherit the value
// regardless or row or column direction.
const rowOrientation = `
let rowOrientation = `
flex-direction: row;
align-items: center;
justify-content: ${ justifyContent };
--justification-setting: ${ justifyContent };
`;
if ( setCascadingProperties ) {
// --layout-justification-setting allows children to inherit the value
// regardless or row or column direction.
rowOrientation += `
--layout-justification-setting: ${ justifyContent };
--layout-direction: row;
--layout-wrap: ${ flexWrap };
--layout-justify: ${ justifyContent };
--layout-align: center;
`;
}
const alignItems =
alignItemsMap[ layout.justifyContent ] || alignItemsMap.left;
const columnOrientation = `
let columnOrientation = `
flex-direction: column;
align-items: ${ alignItems };
--justification-setting: ${ alignItems };
`;
if ( setCascadingProperties ) {
columnOrientation += `
--layout-justification-setting: ${ alignItems };
--layout-direction: column;
--layout-justify: initial;
--layout-align: ${ alignItems };
`;
}
return (
<style>{ `
${ appendSelectors( selector ) } {
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/navigation-submenu/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ button.wp-block-navigation-item__content {
color: currentColor;
font-size: inherit;
font-family: inherit;

// Buttons default to center alignment. This becomes visible
// when a menu item label is long enough to wrap.
text-align: left;
}

.wp-block-navigation-submenu__toggle {
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/navigation/block.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@
"allowSwitching": false,
"allowInheriting": false,
"default": {
"type": "flex"
"type": "flex",
"setCascadingProperties": true
}
}
},
Expand Down
6 changes: 1 addition & 5 deletions packages/block-library/src/navigation/edit/responsive-wrapper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,7 @@ export default function ResponsiveWrapper( {
</Button>
) }

<div
className={ responsiveContainerClasses }
id={ modalId }
aria-hidden={ ! isOpen }
>
<div className={ responsiveContainerClasses } id={ modalId }>
<div
className="wp-block-navigation__responsive-close"
tabIndex="-1"
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/navigation/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ function( $block ) {

$responsive_container_markup = sprintf(
'<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="%5$s" id="modal-%1$s">
<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 Down
33 changes: 17 additions & 16 deletions packages/block-library/src/navigation/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@

// Navigation block inner container.
.wp-block-navigation__container {
display: flex;
flex-wrap: var(--layout-wrap, wrap);
flex-direction: var(--layout-direction, initial);
justify-content: var(--layout-justify, initial);
align-items: var(--layout-align, initial);

// Reset the default list styles
list-style: none;
Expand All @@ -329,7 +334,11 @@
bottom: 0;

.wp-block-navigation__responsive-container-content {
display: contents;
display: flex;
flex-wrap: var(--layout-wrap, wrap);
flex-direction: var(--layout-direction, initial);
justify-content: var(--layout-justify, initial);
align-items: var(--layout-align, initial);
}

// Overlay menu.
Expand All @@ -350,8 +359,6 @@
// Add extra top padding so items don't conflict with close button.
padding: 72px 24px 24px 24px;
background-color: inherit;
// Fallback to inheritance in case the --justification-setting variable fails.
align-items: inherit;

.wp-block-navigation__responsive-container-content {
// Override the container flex layout settings
Expand All @@ -360,7 +367,11 @@
display: flex;
flex-direction: column;
// Inherit alignment settings from container.
align-items: var(--justification-setting, inherit);
align-items: var(--layout-justification-setting, inherit);

// Always align the contents of the menu to the top.
justify-content: flex-start;

// Allow menu to scroll.
overflow: auto;
padding: 0;
Expand Down Expand Up @@ -412,7 +423,7 @@
display: flex;
flex-direction: column;
// Inherit alignment settings from container.
align-items: var(--justification-setting, inherit);
align-items: var(--layout-justification-setting, initial);
}
}

Expand All @@ -435,7 +446,7 @@
@include break-small() {
&:not(.hidden-by-default) {
&:not(.is-menu-open) {
display: contents;
display: block;
width: 100%;
position: relative;
z-index: 2;
Expand Down Expand Up @@ -518,13 +529,3 @@ html.has-modal-open {
overflow: hidden;
}

.wp-block-navigation__responsive-close,
.wp-block-navigation__responsive-dialog,
.wp-block-navigation__container {
display: contents;

.is-menu-open & {
// Fallback to inheritance in case the --justification-setting variable fails.
align-items: inherit;
}
}
7 changes: 5 additions & 2 deletions packages/block-library/src/page-list/style.scss
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// The Page List block should inherit navigation styles when nested within it
.wp-block-navigation {
.wp-block-page-list {
display: contents;
flex-wrap: wrap;
display: flex;
flex-direction: var(--layout-direction, initial);
justify-content: var(--layout-justify, initial);
align-items: var(--layout-align, initial);
flex-wrap: var(--layout-wrap, wrap);
background-color: inherit;
}

Expand Down

0 comments on commit 0ee7f3b

Please sign in to comment.