Components: Add allowForms prop to SandBox component - #76471
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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.
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. |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
I think I'd like to understand why the existing |
That is a good point. The prop idea came from this comment in the issue, though reading it again I'm not sure if by "it" @talldan meant the "allow-forms" specifically, or all values. |
|
Seems safe:
Issues / suggestions
The implementation seems correct and safe and tests well — it's a small, well-scoped, backwards-compatible addition. The main open question is what @aduth raised: prop vs. default. That's a project-level API design decision, not an actual issue. The trailing whitespace is a minor nit. |
|
Is this still needed/relevant? We've recently added "allow popups" prop so there's pre-existing pattern. |
|
Thanks for flagging @simison . It feels like there's parallel conversations happening between here and #69617, and that both pull requests are basically doing the same thing for different aspects of the sandbox. I don't feel like it was ever really settled whether these make sense as defaults or if we should just open up the entire cc @Mamaduka @obenland @Imran92 @jsnajdr @mirka as having participated in that other pull request. |
|
Thanks for the ping. This would require rebasing after #69617. Now it's worth deciding on the format we plan to support new |
|
Adding another Both That raises a question: should we enable these capabilities by default? The |
What?
Closes #48942
Why?
Currently there is no way to allow forms in the Sandbox component, so in cases where that is necessary, like submitting your date of birth to view age-restricted content, the submission is blocked.
How?
By adding an
allowFormsprop to the SandBox component that includesallow-formsto the iframe, making it possible to allow it selectively.Testing Instructions
Storybook can be used for that.
1- Run storybook with
npm run storybook:dev2 - Go to Components > Utilities > SandBox > With Form
3 - Check that you can submit the form
4 - Disable allowForms and reload
5 - Check that you can't submit the form
Testing Instructions for Keyboard
Screenshots or screencast
Use of AI Tools
Claude Code was used to generate tests.