Support controlling open/closed state for Dropdown and DropdownMenu#54257
Support controlling open/closed state for Dropdown and DropdownMenu#54257
Conversation
|
Unit tests are broken, so we;d need to look into that and see if this approach is still feasible without introducing breaking changes. |
| useEffect( | ||
| () => () => { | ||
| if ( onToggle && isOpen ) { | ||
| onToggle( false ); | ||
| } | ||
| }, | ||
| [ onToggle, isOpen ] | ||
| ); |
There was a problem hiding this comment.
Nice one, this is the removal that I was missing from my experiment 🎉
There was a problem hiding this comment.
Nice one, this PR unblocks #54083 for me!
Unit tests are broken, so we;d need to look into that and see if this approach is still feasible without introducing breaking changes.
I've pulled in most of the changes here into #54083 where the unit tests are passing. Are they failing in this PR because of the temporarily commented out close() call in closeIfFocusOutside()?
Edit: the unit tests are passing over in #54083, but the e2es are failing, so there's likely still a bit to fix up. Still, I'm feeling confident about the direction, now!
Edit 2: I think I found the source of the e2e failures on the other PR #54083 — turns out I needed to explicitly clear the state in a couple of places, and in one e2e wait for the menu to be open before firing a keyboard shortcut. So, all up, looking pretty good so far 🙂
|
Cool! In that case, we can close this PR as it served its purpose :) |
|
Just seen #54083 (comment) — I'm going to reopen, so that we can use this PR to make changes to the components and keep the rest of the change sin #54083 |
15b31be to
6ea9b0a
Compare
6ea9b0a to
ab6e115
Compare
packages/components/CHANGELOG.md
Outdated
There was a problem hiding this comment.
Moved this entry to the previous release section, since I realised that the corresponding PR was merged before August 31st
andrewserong
left a comment
There was a problem hiding this comment.
Thanks again for all the work here, this looks great to me!
✅ The structure of the new controlled props matches what I need to get #54083 working correctly
✅ Code looks good, and documentation changes read well to me
✅ Smoke tested the Dropdown and DropdownMenu components in Storybook and across usage in the post and site editors
LGTM! ✨
What?
Add support to control the open state of
DropdownandDropdownMenucomponentsWhy?
To give more choice to consumers of the components on how to use such components (see for example #54083)
How?
By adding
open/onToggle/defaultOpenprops, using theuseControlledValuehook and refactoring as needed.Testing Instructions
DropdownandDropdownMenuin controlled mode (see d8379d9 and 2d35f36 to see the required changes)npm run storybook:dev) and make sure that the components continue to work as expected✍️ Dev note
The open/closed state of the
DropdownandDropdownMenucomponents can now be controlled by their consumers via theopen,onToggleanddefaultOpenprops, allowing more flexibility and more advanced behaviors when using these components.