Allow using groups and columns inside the experimental form block #55758
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aristath
added
[Type] Enhancement
aristath
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
aristath
changed the title
aristath
requested review from
draganescu
and removed request for
juanmaguitar,
ajitbohra,
ryanwelcher,
ndiego and
fabiankaegy
fabiankaegy
approved these changes
Nov 1, 2023
|
Size Change: +210 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
aristath
force-pushed
the
add/groups-in-forms
branch
from
d8ffcb8
to
413ec7e
Compare
|
Let me stop auto-merge. Wouldn't this PR allow us to put the Form block inside of the Form block? I have not tested this PR, so sorry if I am mistaken. Form > Group > Form My understanding is that nesting of form elements should not be allowed. |
|
@t-hamano you're absolutely right... Thank you for catching that! 👍 |
|
Since there may be cases where multiple forms are inserted on a page, simply setting |
|
@t-hamano maybe we should have a gutenberg/packages/block-editor/src/store/selectors.js Lines 1385 to 1399 in 76ce7e0 canInsert const below that snippet 🤔
EDIT: Or maybe we could define in |
|
@gziolo and I have long been talking about more strict rules for the block API in the block API tracking issue: #41236 So from my perspective that issue is not new and has little to do with the update here. I don't think it should prevent moving this forward but would of course love a solution for it :) |
|
Flaky tests detected in b977750. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6719428592
|
|
What about using the This code is an example. |
|
The simplest change I can think of, is to allow values like |
Huh... Good call! Thank you @t-hamano! Pushed a commit implementing that filter. |
t-hamano
reviewed
Nov 1, 2023
There was a problem hiding this comment.
In my testing, nesting forms appear to work correctly in all cases 👍
- Root > ✅ Form
- Root > Group > ✅ Form
- Root > Columns > Column > ✅ Form
- Root > Form > Group > ❌ Form
- Root > Form > Columns > Column > ❌ Form
As for filters, I think it's better to run them inside the init() function.
export const init = () => {
// Prevent adding forms inside forms.
// ...
return initBlock( { name, metadata, settings } );
};
Makes sense... Done. |
I see the same filter allows removing Post Content and Template Part blocks from the post editor. It also prevents adding a Template Part block inside the Query Loop and inside the Post Content block which plays a similar role. I guess it's fine to reuse it here as well.
I agree it shouldn't be a blocker, but it looks like there is a working solution in place. The remaining question is how to offer a general solution for the same problem. There are many way how it can be approached, and the filter used would be one of them if we consider it an edge case. We could also offer a way to list disallowed parents/ancestors separately, or in this particular case offer something like |
Yeah also you can pretty much always circumvent it via a synced pattern |
|
Since the concerns for this PR were addressed, I went ahead and merged it. |
draganescu
reviewed
Nov 2, 2023
| const DISALLOWED_PARENTS = [ 'core/form' ]; | ||
| addFilter( | ||
| 'blockEditor.__unstableCanInsertBlockType', | ||
| 'removeTemplatePartsFromPostTemplates', |
There was a problem hiding this comment.
What do template parts and post templates have to do with this? :)
There was a problem hiding this comment.
Good catch, this namespace would need to be changed to something like removeFormFromInserter 😅
There was a problem hiding this comment.
Yes, something like core/block-library/preventInsertingFormIntoAnotherForm would fit better. I don't think we have ever established a good pattern for naming filters.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Allows adding columns and groups inside a form block.
This will allow us to build some more complex forms, with different layouts etc.
Why?
Because right now, forms only allow input fields one below the other, which limits our ability to build beautiful things.
How?
parentto usingancestorinblock.jsonfiles for input blockscore/columnsandcore/groupblocks in the allowed blocks for formsTesting Instructions
Testing Instructions for Keyboard
Not applicable