Menu: throw when subcomponents are not rendered inside top level Menu #67411
Conversation
ciampo
added
[Type] Enhancement
|
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. |
ciampo
force-pushed
the
feat/menu-add-runtime-check
branch
from
bb3bf84 to
df87182
Compare
|
Flaky tests detected in 5fb1df2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12081605771
|
| throw new Error( | ||
| 'Menu.CheckboxItem can only be rendered inside a Menu component' | ||
| ); |
There was a problem hiding this comment.
Just for context to anyone else reading this: throwing an error in this scenario makes sense even if we just look at Ariakit - it is consistent with how Ariakit does it:
As you can see, if we don't have this check, Ariakit will throw this error:
MenuItem must be wrapped in a MenuList, Menu or Menubar component
however, relying on this would be confusing, since our Menu components don't necessarily follow the same naming scheme.
packages/components/CHANGELOG.md
Outdated
|
|
||
| ### Internal | ||
|
|
There was a problem hiding this comment.
No need to add yet another "Internal" section.
| ### Internal |
There was a problem hiding this comment.
My bad, I was meant to rename it to "Experimental" and I forgot to do it. Will amend and merge
…WordPress#67411) * Menu: throw when subcomponents are not rendered inside top level Menu * CHANGELOG * Remove unnecessary optional chaining * Rename changelog section --- Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]>
…#67411) * Menu: throw when subcomponents are not rendered inside top level Menu * CHANGELOG * Remove unnecessary optional chaining * Rename changelog section --- Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]>
What?
Part of #66530
Extracted from #66383
Add a check for all
Menusubcomponents, so that they throw if not rendered inside a parentMenucomponent.Why?
This check ensures that the
Menucompound component is used as intended (see #66530)How?
We perform a check on the internal component's context (which is created and exposed in the top level
Menucomponent)Testing Instructions
Try to render any subcomponents in isolation (ie. not inside a
Menucomponent) — the component should throw an error.