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.

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

Improve the various Alignment controls props handling #63696

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ function AlignmentUI( {
value,
onChange,
alignmentControls = DEFAULT_ALIGNMENT_CONTROLS,
label = __( 'Align text' ),
description = __( 'Change text alignment' ),
label = __( 'Align block' ),
description,
isCollapsed = true,
isToolbar,
} ) {
Expand All @@ -55,9 +55,7 @@ function AlignmentUI( {
const extraProps = isToolbar
? { isCollapsed }
: {
toggleProps: {
description,
},
toggleProps: description ? { description } : {},
popoverProps: POPOVER_PROPS,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ exports[`BlockAlignmentUI should match snapshot when controls are hidden 1`] = `
<button
aria-expanded="false"
aria-haspopup="true"
aria-label="Align"
aria-label="Align block"
class="components-button components-dropdown-menu__toggle has-icon"
data-toolbar-item="true"
type="button"
Expand All @@ -36,7 +36,7 @@ exports[`BlockAlignmentUI should match snapshot when controls are visible 1`] =
<div
class="components-toolbar"
icon="[object Object]"
label="Align"
label="Align block"
>
<div>
<button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe( 'BlockAlignmentUI', () => {

await user.click(
screen.getByRole( 'button', {
name: 'Align',
name: 'Align block',
} )
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ function BlockAlignmentUI( {
controls,
isToolbar,
isCollapsed = true,
label = __( 'Align block' ),
description,
} ) {
const enabledControls = useAvailableAlignments( controls );
const hasEnabledControls = !! enabledControls.length;
Expand All @@ -47,7 +49,7 @@ function BlockAlignmentUI( {
icon: activeAlignmentControl
? activeAlignmentControl.icon
: defaultAlignmentControl.icon,
label: __( 'Align' ),
label,
};
const extraProps = isToolbar
? {
Expand All @@ -64,7 +66,9 @@ function BlockAlignmentUI( {
} ),
}
: {
toggleProps: { description: __( 'Change alignment' ) },
toggleProps: description
? { 'aria-description': description }
: {},
children: ( { onClose } ) => {
return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ exports[`BlockVerticalAlignmentUI should match snapshot when controls are hidden
<button
aria-expanded="false"
aria-haspopup="true"
aria-label="Change vertical alignment"
aria-label="Align content vertically"
class="components-button components-dropdown-menu__toggle has-icon"
data-toolbar-item="true"
type="button"
Expand All @@ -36,7 +36,7 @@ exports[`BlockVerticalAlignmentUI should match snapshot when controls are visibl
<div
class="components-toolbar"
icon="[object Object]"
label="Change vertical alignment"
label="Align content vertically"
>
<div>
<button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,22 @@ describe( 'BlockVerticalAlignmentUI', () => {
expect( container ).toMatchSnapshot();
} );

test( 'should use custom label and description', () => {
render(
<BlockVerticalAlignmentUI
value={ alignment }
onChange={ onChange }
label="Custom Label"
description="Custom Description"
/>
);

const button = screen.getByRole( 'button', { name: 'Custom Label' } );
expect( button ).toHaveAttribute( 'aria-label', 'Custom Label' );
// The description is now part of the toggleProps and not directly accessible
// We can't easily test for the describedBy attribute here
} );

test( 'should expand controls when toggled', async () => {
const user = userEvent.setup();

Expand All @@ -59,7 +75,7 @@ describe( 'BlockVerticalAlignmentUI', () => {

await user.click(
screen.getByRole( 'button', {
name: 'Change vertical alignment',
name: 'Align content vertically',
} )
);

Expand All @@ -86,7 +102,7 @@ describe( 'BlockVerticalAlignmentUI', () => {
);

const activeControl = screen.getByRole( 'button', {
name: `Align ${ alignment }`,
name: `Align top`,
pressed: true,
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ function BlockVerticalAlignmentUI( {
controls = DEFAULT_CONTROLS,
isCollapsed = true,
isToolbar,
label = _x(
'Align content vertically',
'Block vertical alignment setting label'
),
description,
} ) {
function applyOrUnset( align ) {
return () => onChange( value === align ? undefined : align );
Expand All @@ -53,30 +58,33 @@ function BlockVerticalAlignmentUI( {
BLOCK_ALIGNMENTS_CONTROLS[ DEFAULT_CONTROL ];

const UIComponent = isToolbar ? ToolbarGroup : ToolbarDropdownMenu;
const extraProps = isToolbar ? { isCollapsed } : {};
const extraProps = isToolbar
? { isCollapsed }
: {
popoverProps: { focusOnMount: 'firstElement' },
toggleProps: description
? {
describedBy: description,
}
: {},
};

return (
<UIComponent
icon={
activeAlignment
? activeAlignment.icon
: defaultAlignmentControl.icon
}
label={ _x(
'Change vertical alignment',
'Block vertical alignment setting label'
) }
controls={ controls.map( ( control ) => {
return {
...BLOCK_ALIGNMENTS_CONTROLS[ control ],
isActive: value === control,
role: isCollapsed ? 'menuitemradio' : undefined,
onClick: applyOrUnset( control ),
};
} ) }
{ ...extraProps }
/>
);
const commonProps = {
icon: activeAlignment
? activeAlignment.icon
: defaultAlignmentControl.icon,
label,
controls: controls.map( ( control ) => {
return {
...BLOCK_ALIGNMENTS_CONTROLS[ control ],
isActive: value === control,
role: isCollapsed ? 'menuitemradio' : undefined,
onClick: applyOrUnset( control ),
};
} ),
};

return <UIComponent { ...commonProps } { ...extraProps } />;
}

/**
Expand Down
13 changes: 4 additions & 9 deletions packages/block-library/src/columns/test/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,7 @@ describe( 'Columns block', () => {
fireEvent.press( columnsBlock );

// Open vertical alignment menu
const verticalAlignmentButton = getByLabelText(
/Change vertical alignment/
);
const verticalAlignmentButton = getByLabelText( /Align vertically/ );
fireEvent.press( verticalAlignmentButton );

// Get Align top button
Expand All @@ -281,9 +279,8 @@ describe( 'Columns block', () => {
fireEvent.press( columnsBlock );

// Open vertical alignment menu
const verticalAlignmentButton = screen.getByLabelText(
/Change vertical alignment/
);
const verticalAlignmentButton =
screen.getByLabelText( /Align vertically/ );
fireEvent.press( verticalAlignmentButton );

// Get Align top button
Expand Down Expand Up @@ -320,9 +317,7 @@ describe( 'Columns block', () => {
fireEvent.press( columnsBlock );

// Open vertical alignment menu
const verticalAlignmentButton = getByLabelText(
/Change vertical alignment/
);
const verticalAlignmentButton = getByLabelText( /Align vertically/ );
fireEvent.press( verticalAlignmentButton );

// Get Align top button
Expand Down
9 changes: 8 additions & 1 deletion packages/block-library/src/image/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@
"name": "core/image",
"title": "Image",
"category": "media",
"usesContext": [ "allowResize", "imageCrop", "fixedHeight", "postId", "postType", "queryId" ],
"usesContext": [
"allowResize",
"imageCrop",
"fixedHeight",
"postId",
"postType",
"queryId"
],
"description": "Insert an image to make a visual statement.",
"keywords": [ "img", "photo", "picture" ],
"textdomain": "default",
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/media-text/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ function MediaTextEdit( {
<BlockVerticalAlignmentControl
onChange={ onVerticalAlignmentChange }
value={ verticalAlignment }
label={ __( 'Align media and text vertically' ) }
/>
<ToolbarButton
icon={ pullLeft }
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/paragraph/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ function ParagraphBlock( {
: dropCap,
} )
}
label={ __( 'Align text' ) }
/>
<ParagraphRTLControl
direction={ direction }
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/post-content/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@
},
"style": "wp-block-post-content",
"editorStyle": "wp-block-post-content-editor"
}
}
2 changes: 1 addition & 1 deletion packages/block-library/src/table/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ function TableEdit( {
<>
<BlockControls group="block">
<AlignmentControl
label={ __( 'Change column alignment' ) }
label={ __( 'Align columns content' ) }
alignmentControls={ ALIGNMENT_CONTROLS }
value={ getCellAlignment() }
onChange={ ( nextAlign ) =>
Expand Down
Loading