[WB-2152] ActionMenu with custom opener not triggering focused prop change #2864
Conversation
…opagate onBlur event correctly. This change allows to update the focused styles when the opener is blurred.
🦋 Changeset detectedLatest commit: 0765115 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
| onBlur: onBlur | ||
| ? (e: React.FocusEvent) => { | ||
| // This is done to avoid overriding a custom onBlur | ||
| // handler inside the children node | ||
| onBlur(e); | ||
| clickableChildrenProps.onBlur(e); | ||
| } | ||
| : clickableChildrenProps.onBlur, |
There was a problem hiding this comment.
note: This is the actual fix. The rest of the changes in this file is to convert the component into a function one.
There was a problem hiding this comment.
Wow, that's a surprising find! It did not occur to me that onBlur could be the culprit.
|
Size Change: +1 B (0%) Total Size: 109 kB
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (9afbdb9) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR2864"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR2864 |
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-hkbwwtamxc.chromatic.com/ Chromatic results:
|
marcysutton
left a comment
marcysutton
left a comment
There was a problem hiding this comment.
Thanks for getting a fix together so quickly! I tested the stories you mentioned in the PR description manually and they still work. The styling is also improved on the ActionMenu story to only show one focus outline!
I wonder if we could add any test coverage or snapshots to verify the behavior in case it breaks sometime in the future?
| onBlur: onBlur | ||
| ? (e: React.FocusEvent) => { | ||
| // This is done to avoid overriding a custom onBlur | ||
| // handler inside the children node | ||
| onBlur(e); | ||
| clickableChildrenProps.onBlur(e); | ||
| } | ||
| : clickableChildrenProps.onBlur, |
There was a problem hiding this comment.
Wow, that's a surprising find! It did not occur to me that onBlur could be the culprit.
Summary:
DropdownOpenerin a function component.DropdownOpenerto propagate the onBlur event correctly. This changeallows to update the focused styles when the opener is blurred.
Issue: https://khanacademy.atlassian.net/browse/WB-2152
Test plan:
/?path=/story/packages-dropdown-actionmenu--custom-openertabuntil the focus is set on the opener.EnterorSpaceto open the menu.Escapeto close it.tabagain and ensure that the focus styles are removed from the opener.BONUS: Verify that the
SingleSelectandMultiSelectwith custom openerstories continue working as expected.
/?path=/story/packages-dropdown-singleselect--custom-opener/?path=/story/packages-dropdown-multiselect--custom-opener