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

Unify how editor alignments are applied across blocks #21822

Merged
merged 9 commits into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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: 2 additions & 2 deletions packages/base-styles/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ $z-layers: (
".components-drop-zone": 40,
".components-drop-zone__content": 50,

// The block mover for floats should overlap the controls of adjacent blocks.
".block-editor-block-list__block {core/image aligned left or right}": 21,
// The floated blocks should be above the other blocks to allow selection.
"{core/image aligned left or right} .wp-block": 21,

// Small screen inner blocks overlay must be displayed above drop zone,
// settings menu, and movers.
Expand Down
42 changes: 36 additions & 6 deletions packages/block-editor/src/components/block-list/block-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,16 @@ import { BlockContext } from './block';
import ELEMENTS from './block-elements';

const BlockComponent = forwardRef(
( { children, tagName = 'div', __unstableIsHtml, ...props }, wrapper ) => {
(
{
children,
tagName = 'div',
__unstableIsHtml,
wrapperProps: parentWrapperProps,
Copy link
Contributor Author

@youknowriad youknowriad Apr 23, 2020

Choose a reason for hiding this comment

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

This is just needed for the image block now, but we can remove it if ever we refactor it to use the "align" hook instead of a custom implementation.

Copy link
Member

@ellatrix ellatrix Apr 23, 2020

Choose a reason for hiding this comment

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

Can we not use the block context for this?

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 how you'd do so, retrieve the context value, alter it and add a new provider? It would work but it's also not great code either, I'd rather remove this at some point (after the image refactor).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we can probably use getEditPropsWrapper

Copy link
Member

Choose a reason for hiding this comment

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

I meant adding it to useContext( BlockContext ) below

...props
},
wrapper
) => {
const onSelectionStart = useContext( Context );
const [ , setBlockNodes ] = useContext( BlockNodes );
const {
Expand All @@ -49,7 +58,7 @@ const BlockComponent = forwardRef(
name,
mode,
blockTitle,
wrapperProps,
wrapperProps: contextWrapperProps,
} = useContext( BlockContext );
const { initialPosition } = useSelect(
( select ) => {
Expand All @@ -69,7 +78,11 @@ const BlockComponent = forwardRef(
'core/block-editor'
);
const fallbackRef = useRef();

const wrapperProps = {
...contextWrapperProps,
...parentWrapperProps,
};
const isAligned = wrapperProps && !! wrapperProps[ 'data-align' ];
wrapper = wrapper || fallbackRef;

// Provide the selected node, or the first and last nodes of a multi-
Expand Down Expand Up @@ -196,19 +209,20 @@ const BlockComponent = forwardRef(
const blockElementId = `block-${ clientId }${ htmlSuffix }`;
const Animated = animated[ tagName ];

return (
const blockWrapper = (
<Animated
// Overrideable props.
aria-label={ blockLabel }
role="group"
{ ...wrapperProps }
{ ...omit( wrapperProps, [ 'data-align' ] ) }
{ ...props }
id={ blockElementId }
ref={ wrapper }
className={ classnames(
className,
props.className,
wrapperProps && wrapperProps.className
wrapperProps && wrapperProps.className,
! isAligned && 'wp-block'
) }
data-block={ clientId }
data-type={ name }
Expand All @@ -227,6 +241,22 @@ const BlockComponent = forwardRef(
{ children }
</Animated>
);

// For aligned blocks, provide a wrapper element so the block can be
// positioned relative to the block column. This is enabled with the
// .is-block-content className.
if ( isAligned ) {
const alignmentWrapperProps = {
'data-align': wrapperProps[ 'data-align' ],
};
return (
<div className="wp-block" { ...alignmentWrapperProps }>
{ blockWrapper }
</div>
);
}

return blockWrapper;
}
);

Expand Down
9 changes: 1 addition & 8 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function BlockListBlock( {
const wrapperClassName = classnames(
generatedClassName,
customClassName,
'wp-block block-editor-block-list__block',
'block-editor-block-list__block',
{
'has-warning': ! isValid || !! hasError || isUnregisteredBlock,
'is-selected': isSelected,
Expand Down Expand Up @@ -144,13 +144,6 @@ function BlockListBlock( {
/>
);

// For aligned blocks, provide a wrapper element so the block can be
// positioned relative to the block column. This is enabled with the
// .is-block-content className.
if ( ! lightBlockWrapper && isAligned ) {
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
blockEdit = <div className="is-block-content">{ blockEdit }</div>;
}

if ( mode !== 'visual' ) {
blockEdit = <div style={ { display: 'none' } }>{ blockEdit }</div>;
}
Expand Down
65 changes: 28 additions & 37 deletions packages/block-editor/src/components/block-list/style.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
.block-editor-block-list__block {
margin-left: auto;
margin-right: auto;
}

.block-editor-block-list__layout .block-editor-block-list__block { // Needs specificity to override inherited styles.
// While block is being dragged, dim the slot dragged from, and hide some UI.
&.is-dragging {
Expand Down Expand Up @@ -143,7 +138,6 @@
right: $border-width;
}

.is-block-content, // Floats.
&::after { // Everything else.
// 2px outside.
box-shadow: 0 0 0 $border-width-focus $blue-medium-focus;
Expand Down Expand Up @@ -248,17 +242,28 @@
cursor: default;
}

.alignleft,
.alignright {
// Without z-index, won't be clickable as "above" adjacent content.
z-index: z-index(".block-editor-block-list__block {core/image aligned left or right}");
// Clear floats.
&[data-clear="true"] {
float: none;
}

// This essentially duplicates the mobile styles for the appender component.
// It would be nice to be able to use element queries in that component instead https://github.com/tomhodgins/element-queries-spec
.block-editor-block-list__layout {
.block-editor-default-block-appender .block-editor-inserter {
left: auto;
right: $grid-unit-10;
}
}
}

.wp-block {
margin-left: auto;
margin-right: auto;

// Alignments.
&[data-align="left"],
&[data-align="right"] {
// Without z-index, won't be clickable as "above" adjacent content.
z-index: z-index(".block-editor-block-list__block {core/image aligned left or right}");
width: 100%;

// When images are floated, the block itself should collapse to zero height.
Expand All @@ -269,8 +274,14 @@
}
}

&[data-align="left"] > *,
&[data-align="right"] > * {
// Without z-index, won't be clickable as "above" adjacent content.
z-index: z-index("{core/image aligned left or right} .wp-block");
}

// Left.
&[data-align="left"] > .is-block-content {
&[data-align="left"] > * {
// This is in the editor only; the image should be floated on the frontend.
/*!rtl:begin:ignore*/
float: left;
Expand All @@ -279,7 +290,7 @@
}

// Right.
&[data-align="right"] > .is-block-content {
&[data-align="right"] > * {
// Right: This is in the editor only; the image should be floated on the frontend.
/*!rtl:begin:ignore*/
float: right;
Expand All @@ -289,25 +300,9 @@

// Wide and full-wide.
&[data-align="full"],
&[data-align="wide"],
&.alignfull,
&.alignwide {
&[data-align="wide"] {
clear: both;
}

// Clear floats.
&[data-clear="true"] {
float: none;
}

// This essentially duplicates the mobile styles for the appender component.
// It would be nice to be able to use element queries in that component instead https://github.com/tomhodgins/element-queries-spec
.block-editor-block-list__layout {
.block-editor-default-block-appender .block-editor-inserter {
left: auto;
right: $grid-unit-10;
}
}
}

/**
Expand Down Expand Up @@ -624,12 +619,8 @@
padding-right: $block-side-ui-width;
}

// Full-wide. (to account for the padddings added above)
// The first two rules account for the alignment wrapper div for the image block.
> div:not(.block-editor-block-list__block) > .block-editor-block-list__block[data-align="full"],
> div:not(.block-editor-block-list__block) > .block-editor-block-list__block.alignfull,
> .block-editor-block-list__block[data-align="full"],
> .block-editor-block-list__block.alignfull {
// Full-wide. (to account for the paddings added above)
> .wp-block[data-align="full"] {
margin-left: -$block-padding;
margin-right: -$block-padding;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@
padding-left: 0;
padding-right: 0;

> div:not(.block-editor-block-list__block) > .block-editor-block-list__block[data-align="full"],
> div:not(.block-editor-block-list__block) > .block-editor-block-list__block.alignfull,
> .block-editor-block-list__block[data-align="full"],
> .block-editor-block-list__block.alignfull {
> .wp-block[data-align="full"] {
margin-left: 0;
margin-right: 0;
}
Expand Down
18 changes: 8 additions & 10 deletions packages/block-library/src/button/editor.scss
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
.block-editor-block-list__block[data-type="core/button"] {
&[data-align="center"] {
text-align: center;
margin-left: auto;
margin-right: auto;
}
.wp-block[data-align="center"] .wp-block-button {
text-align: center;
margin-left: auto;
margin-right: auto;
}

&[data-align="right"] {
/*!rtl:ignore*/
text-align: right;
}
.wp-block[data-align="right"] .wp-block-button {
/*!rtl:ignore*/
text-align: right;
}

.wp-block-button {
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/button/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ $blocks-button__height: 56px;
cursor: pointer;
display: inline-block;
font-size: $big-font-size;
margin: 0;
padding: 12px 24px;
text-align: center;
text-decoration: none;
Expand Down
7 changes: 4 additions & 3 deletions packages/block-library/src/buttons/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import {
__experimentalAlignmentHookSettingsProvider as AlignmentHookSettingsProvider,
InnerBlocks,
__experimentalBlock as Block,
} from '@wordpress/block-editor';

/**
Expand All @@ -19,17 +20,17 @@ const alignmentHooksSetting = {
isEmbedButton: true,
};

function ButtonsEdit( { className } ) {
function ButtonsEdit() {
return (
<div className={ className }>
<Block.div>
<AlignmentHookSettingsProvider value={ alignmentHooksSetting }>
<InnerBlocks
allowedBlocks={ ALLOWED_BLOCKS }
template={ BUTTONS_TEMPLATE }
__experimentalMoverDirection="horizontal"
/>
</AlignmentHookSettingsProvider>
</div>
</Block.div>
);
}

Expand Down
26 changes: 12 additions & 14 deletions packages/block-library/src/buttons/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,18 @@
width: auto;
}

.wp-block-buttons {
&[data-align="center"] .block-editor-block-list__layout {
display: flex;
align-items: center;
flex-wrap: wrap;
justify-content: center;
}
.wp-block[data-align="center"] > .wp-block-buttons {
display: flex;
align-items: center;
flex-wrap: wrap;
justify-content: center;
}

&[data-align="right"] .block-editor-block-list__layout {
display: flex;
justify-content: flex-end;
}
.wp-block[data-align="right"] > .wp-block-buttons {
display: flex;
justify-content: flex-end;
}

.block-editor-block-list__layout > div:last-child {
display: inline-block;
}
.wp-block-buttons .block-list-appender {
display: inline-block;
}
1 change: 1 addition & 0 deletions packages/block-library/src/buttons/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const settings = {
supports: {
align: true,
alignWide: false,
lightBlockWrapper: true,
},
transforms,
edit,
Expand Down
14 changes: 7 additions & 7 deletions packages/block-library/src/cover/editor.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
.wp-block-cover-image,
.wp-block-cover {

&.components-placeholder h2 {
color: inherit;
}
Expand All @@ -23,13 +22,14 @@
.wp-block-cover__placeholder-background-options {
width: 100%;
}
}

// Apply max-width to floated items that have no intrinsic width.
[data-align="left"] &,
[data-align="right"] & {
max-width: $content-width / 2;
width: 100%;
}
[data-align="left"] > .wp-block-cover,
[data-align="right"] > .wp-block-cover,
[data-align="left"] > .wp-block-cover-image,
[data-align="right"] > .wp-block-cover-image {
max-width: $content-width / 2;
Copy link
Member

@ellatrix ellatrix Apr 23, 2020

Choose a reason for hiding this comment

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

Why is width being defined here?

Copy link
Member

Choose a reason for hiding this comment

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

I'm just curious, it's not a comment to this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just guessing

I believe if we don't do it and align a block (left or right) we won't notice any change as otherwise it will stay with a 100% width

width: 100%;
}

.block-library-cover__reset-button {
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/embed/style.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Apply max-width to floated items that have no intrinsic width
.block-editor-block-list__block[data-type*="core-embed"][data-align="left"] .is-block-content,
.block-editor-block-list__block[data-type*="core-embed"][data-align="right"] .is-block-content,
.wp-block[data-align="left"] > .wp-block-embed,
.wp-block[data-align="right"] > .wp-block-embed,
.wp-block-embed.alignleft,
.wp-block-embed.alignright {
// Instagram widgets have a min-width of 326px, so go a bit beyond that.
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/group/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
/**
* Group: Full Width Alignment
*/
.wp-block[data-type="core/group"][data-align="full"] {
.wp-block[data-align="full"] .wp-block-group {

// First tier of InnerBlocks must act like the container of the standard canvas
> div > .wp-block-group > .wp-block-group__inner-container > .block-editor-inner-blocks {
Expand Down
Loading