Skip to content

"Multiple use" block validation logic improvement.#40901

Closed
delowardev wants to merge 1 commit into
WordPress:trunkfrom
delowardev:fix/block-validation-on-multiple-use
Closed

"Multiple use" block validation logic improvement.#40901
delowardev wants to merge 1 commit into
WordPress:trunkfrom
delowardev:fix/block-validation-on-multiple-use

Conversation

@delowardev

@delowardev delowardev commented May 6, 2022

Copy link
Copy Markdown
Contributor

What?

Fixes: #38502

Why?

Any block that doesn't support the multiple feature, doesn't display the Multiple blocks validation warning when it's inside group blocks.

How?

Include innerBlocks in the validation process.

Testing Instructions

  1. Open post or page to edit
  2. Insert "Group" block
  3. Insert the "More (core/more)" block inside the "Group" block
  4. Duplicate "Group" block.

The "Read more" should display the validation warning.

Screenshots or screencast

Before:

before.mov

After:

after.mov

@delowardev

delowardev commented May 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@getdave Thanks for your feedback on the previous PR. ( My local git history got messed up somehow, So I had to close that PR )

I tried with the core/read-more block and it seems to support multiple. I wonder if that's a mistake?

Could you please try with core/more block; it doesn't support multiple

Using within a Query Loop in the Site Editor, if I duplicate the loop then the editor shows an error. Presumably, it's valid to use the More block twice when it's part of a template so we're going to have to make the validation logic more flexible.

I think core/more is not supposed to be used in the query loop/post-template; instead, it should be used in the post content.

e2e Test has been added.

@skorasaurus skorasaurus added the [Package] Edit Post /packages/edit-post label Dec 25, 2023
@github-actions

github-actions Bot commented Dec 25, 2023

Copy link
Copy Markdown

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: [Package] Edit Post.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

getdave
getdave reviewed Jan 3, 2024
View reviewed changes

@getdave getdave left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. It looks like a good enhancement 👍

Do you think you'll be able to move the tests to use Playwright as we now only use that tool to write new tests?

If you're struggling for time please let us know.

Thanks again

Comment thread packages/e2e-tests/specs/editor/various/validate-multiple-use.test.js
*/
import { createNewPost, insertBlock } from '@wordpress/e2e-test-utils';

describe( 'Validate multiple use', () => {

@getdave getdave Jan 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this test to use Playwright?

@delowardev delowardev Jan 3, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @getdave, thanks for your review. I'm away from work for the next few weeks, so It would be nice if anyone were interested in taking this issue.

@fai-sal fai-sal mentioned this pull request Jan 5, 2024
Merged
@getdave

getdave commented Feb 9, 2024

Copy link
Copy Markdown
Contributor

Shall we close this one in favour of #57576?

@getdave getdave closed this Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@getdave getdave getdave left review comments
@ntwb ntwb Awaiting requested review from ntwb ntwb is a code owner
@nerrad nerrad Awaiting requested review from nerrad nerrad is a code owner
@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner
+1 more reviewer
@NewArk1 NewArk1 NewArk1 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Package] Edit Post /packages/edit-post

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Avoid multiple block adding while "multiple" is false in block.json

4 participants

@delowardev @getdave @NewArk1 @skorasaurus