fix(OverflowMenu): check for custom closeMenu method #7456
Conversation
|
❌ Deploy preview for carbon-components-react failed. 🔨 Explore the source changes: 15d7140 🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-components-react/deploys/60076b24f555ba00077a4ad0 |
|
✔️ Deploy preview for carbon-elements ready! 🔨 Explore the source changes: 15d7140 🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-elements/deploys/60076b24893dce0007061eab 😎 Browse the preview: https://deploy-preview-7456--carbon-elements.netlify.app |
| @@ -494,7 +494,7 @@ class OverflowMenu extends Component { | |||
| const childrenWithProps = React.Children.toArray(children).map( | |||
| (child, index) => | |||
| React.cloneElement(child, { | |||
| closeMenu: this.closeMenu, | |||
| closeMenu: child.props.closeMenu || this.closeMenu, | |||
There was a problem hiding this comment.
Would we need to run both functions instead of deferring to a given child closeMenu? Seems like this.closeMenu has some state logic that we want to run but I might be misunderstanding 👀
There was a problem hiding this comment.
the main intention is to allow a function that is different from or replaces the default closeMenu (without expanding the component API). if the user still wants to run the existing logic they can use the existing onClose prop. maybe it would be more preferable to just add a boolean prop that controls whether or not the menu will close on selection of an item?
Closes #6412
This PR prevents custom
closeMenumethods from being overwritten by the default method in order to support a wider range of interactions on the overflow menuChangelog
Changed
closeMenubefore cloning the element and assigning a default value for the functionTesting / Reviewing
confirm that adding a custom
closeMenumethod on anOverflowMenuItemwill not be overwritten