Fixes #11949: Error: The block "xxx" can have a maximum of 3 keywords.#11953
Fixes #11949: Error: The block "xxx" can have a maximum of 3 keywords.#11953jameelmoses wants to merge 3 commits intoWordPress:masterfrom
Conversation
gziolo
left a comment
There was a problem hiding this comment.
@jameelmoses thanks for your contribution. The change that was introduced causes one of the unit tests to fail. You can run locally npm run test-unit to find out more details. It has to be updated to reflect code changes.
The changes proposed in this PR look good and it seems like a good compromise. I would be happy to get another opinion from @aduth or @youknowriad.
|
@gziolo I ran unit tests, and 2 failed but they are seemingly unrelated to my changes. |
gziolo
left a comment
There was a problem hiding this comment.
The udated version of the code removes the check for keywords completely which is different than the initial proposal. I don’t think this is expected API change.
This PR still have some merge conflicts so that migt why those unrelated tests fail.
I proposed the change at #11949 (comment) . I'd welcome your feedback if you have any reservations, though. |
|
I missed the discussion in the issue and I reviewed based on the previous agreement. I’m fine with your proposal applied in this PR as is. Let’s make sure thete are no merge conflicts and proceed with what we have at the moment 👍 This might help to close similar PRs 😃 |
There was a problem hiding this comment.
I did a cursory review of the codebase to ensure there were no other references to the three-keyword maximum.
I found a reference as part of the @wordpress/rich-text package, but I believe this to be an entirely separate thing, though presumably inspired by the blocks implementation. We should probably bring this into alignment, though I don't think it ought to block the work here (cc @iseulde).
gutenberg/packages/rich-text/src/register-format-type.js
Lines 118 to 123 in a615ce6
As @gziolo noted, there are merge conflicts to resolve, but otherwise the changes look good to me.
|
I opened #13848 to resolve merge conflicts and merge this PR into master. @jameelmoses thanks for your contribution 🎉 |
Description
Fixes: #11949
Instead of failing to register a block if more than 3 keywords are given, only return the first 3 by slicing the keywords array.
How has this been tested?
npm run lintTypes of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: