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 4 commits into
base: trunk
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8159156
Group: Make nav element selectable and add UI for ariaLabel
t-hamano Jan 11, 2025
fa7b95c
Resolve conflicts
t-hamano Jan 17, 2025
4bad275
Merge branch 'trunk' into group/navigation-ui
t-hamano Jan 18, 2025
79d6d8f
Update navigation label
t-hamano Jan 18, 2025
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
43 changes: 31 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 @@ -23,13 +23,14 @@ import { htmlElementMessages } from '../utils/messages';
/**
* 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;

return (
<InspectorControls group="advanced">
<SelectControl
Expand All @@ -44,11 +45,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

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this expected though? I am torn, it would be good to have some user testing.

If I have a group block and I use the transform, either to another block like the details or to a row or stack, the label is kept.

Copy link
Contributor

@carolinan carolinan Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If the UI was always available, I don't think the removal would be expected.
Without the UI, it is more difficult to discover both that the label is removed, or that it is kept.

} );
} }
help={ htmlElementMessages[ tagName ] }
/>
{ tagName === 'nav' && (
<TextControl
label={ __( 'Navigation label' ) }
value={ ariaLabel || '' }
__next40pxDefaultSize
__nextHasNoMarginBottom
onChange={ ( value ) => {
setAttributes( { ariaLabel: value } );
} }
help={ __(
'If a page includes more than one navigation, each should have a unique label.'
) }
/>
) }
</InspectorControls>
);
}
Expand Down Expand Up @@ -125,10 +146,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