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

Image block: Normalize the block toolbar #29205

Merged
merged 4 commits into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ _Related_

- <https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-editor/src/components/autocomplete/README.md>

<a name="BlockAlignmentControl" href="#BlockAlignmentControl">#</a> **BlockAlignmentControl**

Undocumented declaration.

<a name="BlockAlignmentToolbar" href="#BlockAlignmentToolbar">#</a> **BlockAlignmentToolbar**

Undocumented declaration.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Internal dependencies
*/
import BlockAlignmentUI from './ui';

export function BlockAlignmentControl( props ) {
return <BlockAlignmentUI { ...props } isToolbar={ false } />;
}

export function BlockAlignmentToolbar( props ) {
return <BlockAlignmentUI { ...props } isToolbar />;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`BlockAlignmentToolbar should match snapshot 1`] = `
exports[`BlockAlignmentUI should match snapshot 1`] = `
<ToolbarGroup
controls={
Array [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { useSelect } from '@wordpress/data';
/**
* Internal dependencies
*/
import { BlockAlignmentToolbar } from '../';
import BlockAlignmentUI from '../ui';

jest.mock( '@wordpress/data/src/components/use-select', () => {
// This allows us to tweak the returned value on each test
Expand All @@ -20,12 +20,12 @@ jest.mock( '@wordpress/data/src/components/use-select', () => {
} );
useSelect.mockImplementation( () => ( { wideControlsEnabled: false } ) );

describe( 'BlockAlignmentToolbar', () => {
describe( 'BlockAlignmentUI', () => {
const alignment = 'left';
const onChange = jest.fn();

const wrapper = shallow(
<BlockAlignmentToolbar value={ alignment } onChange={ onChange } />
<BlockAlignmentUI value={ alignment } onChange={ onChange } isToolbar />
);

const controls = wrapper.props().controls;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { ToolbarGroup } from '@wordpress/components';
import { DropdownMenu, ToolbarGroup } from '@wordpress/components';
import { useSelect } from '@wordpress/data';
import {
positionCenter,
Expand Down Expand Up @@ -49,19 +49,21 @@ const POPOVER_PROPS = {
isAlternate: true,
};

export function BlockAlignmentToolbar( {
function BlockAlignmentUI( {
value,
onChange,
controls = DEFAULT_CONTROLS,
isToolbar,
Copy link
Member

Choose a reason for hiding this comment

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

Do we plan to remove this once we transition?

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'm not sure we can, we have a valid use-case for it at the moment: The inspector control of the latest posts block. Also, it might have some impact on third party blocks.

isCollapsed = true,
isToolbarButton = true,
} ) {
const { wideControlsEnabled = false } = useSelect( ( select ) => {
const { getSettings } = select( blockEditorStore );
const settings = getSettings();
return {
wideControlsEnabled: settings.alignWide,
};
} );
}, [] );
const layout = useLayout();
const supportsAlignments = layout.type === 'default';

Expand All @@ -88,10 +90,12 @@ export function BlockAlignmentToolbar( {
const defaultAlignmentControl =
BLOCK_ALIGNMENTS_CONTROLS[ DEFAULT_CONTROL ];

const UIComponent = isToolbar ? ToolbarGroup : DropdownMenu;
const extraProps = isToolbar ? { isCollapsed } : { isToolbarButton };

return (
<ToolbarGroup
<UIComponent
popoverProps={ POPOVER_PROPS }
isCollapsed={ isCollapsed }
icon={
activeAlignmentControl
? activeAlignmentControl.icon
Expand All @@ -107,8 +111,9 @@ export function BlockAlignmentToolbar( {
onClick: applyOrUnset( control ),
};
} ) }
{ ...extraProps }
/>
);
}

export default BlockAlignmentToolbar;
export default BlockAlignmentUI;
5 changes: 4 additions & 1 deletion packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ export * from './gradients';
export * from './font-sizes';
export { default as AlignmentToolbar } from './alignment-toolbar';
export { default as Autocomplete } from './autocomplete';
export { default as BlockAlignmentToolbar } from './block-alignment-toolbar';
export {
BlockAlignmentControl,
BlockAlignmentToolbar,
} from './block-alignment-control';
export { default as __experimentalBlockFullHeightAligmentToolbar } from './block-full-height-alignment-toolbar';
export { default as __experimentalBlockAlignmentMatrixToolbar } from './block-alignment-matrix-toolbar';
export { default as BlockBreadcrumb } from './block-breadcrumb';
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/index.native.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Block Creation Components
export { default as BlockAlignmentToolbar } from './block-alignment-toolbar';
export { BlockAlignmentToolbar } from './block-alignment-control';
export { BlockContextProvider } from './block-context';
export { default as BlockControls } from './block-controls';
export { default as BlockEdit, useBlockEditContext } from './block-edit';
Expand Down
75 changes: 34 additions & 41 deletions packages/block-library/src/image/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,14 +264,6 @@ export function ImageEdit( {
}, [ isTemp ] );

const isExternal = isExternalImage( id, url );
const controls = (
<BlockControls>
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
/>
</BlockControls>
);
const src = isExternal ? url : undefined;
const mediaPreview = !! url && (
<img
Expand All @@ -282,21 +274,6 @@ export function ImageEdit( {
/>
);

const mediaPlaceholder = (
<MediaPlaceholder
icon={ <BlockIcon icon={ icon } /> }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
notices={ noticeUI }
onError={ onUploadError }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
mediaPreview={ mediaPreview }
disableMediaButtons={ url }
/>
);

const classes = classnames( className, {
'is-transient': isBlobURL( url ),
'is-resized': !! width || !! height,
Expand All @@ -310,25 +287,41 @@ export function ImageEdit( {
} );

return (
<>
{ controls }
<figure { ...blockProps }>
{ url && (
<Image
attributes={ attributes }
setAttributes={ setAttributes }
isSelected={ isSelected }
insertBlocksAfter={ insertBlocksAfter }
onReplace={ onReplace }
onSelectImage={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
containerRef={ ref }
<figure { ...blockProps }>
{ url && (
<Image
attributes={ attributes }
setAttributes={ setAttributes }
isSelected={ isSelected }
insertBlocksAfter={ insertBlocksAfter }
onReplace={ onReplace }
onSelectImage={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
containerRef={ ref }
/>
) }
{ ! url && (
<BlockControls>
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
/>
) }
{ mediaPlaceholder }
</figure>
</>
</BlockControls>
) }
<MediaPlaceholder
icon={ <BlockIcon icon={ icon } /> }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
notices={ noticeUI }
onError={ onUploadError }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
mediaPreview={ mediaPreview }
disableMediaButtons={ url }
/>
</figure>
);
}

Expand Down
2 changes: 0 additions & 2 deletions packages/block-library/src/image/image-editing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ export default function ImageEditor( {
<AspectRatioDropdown toggleProps={ toggleProps } />
) }
</ToolbarItem>
</ToolbarGroup>
<ToolbarGroup>
<RotationButton />
</ToolbarGroup>
<ToolbarGroup>
Expand Down
61 changes: 35 additions & 26 deletions packages/block-library/src/image/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
__experimentalImageURLInputUI as ImageURLInputUI,
MediaReplaceFlow,
store as blockEditorStore,
BlockAlignmentControl,
} from '@wordpress/block-editor';
import { useEffect, useState, useRef } from '@wordpress/element';
import { __, sprintf, isRTL } from '@wordpress/i18n';
Expand Down Expand Up @@ -259,6 +260,16 @@ export default function Image( {
} );
}

function updateAlignment( nextAlign ) {
const extraUpdatedAttributes = [ 'wide', 'full' ].includes( nextAlign )
? { width: undefined, height: undefined }
: {};
setAttributes( {
...extraUpdatedAttributes,
align: nextAlign,
} );
}

useEffect( () => {
if ( ! isSelected ) {
setIsEditingImage( false );
Expand All @@ -271,8 +282,12 @@ export default function Image( {
const controls = (
<>
<BlockControls>
{ ! multiImageSelection && ! isEditingImage && (
<ToolbarGroup>
<ToolbarGroup>
<BlockAlignmentControl
value={ align }
onChange={ updateAlignment }
/>
{ ! multiImageSelection && ! isEditingImage && (
<ImageURLInputUI
url={ href || '' }
onChangeUrl={ onSetHref }
Expand All @@ -283,39 +298,22 @@ export default function Image( {
linkClass={ linkClass }
rel={ rel }
/>
</ToolbarGroup>
) }
{ allowCrop && (
<ToolbarGroup>
) }
{ allowCrop && (
<ToolbarButton
onClick={ () => setIsEditingImage( true ) }
icon={ crop }
label={ __( 'Crop' ) }
/>
</ToolbarGroup>
) }
{ externalBlob && (
<ToolbarGroup>
) }
{ externalBlob && (
<ToolbarButton
onClick={ uploadExternal }
icon={ upload }
label={ __( 'Upload external image' ) }
/>
</ToolbarGroup>
) }
{ ! multiImageSelection && ! isEditingImage && (
<MediaReplaceFlow
mediaId={ id }
mediaURL={ url }
allowedTypes={ ALLOWED_MEDIA_TYPES }
accept="image/*"
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
onError={ onUploadError }
/>
) }
{ ! multiImageSelection && coverBlockExists && (
<ToolbarGroup>
) }
{ ! multiImageSelection && coverBlockExists && (
<ToolbarButton
icon={ textColor }
label={ __( 'Add text over image' ) }
Expand All @@ -326,7 +324,18 @@ export default function Image( {
)
}
/>
</ToolbarGroup>
) }
</ToolbarGroup>
{ ! multiImageSelection && ! isEditingImage && (
<MediaReplaceFlow
mediaId={ id }
mediaURL={ url }
allowedTypes={ ALLOWED_MEDIA_TYPES }
accept="image/*"
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
onError={ onUploadError }
/>
) }
</BlockControls>
<InspectorControls>
Expand Down
8 changes: 6 additions & 2 deletions packages/components/src/dropdown-menu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { menu } from '@wordpress/icons';
* Internal dependencies
*/
import Button from '../button';
import ToolbarButton from '../toolbar-button';
import Dropdown from '../dropdown';
import { NavigableMenu } from '../navigable-container';

Expand Down Expand Up @@ -49,6 +50,7 @@ function DropdownMenu( {
menuLabel,
position,
noIcons,
isToolbarButton = false,
Copy link
Member

Choose a reason for hiding this comment

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

Was this added for convenience? This can already be achieved with this code:

<ToolbarItem>
  { ( toolbarItemProps ) => (
    <DropdownMenu toggleProps={ toolbarItemProps } />
  ) }
</ToolbarItem>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL I guess I'll refactor these to avoid the prop :)

} ) {
if ( menuLabel ) {
deprecated( '`menuLabel` prop in `DropdownComponent`', {
Expand Down Expand Up @@ -84,6 +86,8 @@ function DropdownMenu( {
popoverProps
);

const ButtonComponent = isToolbarButton ? ToolbarButton : Button;

return (
<Dropdown
className={ classnames( 'components-dropdown-menu', className ) }
Expand Down Expand Up @@ -113,7 +117,7 @@ function DropdownMenu( {
);

return (
<Button
<ButtonComponent
{ ...mergedToggleProps }
icon={ icon }
onClick={ ( event ) => {
Expand All @@ -135,7 +139,7 @@ function DropdownMenu( {
showTooltip={ toggleProps?.showTooltip ?? true }
>
{ mergedToggleProps.children }
</Button>
</ButtonComponent>
);
} }
renderContent={ ( props ) => {
Expand Down