[Table] Extended labelRowsPerPage support from string to ReactNode

[romanyanke]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Here you allow to use labelRowsPerPage as React.Node https://github.com/mui-org/material-ui/blob/27471b4564eb40ff769352d73a29938d25804e45/packages/material-ui/src/TablePagination/TablePagination.d.ts#L23

Here you use it as a string https://github.com/mui-org/material-ui/blob/c253b2813822239672b7a2a589324e5e00b97820/packages/material-ui/src/TablePagination/TablePagination.js#L125

So this is vaild but will generate en error "Warning: Failed prop type: Invalid prop aria-label of type object supplied to ForwardRef(SelectInput), expected string."

<TablePagination
  labelRowsPerPage={<b>I will warn</b>}

Expected Behavior 🤔

I still want to pass a React.Node without a warning

Your Environment 🌎

Tech	Version
Material-UI	v4.9.9
React	16.13.1
Browser	*
TypeScript	3.8.3
etc.

[oliviertassinari]

@romanyanke Thanks for mentioning this inconsistency. Would the following solve the problem?

diff --git a/packages/material-ui/src/TablePagination/TablePagination.js b/packages/material-ui/src/TablePagination/TablePagination.js
index 8ab941190..9c7a0302b 100644
--- a/packages/material-ui/src/TablePagination/TablePagination.js
+++ b/packages/material-ui/src/TablePagination/TablePagination.js
@@ -10,6 +10,7 @@ import TableCell from '../TableCell';
 import Toolbar from '../Toolbar';
 import Typography from '../Typography';
 import TablePaginationActions from './TablePaginationActions';
+import useId from '../utils/unstable_useId';

 export const styles = (theme) => ({
   /* Styles applied to the root element. */
@@ -102,6 +103,7 @@ const TablePagination = React.forwardRef(function TablePagination(props, ref) {
     colSpan = colSpanProp || 1000; // col-span over everything
   }

+  const labelId = useId();
   const MenuItemComponent = SelectProps.native ? 'option' : MenuItem;

   return (
@@ -109,7 +111,7 @@ const TablePagination = React.forwardRef(function TablePagination(props, ref) {
       <Toolbar className={classes.toolbar}>
         <div className={classes.spacer} />
         {rowsPerPageOptions.length > 1 && (
-          <Typography color="inherit" variant="body2" className={classes.caption}>
+          <Typography color="inherit" variant="body2" className={classes.caption} id={labelId}>
             {labelRowsPerPage}
           </Typography>
         )}
@@ -122,7 +124,7 @@ const TablePagination = React.forwardRef(function TablePagination(props, ref) {
             input={<InputBase className={clsx(classes.input, classes.selectRoot)} />}
             value={rowsPerPage}
             onChange={onChangeRowsPerPage}
-            inputProps={{ 'aria-label': labelRowsPerPage }}
+            labelId={labelId}
             {...SelectProps}
           >
             {rowsPerPageOptions.map((rowsPerPageOption) => (

Do you want to submit a pull request? :)

[eps1lon]

I'm generally in favor of referencing the label instead of duplicating it.

However, this particular use case can be solved by separating markup and styles:

const useStyles = makeStyles({
  caption: {
    fontWeight: "bold"
  }
});

export default function Demo() {
  const classes = useStyles();
  return <TablePagination classes={classes} labelRowsPerPage="I will warn" />;
}

[oliviertassinari]

@eps1lon A couple of internationalization libraries are implemented with a component, for instance, react-intl's <FormattedMessage /> or react-18next's <Trans />. Allowing a node would help for these cases. @romanyanke What's your use case for using a React.ReactNode over a string?

[eps1lon]

Sure. That's why I'm generally in favor. react-intl has useIntl().formatMessage for usage in attributes.

[romanyanke]

@oliviertassinari react-intl's is exactly my case.
I'm using material-ui with typescript a lot and there are props that requires only string. The other props requires ReactNode. In my opinion ReactNode is much greater in terms of good API.

[AlexAndriyanenko]

Is this still open, can i work on this issue?

[AlexAndriyanenko]

I can see that in #20685 pull request ReactNode was changed to string type. Kind of disappointed what exactly should be done. It should support both string and ReactNode types?

[oliviertassinari]

@AlexAndriyanenko We would love to receive your contribution to this problem :). Regarding #20685, the definition was updated to match the implementation. We would need to apply the opposite diff to solve this issue.