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

Refactor the Media&text block to use the new color support flag #21169

Merged
merged 4 commits into from
Mar 30, 2020
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
6 changes: 0 additions & 6 deletions packages/block-library/src/media-text/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@
"type": "string",
"default": "wide"
},
"backgroundColor": {
Copy link
Member

Choose a reason for hiding this comment

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

It's great to see all this disappearing :)

"type": "string"
},
"customBackgroundColor": {
"type": "string"
},
"mediaAlt": {
"type": "string",
"source": "attribute",
Expand Down
190 changes: 179 additions & 11 deletions packages/block-library/src/media-text/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import classnames from 'classnames';
import { noop } from 'lodash';
import { noop, isEmpty, omit } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -16,6 +16,21 @@ import { imageFillStyles } from './media-container';

const DEFAULT_MEDIA_WIDTH = 50;

const migrateCustomColors = ( attributes ) => {
if ( ! attributes.customBackgroundColor ) {
return attributes;
}
const style = {
color: {
background: attributes.customBackgroundColor,
},
};
return {
...omit( attributes, [ 'customBackgroundColor' ] ),
style,
};
};

const baseAttributes = {
align: {
type: 'string',
Expand All @@ -24,9 +39,6 @@ const baseAttributes = {
backgroundColor: {
type: 'string',
},
customBackgroundColor: {
type: 'string',
},
mediaAlt: {
type: 'string',
source: 'attribute',
Expand All @@ -41,12 +53,6 @@ const baseAttributes = {
mediaId: {
type: 'number',
},
mediaUrl: {
type: 'string',
source: 'attribute',
selector: 'figure video,figure img',
attribute: 'src',
},
mediaType: {
type: 'string',
},
Expand All @@ -64,6 +70,39 @@ export default [
{
attributes: {
...baseAttributes,
customBackgroundColor: {
type: 'string',
},
mediaLink: {
type: 'string',
},
linkDestination: {
type: 'string',
},
linkTarget: {
type: 'string',
source: 'attribute',
selector: 'figure a',
attribute: 'target',
},
href: {
type: 'string',
source: 'attribute',
selector: 'figure a',
attribute: 'href',
},
rel: {
type: 'string',
source: 'attribute',
selector: 'figure a',
attribute: 'rel',
},
linkClass: {
type: 'string',
source: 'attribute',
selector: 'figure a',
attribute: 'class',
},
verticalAlignment: {
type: 'string',
},
Expand All @@ -74,6 +113,124 @@ export default [
type: 'object',
},
},
migrate: migrateCustomColors,
save( { attributes } ) {
const {
backgroundColor,
customBackgroundColor,
isStackedOnMobile,
mediaAlt,
mediaPosition,
mediaType,
mediaUrl,
mediaWidth,
mediaId,
verticalAlignment,
imageFill,
focalPoint,
linkClass,
href,
linkTarget,
rel,
} = attributes;
const newRel = isEmpty( rel ) ? undefined : rel;

let image = (
<img
src={ mediaUrl }
alt={ mediaAlt }
className={
mediaId && mediaType === 'image'
? `wp-image-${ mediaId }`
: null
}
/>
);

if ( href ) {
image = (
<a
className={ linkClass }
href={ href }
target={ linkTarget }
rel={ newRel }
>
{ image }
</a>
);
}

const mediaTypeRenders = {
image: () => image,
video: () => <video controls src={ mediaUrl } />,
};
const backgroundClass = getColorClassName(
'background-color',
backgroundColor
);
const className = classnames( {
'has-media-on-the-right': 'right' === mediaPosition,
'has-background': backgroundClass || customBackgroundColor,
[ backgroundClass ]: backgroundClass,
'is-stacked-on-mobile': isStackedOnMobile,
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
'is-image-fill': imageFill,
} );
const backgroundStyles = imageFill
? imageFillStyles( mediaUrl, focalPoint )
: {};

let gridTemplateColumns;
if ( mediaWidth !== DEFAULT_MEDIA_WIDTH ) {
gridTemplateColumns =
'right' === mediaPosition
? `auto ${ mediaWidth }%`
: `${ mediaWidth }% auto`;
}
const style = {
backgroundColor: backgroundClass
? undefined
: customBackgroundColor,
gridTemplateColumns,
};
return (
<div className={ className } style={ style }>
<figure
className="wp-block-media-text__media"
style={ backgroundStyles }
>
{ ( mediaTypeRenders[ mediaType ] || noop )() }
</figure>
<div className="wp-block-media-text__content">
<InnerBlocks.Content />
</div>
</div>
);
},
},
{
attributes: {
...baseAttributes,
customBackgroundColor: {
type: 'string',
},
mediaUrl: {
type: 'string',
source: 'attribute',
selector: 'figure video,figure img',
attribute: 'src',
},
verticalAlignment: {
type: 'string',
},
imageFill: {
type: 'boolean',
},
focalPoint: {
type: 'object',
},
},
migrate: migrateCustomColors,
save( { attributes } ) {
const {
backgroundColor,
Expand Down Expand Up @@ -147,7 +304,18 @@ export default [
},
},
{
attributes: baseAttributes,
attributes: {
...baseAttributes,
customBackgroundColor: {
type: 'string',
},
mediaUrl: {
type: 'string',
source: 'attribute',
selector: 'figure video,figure img',
attribute: 'src',
},
},
save( { attributes } ) {
const {
backgroundColor,
Expand Down
20 changes: 0 additions & 20 deletions packages/block-library/src/media-text/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import {
BlockVerticalAlignmentToolbar,
InnerBlocks,
InspectorControls,
PanelColorSettings,
withColors,
__experimentalImageURLInputUI as ImageURLInputUI,
} from '@wordpress/block-editor';
import { Component } from '@wordpress/element';
Expand Down Expand Up @@ -183,10 +181,8 @@ class MediaTextEdit extends Component {
const {
attributes,
className,
backgroundColor,
isSelected,
setAttributes,
setBackgroundColor,
image,
} = this.props;
const {
Expand All @@ -210,8 +206,6 @@ class MediaTextEdit extends Component {
const classNames = classnames( className, {
'has-media-on-the-right': 'right' === mediaPosition,
'is-selected': isSelected,
'has-background': backgroundColor.class || backgroundColor.color,
[ backgroundColor.class ]: backgroundColor.class,
'is-stacked-on-mobile': isStackedOnMobile,
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
'is-image-fill': imageFill,
Expand All @@ -224,15 +218,7 @@ class MediaTextEdit extends Component {
const style = {
gridTemplateColumns,
msGridColumns: gridTemplateColumns,
backgroundColor: backgroundColor.color,
};
const colorSettings = [
{
value: backgroundColor.color,
onChange: setBackgroundColor,
label: __( 'Background color' ),
},
];
const toolbarControls = [
{
icon: pullLeft,
Expand Down Expand Up @@ -311,11 +297,6 @@ class MediaTextEdit extends Component {
<>
<InspectorControls>
{ mediaTextGeneralSettings }
<PanelColorSettings
title={ __( 'Color settings' ) }
initialOpen={ false }
colorSettings={ colorSettings }
/>
</InspectorControls>
<BlockControls>
<ToolbarGroup controls={ toolbarControls } />
Expand Down Expand Up @@ -352,7 +333,6 @@ class MediaTextEdit extends Component {
}

export default compose( [
withColors( 'backgroundColor' ),
withSelect( ( select, props ) => {
const { getMedia } = select( 'core' );
const {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/media-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const settings = {
supports: {
align: [ 'wide', 'full' ],
html: false,
__experimentalColor: true,
},
example: {
attributes: {
Expand Down
11 changes: 1 addition & 10 deletions packages/block-library/src/media-text/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { noop, isEmpty } from 'lodash';
/**
* WordPress dependencies
*/
import { InnerBlocks, getColorClassName } from '@wordpress/block-editor';
import { InnerBlocks } from '@wordpress/block-editor';

/**
* Internal dependencies
Expand All @@ -18,8 +18,6 @@ const DEFAULT_MEDIA_WIDTH = 50;

export default function save( { attributes } ) {
const {
backgroundColor,
customBackgroundColor,
isStackedOnMobile,
mediaAlt,
mediaPosition,
Expand Down Expand Up @@ -66,14 +64,8 @@ export default function save( { attributes } ) {
image: () => image,
video: () => <video controls src={ mediaUrl } />,
};
const backgroundClass = getColorClassName(
'background-color',
backgroundColor
);
const className = classnames( {
'has-media-on-the-right': 'right' === mediaPosition,
'has-background': backgroundClass || customBackgroundColor,
[ backgroundClass ]: backgroundClass,
'is-stacked-on-mobile': isStackedOnMobile,
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
'is-image-fill': imageFill,
Expand All @@ -90,7 +82,6 @@ export default function save( { attributes } ) {
: `${ mediaWidth }% auto`;
}
const style = {
backgroundColor: backgroundClass ? undefined : customBackgroundColor,
gridTemplateColumns,
};
return (
Expand Down
7 changes: 5 additions & 2 deletions packages/block-library/src/media-text/style.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
.wp-block-media-text {
background-color: var(--wp--color--background);
color: var(--wp--color--text);

// This block's direction should not be manipulated by rtl, as the mediaPosition control does.
/*!rtl:begin:ignore*/
direction: ltr;
Expand Down Expand Up @@ -65,8 +68,8 @@
/*!rtl:end:ignore*/
}

.wp-block-media-text > figure > img,
.wp-block-media-text > figure > video {
.wp-block-media-text__media img,
.wp-block-media-text__media video {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasmussen looks like the styles to handle the image height properly were already present but not applied in the editor because of an extra div. This should fix it

max-width: unset;
width: 100%;
vertical-align: middle;
Expand Down