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

Allow blocks to handle align as a classname instead of a wrapper block #31201

Open
dougwollison opened this issue Apr 26, 2021 · 1 comment
Open
Labels
Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.

Comments

@dougwollison
Copy link
Contributor

dougwollison commented Apr 26, 2021

What problem does this address?

Currently, if a block offers alignment handling, it wraps the block in a [data-align] div, regardless of if the block supports the light wrapper. This breaks any styling that relies on a block's placement in a container or relation to other blocks (e.g. first/last child, or proceeding a sepcific other block).

What is your proposed solution?

Either an alignWrapper support flag, or have the align support accept an object with the values and withWrapper specified (would also potentially allow additional alignment-related rules to be specified). This would then tell the with-data-align hook to apply it as a classname rather than wrapperProps.

The former is how I'm patching it in on a project, as a BlockListBlock filter that runs just before withDataAlign, removing the align proper from wrapperProps and adding it as a class instead.

addFilter( 'editor.BlockListBlock', 'premise/no-align-wrapper', createHigherOrderComponent( BlockListBlock => props => {
	const { name: blockName, className, wrapperProps, ...otherProps } = props;
	const alignWrapper = hasBlockSupport( blockName, 'premise/noAlignWrapper', true );

	if ( wrapperProps && ! alignWrapper ) {
		// extract the align value
		const { 'data-align': align, newWrapperProps } = wrapperProps;

		// add it a classname
		let newClassName = className;
		if ( align ) {
			newClassName = classnames( className, `align${ align }` );
		}

		// use the new classname + wrapper props, sans data-align
		return <BlockListBlock
			{ ...otherProps }
			name={ name }
			className={ newClassName }
			wrapperProps={ newWrapperProps }
		/>;
	}

	return <BlockListBlock { ...props } />;
} ), 9 );
@paaljoachim paaljoachim added the Needs Technical Feedback Needs testing from a developer perspective. label May 6, 2021
@annezazu annezazu added the [Type] Enhancement A suggestion for improvement. label May 6, 2021
@justintadlock
Copy link
Contributor

This has been the bane of my existence for a while. What I ended up doing to fix this (I hope) is split my :first-child rule between the front-end and editor. I'm not sure if there are edge cases I'm missing.

Even if this is the ideal solution for now, the ultimate goal is for themes to have a single stylesheet for both the front end and editor. I'd love to see this addressed.

Front end stylesheet:

:first-child {
	margin-top: 0;
}

Editor stylesheet:

:not(.wp-block[data-align]) > :first-child {
	margin-top: 0;
}

I use margin-top for vertical alignment. Others use :last-child + margin-bottom. The concept should be the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Technical Feedback Needs testing from a developer perspective. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants