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

Broken align inside the editor for non-core blocks #25088

Open
ice9js opened this issue Sep 4, 2020 · 7 comments
Open

Broken align inside the editor for non-core blocks #25088

ice9js opened this issue Sep 4, 2020 · 7 comments
Labels
Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended

Comments

@ice9js
Copy link
Contributor

ice9js commented Sep 4, 2020

I noticed there's been some changes to how align is handled in the new WP version (5.5) which unfortunately seems to be a breaking change for all non-core blocks using the setting.

Previously the markup structure inside the editor would be something like this:

<div ...>                   // Gutenberg's Block wrapper
  <div data-align="center"> // This one is injected by Gutenberg as well and used to control the alignment
    ...                     // Actual block goes here
  </div>
</div>

Right now, for the core blocks the order seems to have switched. As in:

  • They specify supports.lightBlockWrapper: true in their definition which seems to remove the outer wrapper.
  • They then re-add the wrapper inside the individual blocks by wrapping them with __experimentalBlock which means the wrapper now ends up within the 'align div'.

Unfortunately, this causes some weird side-effects for blocks still using the 'old approach' with the wrapper spanning 100% of the page width and the toolbar always appearing to the very far left. Full-screen alignment also leaves some margin on both sides right now.

I'll be happy to write a fix but I'd appreciate some guidance as I don't know the full background for these changes.

Steps to reproduce the behavior:

  1. Create a new post/Go to the editor.
  2. Add any non-core block that has the align option and isn't by any chance using the __experimental API to the post.
  3. You should see the 'block wrapper' now spans the entire width of the editor regardless of the setting.

Expected behavior

The wrapper should wrap the block as closely as possible as is with the updated image block, or it should have a fixed width based on the align setting as was before.

Screenshots

The poll block from Crowdsignal Forms and the map embed from WordPress.com:

Screen Shot 2020-09-04 at 9 08 52 PM

Screen Shot 2020-09-04 at 9 08 05 PM

Compared to the core image block (using the new approach) in the same post:

Screen Shot 2020-09-04 at 9 08 22 PM

Editor version (please complete the following information):

  • WordPress version: 5.5.1
  • Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? default
@galloppinggryphon
Copy link

Yep, this is a problem. I found the same and reported it in #24721 a few days ago. I believed it might be related to a similar issue concerning duplication of the data-align attribute.

@youknowriad
Copy link
Contributor

Hey folks! anyone able to provide us with a simple block registration code to reproduce the issue?

@youknowriad youknowriad added [Type] Bug An existing feature does not function as intended Needs Testing Needs further testing to be confirmed. labels Mar 8, 2021
@talldan talldan added [Status] Needs More Info Follow-up required in order to be actionable. and removed Needs Testing Needs further testing to be confirmed. labels Jun 8, 2021
@github-actions
Copy link

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Jun 24, 2021
@priethor
Copy link
Contributor

Let's close this issue as we are unable to replicate and the activity has stalled. Feel free to reopen if the issue resurfaces.

@acketon
Copy link

acketon commented Jun 2, 2023

I know this is an old issue but while working on some upgrade testing to upgrade our environment from WP 5.4 to WP 5.7 I've run into the same issues with our custom blocks.

Note:
Our blocks were built a few years ago against WP 4.9 & the Gutenberg Plugin and with CGB-Scripts to compile everything. They are in use in a large network across many sites and the issues we're having with the blocks have been delaying our upgrade of the entire network for over a year now. We've fixed most of the issues that were causing blocks to completely break, or fail to load but are still tackling some of these styling & layout issues. While searching for a solution to what I've been seeing I found this issue and #24721 both of which were closed due to lack of info. I thought I could maybe pickup and add some more detail to what I'm seeing here and would welcome suggestions on ways to fix it as best as possible without rebuilding the entire block. (we have about 40 custom blocks in two plugins so rebuilding them on API 2 is going to take some effort and time)

Screen Shot 2023-06-02 at 4 46 40 PM

Screen.Recording.2023-06-02.at.4.34.28.PM.mp4

And here is the code for the block. I deleted most of this and just outputted a

in the editor and I was still getting the same double data-align properties and the opposite wrapping vs core blocks which is giving us issues with the default alignment styles applied to all blocks.

/**
 * BLOCK: bu/buniverse
 *
 * Register a BUniverse embed block with Gutenberg.
 */

// External dependencies.
import classnames from 'classnames';

// Internal dependencies.
import getAllowedFormats from '../../global/allowed-formats';
import blockIcons from '../../components/block-icons/';

//  Import CSS.
import './style.scss';
import './editor.scss';

// WordPress dependencies.
const {
	__,
} = wp.i18n;
const {
	registerBlockType,
} = wp.blocks;
const {
	Fragment,
} = wp.element;
const {
	PanelBody,
	Path,
	RadioControl,
	SVG,
	TextControl,
	ToggleControl,
} = wp.components;
const {
	InspectorControls,
	RichText,
} = ( 'undefined' === typeof wp.blockEditor ) ? wp.editor : wp.blockEditor;

/**
 * Returns the class list for the block based on the current settings.
 *
 * @param {string} className     Default classes assigned to the block.
 * @param {string} stylizedTitle If the block has a stylized title.
 */
const getClasses = ( className, aspectRatio ) => {
	return (
		classnames(
			'wp-block-global-buniverse',
			{
				[ aspectRatio ]: aspectRatio,
				[ className ]: className,
			}
		)
	);
};

// Register the block.
registerBlockType( 'bu/buniverse', {
	title: __( 'BUniverse Video' ),
	description: __( '' ),
	icon: blockIcons('buniverse'),
	category: 'bu',
	attributes: {
		id: {
			type: 'string',
		},
		aspectRatio: {
			type: 'string',
		},
		caption: {
			type: 'string',
		},
		controls: {
			type: 'number',
			default: 1,
		},
		autoplay: {
			type: 'number',
			default: 0,
		},
		start: {
			type: 'number',
		},
		minutes: {
			type: 'number',
		},
		seconds: {
			type: 'number',
		},
		className: {
			type: 'string',
			default: '',
		},
	},
	supports: {
		align: true,
	},

	edit( props ) {
		const {
			attributes: {
				id,
				aspectRatio,
				caption,
				controls,
				autoplay,
				minutes,
				seconds,
			},
			className,
			isSelected,
			setAttributes,
		} = props;

		/**
		 * Sets the value for the `minutes` attribute and
		 * calculates a new value to set for the `start` attribute.
		 *
		 * Note: no calculations are done to account for values
		 * greater than 60 entered into the `seconds` input, so
		 * as to avoid subverting expectations in cases where a
		 * user might deliberately do so.
		 *
		 * @param {string} value The value entered into the input.
		 */
		const onChangeMinutes = ( value ) => {
			const newValue = Number( value );
			const newStart = newValue * 60 + ( ( seconds ) ? seconds : 0 );

			setAttributes( { minutes: newValue } );
			setAttributes( { start: newStart } );
		};

		/**
		 * Sets the value for the `seconds` attribute and
		 * calculates a new value to set for the `start` attribute.
		 *
		 * Note: See the above note about calculating `seconds` values.
		 *
		 * @param {string} value The value entered into the input.
		 */
		const onChangeSeconds = ( value ) => {
			const newValue = Number( value );
			const newStart = newValue + ( ( minutes ) ? minutes * 60 : 0 );

			setAttributes( { seconds: newValue } );
			setAttributes( { start: newStart } );
		};

		// Build out the basic url, intentionally leaving off the extra parameters
		// because they cause the iframe to reload every time they're changed.
		const url = `//www.bu.edu/buniverse/interface/embed/embed.html?v=${id}&jsapi=1`;

		return(
			<Fragment>
				<InspectorControls>
					<PanelBody title={ __( 'Video Settings' ) }>
						<TextControl
								label={ __( 'Video ID:' ) }
								className="buniverse-set-video-id"
								value={ id }
								onChange={ ( value ) => setAttributes( { id: value } ) }
								help={ __( 'Enter the ID from the BUniverse URL') }
							/>
						<RadioControl
							className="buniverse-aspect-ratio-options"
							label={ __( 'Aspect Ratio' ) }
							selected={ aspectRatio }
							help={ __( '16:9 is typically used on widescreen video. 4:3 is often used for older fullscreen video. 1:1 is square. 9:16 and 3:4 are used for vertical video.' ) }
							options={ [
								{ label: '16:9', value: 'has-aspectratio-16by9' },
								{ label: '4:3', value: 'has-aspectratio-4by3' },
								{ label: '1:1', value: 'has-aspectratio-1by1' },
								{ label: '3:4', value: 'has-aspectratio-3by4' },
								{ label: '9:16', value: 'has-aspectratio-9by16' },
							] }
							onChange={ option => setAttributes( { aspectRatio: option } ) }
						/>
						<div className="buniverse-parameter-toggles">
							<ToggleControl
								label={ __( 'Hide Player Controls' ) }
								checked={ controls === 0 }
								onChange={ () => setAttributes( { controls: ( controls === 0 ) ? 1 : 0 } ) }
							/>
							<ToggleControl
								label={ __( 'Auto Start (muted)' ) }
								checked={ autoplay === 1 }
								onChange={ () => setAttributes( { autoplay: ( autoplay === 0 ) ? 1 : 0 } ) }
							/>
						</div>
						<div className="buniverse-start-time">
							<TextControl
								label={ __( 'Start At' ) }
								type="number"
								value={ minutes }
								onChange={ onChangeMinutes }
							/>:
							<TextControl
								type="number"
								value={ seconds }
								onChange={ onChangeSeconds }
							/>
						</div>
					</PanelBody>
				</InspectorControls>

				<figure className={ getClasses( className, aspectRatio ) }>

					<div className="wp-block-global-buniverse-wrapper">
						{ ! id && (
							<div className="wp-block-global-buinverse-placeholder">
								<div className="buniverse-logo"></div>
								<TextControl
									placeholder={ __( 'Enter BUniverse video ID here…' ) }
									value={ id }
									onChange={ ( value ) => setAttributes( { id: value } ) }
								/>
								<div className="buniverse-video-id-screenshot"></div>
							</div>
						) }
						{ id && (
							<iframe
								src={ url }
								frameborder="0"
								allow="autoplay; fullscreen"
							></iframe>
						) }
					</div>

					{ ( ! RichText.isEmpty( caption ) || isSelected ) && (
						<figcaption>
							<RichText
								tagName="p"
								className="wp-block-global-buniverse-caption wp-prepress-component-caption"
								placeholder={ __( 'Add a caption and/or media credit...' ) }
								value={ caption }
								onChange={ value => setAttributes( { caption: value } ) }
								formattingControls={ getAllowedFormats( 'formattingControls', [ 'bold', 'italic', 'link' ] ) }
								allowedFormats={ getAllowedFormats( 'allowedFormats', [ 'core/bold', 'core/italic', 'core/link' ] ) }
								keepPlaceholderOnFocus
							/>
						</figcaption>
					) }
				</figure>
			</Fragment>
		);
	},

	save( { attributes } ) {
		const {
			id,
			aspectRatio,
			caption,
			controls,
			autoplay,
			start,
			className,
		} = attributes;

		// Build out the full url.
		let url = `//www.bu.edu/buniverse/interface/embed/embed.html?v=${id}&jsapi=1`;
		url += ( controls !== 1 ) ? '&controls=0' : '';
		url += ( autoplay === 1 ) ? '&autoplay=true' : '';
		url += ( start ) ? `&start=${start}` : '';

		return(
			<figure className={ getClasses( className, aspectRatio ) }>
				<div className="wp-block-global-buniverse-wrapper">
					{ id && (
						<iframe
							src={ encodeURI( url ) }
							frameborder="0"
							allow="autoplay; fullscreen"
						></iframe>
					) }
				</div>
					{ caption && (
						<figcaption>
							<p class="wp-block-global-buniverse-caption wp-prepress-component-caption">
								{ caption }
							</p>
						</figcaption>
					)}

			</figure>
		);
	},
} );

@priethor priethor removed [Status] Needs More Info Follow-up required in order to be actionable. [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. labels Jun 5, 2023
@priethor priethor reopened this Jun 5, 2023
@acketon
Copy link

acketon commented Jun 6, 2023

Here is a smaller sample block I created that also has the same issues:

/**
 * BLOCK: bu/sample
 *
 * Register a sample block with Gutenberg.
 */

// External dependencies.
//import classnames from 'classnames';

// Internal dependencies.


//  Import CSS.
import './style.scss';
import './editor.scss';

// WordPress dependencies.
const {
	__,
} = wp.i18n;
const {
	registerBlockType,
} = wp.blocks;


// Register the block.
registerBlockType( 'bu/sample', {
	title: __( 'Sample Block' ),
	description: __( '' ),
	category: 'bu',
	attributes: {

	},
	supports: {
		align: true,
	},

	edit( props ) {
		const {
			attributes: {

			},
			className,
			isSelected,
			setAttributes,
		} = props;


		return(

			<div className={ className }>
				<div className="wp-block-sample-inner">
					<p>Sample Block</p>
				</div>
			</div>

		);
	},

	save( { attributes } ) {
		const {
			className,
		} = attributes;

		return(
			<div className={ className }>
				<div className="wp-block-sample">
					<p>Sample Block</p>
				</div>
			</div>
		);
	},
} );

@acketon
Copy link

acketon commented Jun 16, 2023

I've unfortunately been unable to find a solution to resolve this in WP5.7. It seems left & right aligned blocks that use apiVersion: 1 have broken styles in the editor for the Toolbar position, outline, selection of the block in some cases, and just alignment in general.

With a lot of custom CSS I can fix some of those issues but can't get the toolbar to be positioned properly above the block when it is floated, it is positioned farther away. Similarly the outline provided by the :after pseudo element won't wrap the block. I could create my own outline styles on different elements but that seems like a bad approach.

The best solution I've found is to upgrade the blocks to apiVersion:2 but that is time consuming and requires refactoring some code for some of the blocks. It looks like there are some differences to how className is handled that doesn't seem to be documented anywhere I can find. We have a number of blocks that on apiVersion:1 were checking className.includes('is-style-something') for a selected style and altering the markup or UI and those break because className is sometimes undefined after switching to block api 2. I haven't found any good documentation on upgrading blocks to block api 2 which makes this all the more difficult to figure out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

7 participants