WordPress / gutenberg

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

Registering block variation transform with class name overrides existing classes #31064

Closed coreyworrell closed 3 years ago

coreyworrell commented 3 years ago

Description

When using wp.blocks.registerBlockVariation(), if you set { attributes: { className: 'is-style-example' } }, any additional classes that had been set in the Advanced panel for the block are removed when using the "Transform to variation" option.

I would assume that the class would just be added/removed to any existing classes instead of completely overriding them.

Step-by-step reproduction instructions

  1. Register block variation
  2. Add Heading block
  3. Select new variation from the "Transform to variation" option that appears in editor inspector sidebar.

Expected behaviour

The class is-style-serif would be added to the additional CSS classes and the variation would be active.

Actual behaviour

All additional classes are removed and only the className attribute value is added.

Screenshots or screen recording (optional)

https://streamable.com/q7lsgd

Code snippet (optional)

const { registerBlockVariation } = wp.blocks

registerBlockVariation('core/heading', {
    name: 'serif',
    title: 'Serif',
    scope: ['transform'],
    attributes: {
        className: 'is-style-serif',
    },
})

WordPress information

Device information

gziolo commented 3 years ago

It seems like a valid concern, @ntsekouras what do you think?

ntsekouras commented 3 years ago

While the issue @coreyworrell describes is valid, the example provided is the perfect example of when to use block styles see here.

Does you use case involves more attributes in the block variation?

I guess we could add explicit support for className merging but I'm not too fond of adding an attribute special handling explicitly. On the other hand className is quite special on its own... 🤔

coreyworrell commented 3 years ago

@ntsekouras It would be a perfect case for block styles, except that you can currently only select a single block style. We are already using a style for a bordered header, so we cannot select both bordered and serif styles at once.

coreyworrell commented 3 years ago

If you notice in the recording also that you cannot unselect the variation. Maybe that is intended, but as of now I guess if you want to register a variation you must actually register two variations (a default and the other).

ntsekouras commented 3 years ago

@coreymckrill I understand your use case and it is a valid scenario. The approach for this though should involve only the block styles feature and not mix the two APIs (block styles, block variations).

Block variations API is a way to better handle similar blocks with different attributes that mostly affect the block's behaviour. For example we cannot unset a block variation because it would rely on arbitrary decisions about the new attribute values.

I'll close thin in favor of #14598 which is about allowing blocks to support multiple block styles.

coreymckrill commented 3 years ago

cc @coreyworrell