WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10k stars 4.02k forks source link

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

Open ice9js opened 3 years ago

ice9js commented 3 years ago

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:

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):

galloppinggryphon commented 3 years ago

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 commented 3 years ago

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

github-actions[bot] commented 3 years ago

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.

priethor commented 2 years ago

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 commented 1 year ago

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

https://github.com/WordPress/gutenberg/assets/3139096/c4f0fb22-3c1c-48c9-bddb-8424bcdf7b35

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>
        );
    },
} );
acketon commented 1 year ago

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 commented 1 year ago

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.