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

Group: Make nav element selectable and add UI for ariaLabel #68611

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
+33 −12
Open

Group: Make nav element selectable and add UI for ariaLabel #68611

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 33 additions & 12 deletions packages/block-library/src/group/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
useInnerBlocksProps,
store as blockEditorStore,
} from '@wordpress/block-editor';
import { SelectControl } from '@wordpress/components';
import { SelectControl, TextControl } from '@wordpress/components';
import { useRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { View } from '@wordpress/primitives';
Expand All @@ -22,13 +22,13 @@ import GroupPlaceHolder, { useShouldShowPlaceHolder } from './placeholder';
/**
* Render inspector controls for the Group block.
*
* @param {Object} props Component props.
* @param {string} props.tagName The HTML tag name.
* @param {Function} props.onSelectTagName onChange function for the SelectControl.
*
* @return {JSX.Element} The control group.
* @param {Object} props Component props.
* @param {Object} props.attributes Block attributes.
* @param {(attributes: Object) => void} props.setAttributes Callback for updating block attributes.
* @return {JSX.Element} The control group.
*/
function GroupEditControls( { tagName, onSelectTagName } ) {
function GroupEditControls( { attributes, setAttributes } ) {
const { tagName, ariaLabel } = attributes;
const htmlElementMessages = {
header: __(
'The <header> element should represent introductory content, typically a group of introductory or navigational aids.'
Expand All @@ -48,6 +48,9 @@ function GroupEditControls( { tagName, onSelectTagName } ) {
footer: __(
'The <footer> element should represent a footer for its nearest sectioning element (e.g.: <section>, <article>, <main> etc.).'
),
nav: __(
'The <nav> element should represent a section of a page that links to other pages or to parts within the page.'
),
};
return (
<InspectorControls group="advanced">
Expand All @@ -63,11 +66,31 @@ function GroupEditControls( { tagName, onSelectTagName } ) {
{ label: '<article>', value: 'article' },
{ label: '<aside>', value: 'aside' },
{ label: '<footer>', value: 'footer' },
{ label: '<nav>', value: 'nav' },
] }
value={ tagName }
onChange={ onSelectTagName }
onChange={ ( value ) => {
setAttributes( {
tagName: value,
ariaLabel: value === 'nav' ? ariaLabel : undefined,
Copy link
Member

Choose a reason for hiding this comment

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

This is going to override already existing aria labels when the tag name is changed to and from nav

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should work like this:

  • When changed TO a nav element: existing aria-Label is maintained
  • When changed FROM a nav element: aria-Label is removed

} );
} }
help={ htmlElementMessages[ tagName ] }
/>
{ tagName === 'nav' && (
<TextControl
label={ __( 'Navigation label' ) }
value={ ariaLabel || '' }
__next40pxDefaultSize
__nextHasNoMarginBottom
onChange={ ( value ) => {
setAttributes( { ariaLabel: value } );
} }
help={ __(
'Add a label to describe the purpose of this navigation element.'
) }
/>
) }
Comment on lines +80 to +93
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's just time to add a full standardized UI for the aria label block support 🤔

In reality users can already set this attribute in code today. It just doesn't have a UI... I'm not sure only showing it for the nav tag name makes it more clear 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if it's just time to add a full standardized UI for the aria label block support

That's certainly true, and maybe we should do so in the future.

Personally, I'm cautious about changing the block support spec right now. For now, I prefer to expose the UI only to the Group block. I'd like to hear other people's opinions on this.

</InspectorControls>
);
}
Expand Down Expand Up @@ -144,10 +167,8 @@ function GroupEdit( { attributes, name, setAttributes, clientId } ) {
return (
<>
<GroupEditControls
tagName={ TagName }
onSelectTagName={ ( value ) =>
setAttributes( { tagName: value } )
}
attributes={ attributes }
setAttributes={ setAttributes }
/>
{ showPlaceholder && (
<View>
Expand Down
Loading