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: Add group block variations to Group toolbar #39920

Merged
merged 8 commits into from
Apr 3, 2022
Merged

Group: Add group block variations to Group toolbar #39920

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
33a47aa
Add group variations to the Group block toolbar
Mar 30, 2022
0985239
Check the newblocks array has items in it
Mar 30, 2022
23464a8
There is no point in setting attribute if type===group
Mar 31, 2022
cf14af7
Add comment about why updateBlockAttributes is not used
Mar 31, 2022
b7a53bb
Move variations const outside of component
Mar 31, 2022
c3b3046
Fix toolbar group selectors
youknowriad Mar 31, 2022
1428c39
Improve Row translation context
aaronrobertshaw Apr 1, 2022
954ef35
Rename variations to layouts and remove group check
aaronrobertshaw Apr 1, 2022
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
33 changes: 28 additions & 5 deletions packages/block-editor/src/components/convert-to-group-buttons/toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { useDispatch, useSelect } from '@wordpress/data';
import { switchToBlockType } from '@wordpress/blocks';
import { ToolbarButton, ToolbarGroup } from '@wordpress/components';
import { group } from '@wordpress/icons';
import { group, row, stack } from '@wordpress/icons';
import { _x } from '@wordpress/i18n';

/**
Expand All @@ -13,7 +13,7 @@ import { _x } from '@wordpress/i18n';
import { useConvertToGroupButtonProps } from '../convert-to-group-buttons';
import { store as blockEditorStore } from '../../store';

function BlockGroupToolbar( { label = _x( 'Group', 'verb' ) } ) {
function BlockGroupToolbar() {
const {
blocksSelection,
clientIds,
Expand All @@ -32,16 +32,29 @@ function BlockGroupToolbar( { label = _x( 'Group', 'verb' ) } ) {
[ clientIds ]
);

const onConvertToGroup = () => {
const variationAttributes = {
row: { type: 'flex' },
stack: { type: 'flex', orientation: 'vertical' },
};
Mamaduka marked this conversation as resolved.
Show resolved Hide resolved

const onConvertToGroup = ( variation = 'group' ) => {
const newBlocks = switchToBlockType(
blocksSelection,
groupingBlockName
);
if ( newBlocks ) {

if ( newBlocks && newBlocks.length > 0 ) {
newBlocks[ 0 ].attributes.layout =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would be nice if these attributes could be passed to switchToBlockType, or some related method, but I couldn't see a way to do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... maybe updateBlockAttributes would be better here, will take a look at that

Copy link
Contributor Author

@glendaviesnz glendaviesnz Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I tried replacing this with

updateBlockAttributes( newBlocks[ 0 ].clientId, {
	layout: variationAttributes[ variation ],
} );

which seems like the more legit way to do this, but it doesn't work for some reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

which seems like the more legit way to do this, but it doesn't work for some reason?

Given the new group block isn't yet in the block editor store, the use of updateBlockAttributes won't work here. As @talldan suggested, perhaps a comment explaining why the current approach is used might save someone else in future from wondering the same thing.

variation !== 'group'
? variationAttributes[ variation ]
: undefined;
replaceBlocks( clientIds, newBlocks );
}
};

const onConvertToRow = () => onConvertToGroup( 'row' );
const onConvertToStack = () => onConvertToGroup( 'stack' );

// Don't render the button if the current selection cannot be grouped.
// A good example is selecting multiple button blocks within a Buttons block:
// The group block is not a valid child of Buttons, so we should not show the button.
Expand All @@ -54,9 +67,19 @@ function BlockGroupToolbar( { label = _x( 'Group', 'verb' ) } ) {
<ToolbarGroup>
<ToolbarButton
icon={ group }
label={ label }
label={ _x( 'Group', 'verb' ) }
onClick={ onConvertToGroup }
/>
<ToolbarButton
icon={ row }
label={ _x( 'Row', 'verb' ) }
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
onClick={ onConvertToRow }
/>
<ToolbarButton
icon={ stack }
label={ _x( 'Stack', 'verb' ) }
onClick={ onConvertToStack }
/>
</ToolbarGroup>
);
}
Expand Down