Size Change: +38 B (0%)

Total Size: 1.94 MB

_{compressed-size-action}

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: michalczaplinski <czapla@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Thank you @michalczaplinski!

Unfortunately, e2e tests do not seem to agree with this reasoning 😂 -- I'll have to figure out what's going on there.

Thank you @michalczaplinski!

Unfortunately, e2e tests do not seem to agree with this reasoning 😂 -- I'll have to figure out what's going on there.

😅 . Yeah, you're right.

I thought this might be because the current PR should perhaps be stacked on top of #63100. I checkout out both and merged #63100 into the current one locally, but the e2e tests still fail.

I've started digging. Here's one of the tests that's failing:

The "Large Quote" variation of the Quote block is registered by an e2e test helper plugin:

Here's a screenshot (that's part of the artifact generated by CI when the e2e test failed):

I tried to repro that e2e error manually but haven't been able so far.

Anyway, I suspect it has to do with two different variations having isDefault set to true 😬

Anyway, I suspect it has to do with two different variations having isDefault set to true 😬

It’s possible to accidentally set multiple variations as default through plugins so it picks the first default found if I recall correctly.

This issue has been punted to 6.8 and has not seen any activity since. Let's move this issue to the 6.9 Editor Task Board and see what can be done in 6.9.

For now, I would like to merge the latest trunk into this PR, but first I would appreciate it if you could check whether this PR is currently valid.

From what I can tell, the reason for the E2E test failure is that the displayed block name has changed. For example, try selecting the 50 / 50 variation in the Columns block.

I think we need the logic to check the scope somewhere. 80338cf is one approach, but it may not be correct.

Flaky tests detected in ea07950.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17402571594
📝 Reported issues:

• [Flaky Test] ensures List View global shortcut works properly #51533 in /test/e2e/specs/editor/various/list-view.spec.js

Thank you very much for picking this up, @t-hamano, and apologies for not replying sooner!

Your fix looks good and makes sense to me. It might require some more tweaking, as the newly added unit tests are failing now.

80338cf is one approach, but it may not be correct.

I realized that 80338cf is not the right approach 😅 If I cherry-pick ecedae0 on top of this PR, the normal Group variation is no longer marked as active in the sidebar:

In any case, this PR does not appear to be essential for WP 6.9, so I would like to remove it from the project board.

I realized that 80338cf is not the right approach 😅 If I cherry-pick ecedae0 on top of this PR, the normal Group variation is no longer marked as active in the sidebar: [...]

I think I found a solution to this problem: 7dad4d2

We still need to update the unit test to set the scope argument to either block or transform (and maybe add a test for inserter scope and/or no scope argument to verify that it does not fall back to the isDefault variation in those cases). We should probably also audit other callsites of getActiveBlockVariation.

In any case, this PR does not appear to be essential for WP 6.9, so I would like to remove it from the project board.

Yep, that's perfectly fine IMO 👍

@ockham Thanks for the update!

P.S. When we ship this PR, the logic of the getActiveBlockVariation selector will change a bit, so a dev note might be needed. I"m sure the selector is often used by block developers.

P.S. I tested #63100 (comment) again with the following code:

wp.blocks.registerBlockVariation("core/group", {
	name: "wpcomsp/marquee-content",
	title: "Marquee Content",
	icon: "editor-table",
	description: "A marquee content block.",
	scope: ["block", "inserter"],
	attributes: {
		isMarquee: true,
		layout: { type: "flex", flexWrap: "nowrap" },
		className: "wpcomsp-marquee-content",
		allowedBlocks: [
			"core/paragraph",
			"core/heading",
			"core/image",
			"core/buttons",
		],
	},
	isActive: ["layout.type", "isMarquee"],
});

I can see the Marquee Content block in the inserter, but upon inserting I get the Row block instead.

Curiously, when I increase the specificity by adding an additional item in the isActive array like "layout.flexWrap", it seems to work correctly for the Marquee Content block:

  isActive: ["layout.type", "layout.flexWrap", "isMarquee"],

I tested the above behavior and this PR seems to be better than trunk.

When inserting the "Marquee Content" block...

Thank you for resurrecting this, @t-hamano!

Just to be safe, let's add the Needs Dev Note label.