Group block variations not recognized as active

[Humanify-nl]

Description

Using a custom block variation and selecting it in the block inspector does not show the 'is-pressed' css class.

The selected varation does in fact get shown in the editor, it is just the button press that does not get registered somehow.

Step-by-step reproduction instructions

Step 1: add a custom variation to the group block.

import { __ } from '@wordpress/i18n';
import { group, row, stack } from '@wordpress/icons';

wp.domReady(() => {
  wp.blocks.registerBlockVariation(
    'core/group',
    {
      name: 'group-section',
      title: __( 'Section' ),
      description: __( 'Gather blocks in a section container.' ),
      attributes: {
        align: 'full',
        className: "is-section",
        layout: { type: 'default', inherit: true }
      },
      scope: [ 'inserter', 'transform' ],
      isActive: ( blockAttributes ) =>
        ! blockAttributes.layout ||
        ! blockAttributes.layout?.type ||
        blockAttributes.layout?.type === 'default',
      icon: group,
    }
  );
}

Step 2: select the variation in the editor.
Step 3: notice that the first button shows is-pressed, instead of the custom variation.

Screenshots, screen recording, code snippet

Environment info

• WordPress 6.0 with theme.json theme
• Gutenberg plugin not installed.

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[Mamaduka]

Hi, @Humanify-nl

Your isActive check matches the default group variation check, so the logic has no way to tell the difference.

I would recommend updating isActive and making it specific to your variation.

gutenberg/packages/block-library/src/group/variations.js

Lines 14 to 17 in 90995b6

	isActive: ( blockAttributes ) =>
	! blockAttributes.layout ||
	! blockAttributes.layout?.type ||
	blockAttributes.layout?.type === 'default',

[Humanify-nl]

Thanks @Mamaduka ,

I didnt realize that isActive logic is needed for a new variation (docs say optional), I guess this has to do with the new icon-based variation selector that is different from regular variation dropdown on which I couldn't find anything in the docs. Apologies for the incorrect bug report!

Sadly, I can't get the logic to work since the group attributes are mostly the same, and adding below code doesn't seem to work.

isActive: ( blockAttributes ) =>
  blockAttributes.className === 'is-section',

I think I'll have to change the core default group isActive function, like:

isActive: ( blockAttributes ) => 
 	! blockAttributes.layout || 
 	! blockAttributes.layout?.type || 
 	blockAttributes.layout?.type === 'default' 
        && ! blockAttributes.className === 'is-section'`. 

But I rather not change the default block code.

It seems a bit complicated that a thing like a variation selector needs this much logic, just to make a new custom variation happen. Similar as to why there is no css class placed on a variation. On the front-end we have no way to know what kind of group block variation we are dealing with, while there are so many use-cases where this is needed.

You're just adding a class. Why don't you use a block style?

We use this with for things like:

• Removing margins between sections that have different background colors, but only on direct children of root, and only for default groups (and not row or stack).
• Adding top and bottom padding to sections that are always needed in our designs, but only for direct children of root, not all groups on the site.

It's possible to do all this with CSS but it needs a lot of code, and is not great, especially when template-parts come into play. A simple class would solve this: .is-section + .is-section { margin-top: 0; }. But I would not really call this a case for a 'block style'.

What to do?

[giannis-koulouris-bb]

I have the same issue, the custom class is being added, but theres no way to trigger isActive on my custom variation, even though I've made it specific for my variation.

[Humanify-nl]

Another attempt at this, since wp6.0.1:

This is my new variation:

wp.blocks.registerBlockVariation(
    'core/group',
    {
      name: 'group-section',
        title: __( 'Section' ),
        description: __( 'Fullwidth section with padding.' ),
        attributes: {
          align: 'full',
          className: "is-section",
          layout: { inherit: true },
          tagName: 'section',
        },
  	scope: [ 'inserter', 'transform' ],
        isActive: ( blockAttributes ) =>
           ! blockAttributes.layout?.type === 'flex' &&
           blockAttributes.className === 'is-section',
        icon: group,
    }
  );

This overrides the default group, and updates the isActive:

  // inherit layout & fullwidth as default variation
  wp.blocks.registerBlockVariation(
    'core/group',
    {
      isDefault: true,
      attributes: {
        align: 'full',
        layout: {
          inherit: true,
        },
      },
      isActive: ( blockAttributes ) =>
        ( ! blockAttributes.layout ||
        ! blockAttributes.layout?.type ||
        blockAttributes.layout?.type === 'default' ) &&
        ! blockAttributes.className === 'is-section',
    }
  );

I've tried a few ways to do this. Maybe my logic is wrong, but in the current state of variation switcher, it seems way too complicated to add a new variation. Is there a way to simplify this? Like, is there a way to register the button press without logic, and instead use a js event?

@youknowriad , I see you've done a lot of work on these variations, do you have an idea how to do this properly?

[cr0ybot]

The issue for the group block is that the logic for determining isActive never gets to any custom variations, because the default group variation (not stack or row) is checking if layout.type is either 'default' or 'constrained', which our custom group variations probably are. I'm a little confused why the default group needs an explicit variation isActive check honestly, if none of the other variations match then it should default isActive to the variation marked isDefault, right?

Here is the default variation:

gutenberg/packages/block-library/src/group/variations.js

Lines 7 to 20 in 05381d2

	const variations = [
	{
	name: 'group',
	title: __( 'Group' ),
	description: __( 'Gather blocks in a container.' ),
	attributes: { layout: { type: 'constrained' } },
	scope: [ 'transform' ],
	isActive: ( blockAttributes ) =>
	! blockAttributes.layout ||
	! blockAttributes.layout?.type ||
	blockAttributes.layout?.type === 'default' ||
	blockAttributes.layout?.type === 'constrained',
	icon: group,
	},

See what I mean? If you're checking for a custom classname or literally anything else, if your variation also happens to use the default or constrained layout value, or doesn't even define layout.type at all, it will think the default Group variation is active.

EDIT: Some additional information on how variations are considered "active":

When you register a block variation via registerBlockVariation(), your variation object is added to the end of an array of variations for the block. When it comes time to check which variation is active, Array.find() is being used under the hood here:

gutenberg/packages/blocks/src/store/selectors.js

Lines 245 to 269 in 44731b9

	export function getActiveBlockVariation( state, blockName, attributes, scope ) {
	const variations = getBlockVariations( state, blockName, scope );
	const match = variations?.find( ( variation ) => {
	if ( Array.isArray( variation.isActive ) ) {
	const blockType = getBlockType( state, blockName );
	const attributeKeys = Object.keys( blockType?.attributes || {} );
	const definedAttributes = variation.isActive.filter(
	( attribute ) => attributeKeys.includes( attribute )
	);
	if ( definedAttributes.length === 0 ) {
	return false;
	}
	return definedAttributes.every(
	( attribute ) =>
	attributes[ attribute ] ===
	variation.attributes[ attribute ]
	);
	}
	return variation.isActive?.( attributes, variation.attributes );
	} );
	return match;
	}

Array.find() "returns the first element in the provided array that satisfies the provided testing function" (MDN). Therefore, since the very first variation defined for the group block is generally satisfied by any additional variation we might ourselves add (unless you're messing with layout.type), no variations on the group block will be considered "active" until something changes.

[ryelle]

I'm running into this on a project now— trying to make a group container with some default content, and I'd like it to be a block variation (rather than pattern or style) so that it can be separate in the List View, but it's never considered "selected".

My code:

registerBlockVariation( 'core/group', {
	name: 'sidebar-container',
	title: __( 'Sidebar Container', 'wporg' ),
	description: __( 'Put blocks in a sticky sidebar alongside content.', 'wporg' ),
	scope: [ 'inserter' ],
	attributes: { className: 'sidebar-container' },
	isActive: [ 'className' ],
	innerBlocks: [ /* default sidebar content */ ]
} );

Changing my attributes to attributes: { layout: { type: 'flex', orientation: 'nonsense' }, className: 'sidebar-container' }, works, but really shouldn't be the answer 😆

I'm going to remove the "needs technical feedback" label because I think we have plenty ^, and add "Bug" because this seems fairly unexpected.

I'm a little confused why the default group needs an explicit variation isActive check honestly, if none of the other variations match then it should default isActive to the variation marked isDefault, right?

I tried this very quickly, and it seems like that prevents the icon from appearing selected in the sidebar.

Given that doesn't quite work, I think the better solution here would be to fix this in the Block API, to check all variations & allow for some solution when multiple isActives are true.

[pbiron]

Changing my attributes to attributes: { layout: { type: 'flex', orientation: 'nonsense' }, className: 'sidebar-container' }, works, but really shouldn't be the answer 😆

I bumped into this problem a few days ago and came looking for an existing issue before opening one of my own.

I can confirm that the hack/work-around @ryelle mentions does work, but as she says, it is far from ideal...as it requires added additional CSS when the group needs to be constrained to style correctly.

Given that doesn't quite work, I think the better solution here would be to fix this in the Block API, to check all variations & allow for some solution when multiple isActives are true.

As I started to look into this problem my first thought for a solution was to try to change getActiveBlockVariation() so that the isActive property from core were always checked after any variations defined outside of core (in plugin/theme). But I quickly realized that while that may work in some cases, it isn't general enough to handle all cases.

So, Kelly's point about a resolution mechanism when multiple variations are matched is needed. Exactly what the resolution mechanism is, I don't have any bright ideas at this time ;-)

[ali-awwad]

what @ryelle mentioned only works until you save, once saved the active state will not be presistent. However, using the same logic I found another workaround:

instead of relying on namespace , I used the same const for className. then inside the isActive function I check for the className.

const GROUP_CARD_VARIATION = 'my-card-group-variation';

registerBlockVariation('core/group', {
        name: GROUP_CARD_VARIATION,
        title: 'Card Group',
        description: 'Give card attributes.',
        icon: 'calendar-alt',
        attributes: {
            layout: { type: 'flex', orientation: 'nonsense' },
            className: GROUP_CARD_VARIATION,
            roundedCorner: {
                type: 'boolean',
                default: false,
            }
        },
        isActive: ({ className }) => {
            return (
                className.includes(GROUP_CARD_VARIATION) // check if className contains GROUP_CARD_VARIATION and not equals. incase you add additional css classes it will still work
            );
        },
        allowedControls: [],
        scope: ['inserter', 'transform'],
    });

and if you are adding filters, like my case, dont forget to use the same logic as above:

const withBookQueryControls = (BlockEdit) => (props) => {
    if (!props?.attributes?.className?.includes(GROUP_CARD_VARIATION)) {
        return <BlockEdit {...props} />
    }
    const { attributes, setAttributes } = props;
    const roundedCss = ' rounded-md overflow-hidden';
    return <>
        <BlockEdit {...props} />
        <InspectorControls>
            <PanelBody title="Card Settings">
                <ToggleControl
                    label="Rounded Corner"
                    checked={attributes.className.includes(roundedCss)}
                    onChange={(v) => setAttributes({ className: v ? attributes.className + roundedCss : attributes.className.replace(roundedCss, '') })}
                />
            </PanelBody>
        </InspectorControls>
    </>
};

addFilter('editor.BlockEdit', 'core/group', withBookQueryControls);

[bhdd]

Is there any known workaround for getting isActive to work for a group block variation of type: 'constrained'? I want to then add block styles to that variation.

[jethin]

+1 to resolving this issue. I'd like to trigger isActive in a group block variation that uses the default layout attributes. To reiterate the above: the default group block variations are consuming isActive (via layout attributes) before it has a chance to check any custom group block variations.

In the meantime I've come up with the following workaround. This is something of a hack, and replaces the entire class attribute of the group on the front end.

Use the flex layout attribute in the block variation to trigger isActive (I'm using "smc-quiz" as my key):

layout: { type: "flex", orientation: "smc-quiz" }

Then filter the block to replace the class attribute server side (here using flow layout):

function smc_group_replace_class($block_content){
    if( is_single() && str_contains($block_content, 'is-smc-quiz') ){
        $block_content = preg_replace('/class="[^"]*/', 'class="wp-block-group is-smc-quiz is-layout-flow wp-block-group-is-layout-flow', $block_content, 1);
    }
    return $block_content;
}
add_filter( 'render_block_core/group', 'smc_group_replace_class', 10, 1 );

You may want to change the returned class and/or edit the conditional checks for your instance.

The code above can probably be improved (can a block variation be targeted by the "render_block" filter?) Please feel to do so.

[ockham]

I think I have an idea how to fix this issue. It consists of two parts:

1. Block Variations: Have getActiveBlockVariation return variation with highest specificity #62031 tl;dr: If there are multiple matches, and all of their isActive properties are arrays (of attributes), return the match whose isActive property has the most items.
2. Block Variations: Support dot notation in isActive string array #62068 E.g. if the variation has a layout attribute that's an object, and that object has a type key (among others), then isArray: [ 'layout.type' ] will compare the value for that key to the block instance's "counterpart".

[ntsekouras]

That's a hard problem indeed. We had some explorations in the past for this to see how it would be best to extend a core block and its variations. We ended up adding a 'dumb' attribute(namespace) in Query Loop block that's been taken into account in isActive function and can be used in filters, hooks, php, etc.. While it might not be ideal, this approach seems reliable and is similar to the workaround suggested above(className, etc..).

Maybe something like that could be a reliable solution, by injecting in all blocks an attribute(like the namespace) with a supports flag? 🤔

[ockham]

Maybe something like that could be a reliable solution, by injecting in all blocks an attribute(like the namespace) with a supports flag? 🤔

I replied to this part in a comment on my PR. Quoting the most relevant part:

Now while [#62031] cannot detect the active variation in the situation you're describing, I believe it would cover the specific use case described in #41303 (if we also implement the idea in #62068) and AFAICS, there wouldn't be any adverse side effects. Do you think it's viable to land those two things before we consider adding a "silent" attribute (like the namespace in this example) to all variations?