[Menu] add checks to prevent error for null child #2429
Conversation
|
I'm not sure. Should we also filter before mapping at this line, to make the count equal to number of non-null children? @oliviertassinari What do you think? |
|
Yeap, I think that it's a good idea to create a filtered children array. |
oliviertassinari
added
the
PR: needs revision
|
@oliviertassinari Please take another look. |
alitaheri
added
Review: review-needed
new feature
| let menuItemCount = 0; | ||
| React.Children.forEach(this.props.children, (child) => { | ||
| for (let child of filteredChildren) { |
There was a problem hiding this comment.
for ... of ... is not supported by IE (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of)
Shouldn't we use forEach?
There was a problem hiding this comment.
babel transforms this. see the emitted code
but I can change it if you want. I just thought it's more expressive.
There was a problem hiding this comment.
Hum, I didn't check, but I remember this issue #1972.
There was a problem hiding this comment.
In my output I see the transformation, but let's not take any chances. Alright I'll Fix this. Thanks for the tip 👍 👍
oliviertassinari
added
PR: needs revision
|
@oliviertassinari This is ready. if it's Ok I'll rebase and merge. 😬 |
alitaheri
added
PR: Needs Review
and removed
new feature
|
@subjectix Yes, could you squash down? |
alitaheri
force-pushed
the
menu-handle-null-child
branch
from
25ddd3b to
8d5ce9f
Compare
[Menu] add checks to prevent error for null child
mui#2429 allowed for null children in Menu, however, DropDownMenu threw an exception at Line 272 when child was null when doing a check for a matching value to display.
zannager
added
the
component: menu
Closes #2067.