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

Add initial draft of container block #10562

Closed
wants to merge 8 commits into from
5 changes: 5 additions & 0 deletions packages/block-library/src/container/editor.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.wp-block-container .editor-block-list__block {
// We need to enforce a cascade by telling nested blocks to inherit
Copy link
Member

Choose a reason for hiding this comment

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

Where is there a color otherwise being assigned that is breaking the default cascade? It's not clear to me that this should be needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@aduth See my comment here: #10067 (comment)

We map body to .editor-block-list__block inside editor-styles.scss which means each block resets the cascade for these properties.

Copy link
Member

Choose a reason for hiding this comment

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

That's a bit unfortunate 😕 But OK 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed we should refactor the editor styles to apply to the editor container. The downside though is that it can affect the UI bits (toolbars, movers...) so we need to be careful there.

Copy link
Member Author

Choose a reason for hiding this comment

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

I almost wonder if there's a way to do a forced reset for Gutenberg chrome? Something to ponder for the future but for now this works :)

// their text color from the container block.
color: inherit;
}
159 changes: 159 additions & 0 deletions packages/block-library/src/container/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { withFallbackStyles } from '@wordpress/components';
import { compose } from '@wordpress/compose';
import { __, sprintf } from '@wordpress/i18n';
import {
getColorClassName,
withColors,
ContrastChecker,
InnerBlocks,
InspectorControls,
PanelColorSettings,
} from '@wordpress/editor';
import { Fragment } from '@wordpress/element';

const applyFallbackStyles = withFallbackStyles( ( node, ownProps ) => {
const { textColor, backgroundColor } = ownProps.attributes;

return {
fallbackBackgroundColor: backgroundColor,
fallbackTextColor: textColor,
};
} );

export const name = 'core/container';

export const settings = {
title: sprintf(
/* translators: Block title modifier */
__( '%1$s (%2$s)' ),
__( 'Container' ),
__( 'beta' )
Copy link
Member

Choose a reason for hiding this comment

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

Is the UI freeze a block freeze? In which case, I don't think we should want anything "beta"-denoted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch 👍

),

icon: <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><path d="M19 12h-2v3h-3v2h5v-5zM7 9h3V7H5v5h2V9zm14-6H3c-1.1 0-2 .9-2 2v14c0 1.1.9 2 2 2h18c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zm0 16.01H3V4.99h18v14.02z" /><path d="M0 0h24v24H0z" fill="none" /></svg>,

category: 'layout',

attributes: {
align: {
type: 'string',
},
backgroundColor: {
type: 'string',
},
customBackgroundColor: {
type: 'string',
},
textColor: {
type: 'string',
},
customTextColor: {
type: 'string',
},
},

description: __( 'Group blocks into a container.' ),

supports: {
align: [ 'wide', 'full' ],
anchor: true,
},

edit: compose( [
withColors( 'backgroundColor', { textColor: 'color' } ),
applyFallbackStyles,
] )( ( props ) => {
const {
backgroundColor,
className,
fallbackBackgroundColor,
fallbackTextColor,
setBackgroundColor,
setTextColor,
textColor,
} = props;

return (
<Fragment>
<InspectorControls>
<PanelColorSettings
title={ __( 'Color Settings' ) }
initialOpen={ false }
colorSettings={ [
{
value: backgroundColor.color,
onChange: setBackgroundColor,
label: __( 'Background Color' ),
},
{
value: textColor.color,
onChange: setTextColor,
label: __( 'Text Color' ),
},
] }
>
<ContrastChecker
{ ...{
textColor: textColor.color,
backgroundColor: backgroundColor.color,
fallbackTextColor,
fallbackBackgroundColor,
} }
/>
</PanelColorSettings>
</InspectorControls>
<div
className={ classnames( className, {
'has-background': backgroundColor.color,
'has-text-color': textColor.color,
[ backgroundColor.class ]: backgroundColor.class,
[ textColor.class ]: textColor.class,
} ) }
style={ {
backgroundColor: backgroundColor.color,
Copy link
Member

Choose a reason for hiding this comment

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

Should the inline styles be necessary? Why isn't it enough to assign the color classes?

(Maybe this is a known limitation, but at a high-level it feels wrong)

Copy link
Member Author

Choose a reason for hiding this comment

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

@aduth if someone picks a custom color, it has to be applied via style. I just copied this over from the paragraph block.

Copy link
Member

Choose a reason for hiding this comment

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

So then what's the purpose of the class name being assigned?

Again, not particularly directed at you or this block, maybe more a general issue we have with colors assignment.

Copy link
Member Author

@chrisvanpatten chrisvanpatten Oct 16, 2018

Choose a reason for hiding this comment

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

@aduth If you pick one of the colors in the palette, it assigns a class. If you pick a color outside the palette, it applies a style.

This is the rendered HTML for a block using the default red/light gray colors:

<div class="wp-block-container has-background has-text-color has-vivid-red-background-color has-very-light-gray-color">
<p>Container, Paragraph One</p>
<p>Container, Paragraph Two</p>
</div>

Vs this rendered HTML from the same block using two custom colors selected via the color picker popover:

<div class="wp-block-container has-background has-text-color" style="background-color:#2eaacf;color:#ffffff">
<p>Container, Paragraph One</p>
<p>Container, Paragraph Two</p>
</div>

I'm not quite sure how this magic works, but it does 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

@aduth Oh I see you're referring to the edit representation of the block, not the front-end version, and that's a great question. I'll take a closer look.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pushing up a commit to fix this. I can open a separate PR to do this for the paragraph block as well.

color: textColor.color,
} }
>
<InnerBlocks />
</div>
</Fragment>
);
} ),

save( { attributes } ) {
const {
backgroundColor,
customBackgroundColor,
customTextColor,
textColor,
} = attributes;

const backgroundColorClass = getColorClassName( 'background-color', backgroundColor );
const textColorClass = getColorClassName( 'color', textColor );

return (
<div
className={ classnames( {
'has-background': backgroundColor || customBackgroundColor,
'has-text-color': textColor || customTextColor,
[ backgroundColorClass ]: backgroundColorClass,
[ textColorClass ]: textColorClass,
} ) }
style={ {
backgroundColor: backgroundColorClass ? undefined : customBackgroundColor,
color: textColorClass ? undefined : customTextColor,
} }
>
<InnerBlocks.Content />
</div>
);
},
};
5 changes: 5 additions & 0 deletions packages/block-library/src/container/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.wp-block-container {
// 1px top/bottom padding allows us to prevent margin collapsing while
Copy link
Member

Choose a reason for hiding this comment

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

Does margin collapse happen on the front-end, or is this meant to offset the editor styles? If the latter, should we then include this only in an editor-specific stylesheet?

Copy link
Member Author

@chrisvanpatten chrisvanpatten Oct 16, 2018

Choose a reason for hiding this comment

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

Front and back-end, but in my testing it was good/necessary for both cases.

// avoiding excessive top/bottom margins.
padding: 1px 1em;
}
1 change: 1 addition & 0 deletions packages/block-library/src/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
@import "./categories/editor.scss";
@import "./code/editor.scss";
@import "./columns/editor.scss";
@import "./container/editor.scss";
@import "./cover-image/editor.scss";
@import "./embed/editor.scss";
@import "./file/editor.scss";
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import * as categories from './categories';
import * as code from './code';
import * as columns from './columns';
import * as column from './columns/column';
import * as container from './container';
import * as coverImage from './cover-image';
import * as embed from './embed';
import * as file from './file';
Expand Down Expand Up @@ -69,6 +70,7 @@ export const registerCoreBlocks = () => {
code,
columns,
column,
container,
coverImage,
embed,
...embed.common,
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
@import "./button/style.scss";
@import "./categories/style.scss";
@import "./columns/style.scss";
@import "./container/style.scss";
@import "./cover-image/style.scss";
@import "./embed/style.scss";
@import "./file/style.scss";
Expand Down