-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Table] Fix TablePagination duplicate key error #18988
Conversation
Details of bundle changes.Comparing: ae837c9...f601639
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice finding :). I'm curious, what's your use case for a different label?
@afzalsayed96 It's a great first pull request on Material-UI 👌🏻. Thank you for working on it! |
@oliviertassinari That was fast! Thank you for merging in. My use case was same as #17885 i.e. wanted to show all rows and provide an option for it. I don't think this feature is documented in docs. So I had to go through the PR to find about it's usage. |
@afzalsayed96 Oh ok, I wasn't expecting a warning in this configuration. |
Oh that's perfect! I somehow missed that. This could be further cleaned up by the following syntax if you choose to upgrade to {rowsPerPageOptions.map(rowsPerPageOption => (
<MenuItemComponent
className={classes.menuItem}
- key={rowsPerPageOption.value ? rowsPerPageOption.value : rowsPerPageOption}
+ key={rowsPerPageOption.value ?? rowsPerPageOption}
- value={rowsPerPageOption.value ? rowsPerPageOption.value : rowsPerPageOption}
+ value={rowsPerPageOption.value ?? rowsPerPageOption}
>
- {rowsPerPageOption.label ? rowsPerPageOption.label : rowsPerPageOption}
+ {rowsPerPageOption.value ?? rowsPerPageOption}
</MenuItemComponent>
))} |
We will probably wait for the syntax to lands in the ECMA-262, thanks for sharing. |
Refer: #17885 (comment)