Material-ui: [Grid] Replace px to rem in spacing prop

[udanpe]

Now

<Grid spacing={8/padding: 4px|16/padding: 8px|32/padding: 16px|40/padding: 32px}>

Need

<Grid spacing={8/padding:0.25rem|16/padding:0.5rem|32/padding:0.75rem|40/padding:1rem}>

or add another prop spacingRem

[oliviertassinari]

@udanpe The Grid component aims to be as simple as possible. You don't have to use the spacing property, you can keep it to 0 and use rem on your side.

[timbakkum]

I changed the spacing function to output a rem string as per the 'bootstrap strategy' example(https://material-ui.com/customization/spacing/). However, this breaks the spacing value on Grid completely as it is trying to divide a string. Maybe it's wise to warn any users that changing the theme spacing function might "break" things?

[oliviertassinari]

@Und3Rdo9 Oh, that's a bug! We could imagine the following change:

diff --git a/packages/material-ui/src/Grid/Grid.js b/packages/material-ui/src/Grid/Grid.js
index 9fa4d375f..c59765def 100644
--- a/packages/material-ui/src/Grid/Grid.js
+++ b/packages/material-ui/src/Grid/Grid.js
@@ -74,11 +74,16 @@ function generateGutter(theme, breakpoint) {
       return;
     }

+    const half =
+      typeof themeSpacing === 'number'
+        ? themeSpacing / 2
+        : themeSpacing.replace(/(\d+\.\d+)/, (str, p1) => p1 / 2);
+
     styles[`spacing-${breakpoint}-${spacing}`] = {
-      margin: -themeSpacing / 2,
+      margin: -half,
       width: `calc(100% + ${themeSpacing}px)`,
       '& > $item': {
-        padding: themeSpacing / 2,
+        padding: half,
       },
     };
   });

[timbakkum]

Yeah, something like that looks like it would do the trick. I didn't realise that you'd consider this a bug as you said you wanted the Grid to be as simple as possible in your previous comment 😅 Should this be reopened?

[oliviertassinari]

@Und3Rdo9 It looks like that I have changed my mind a bit since last year :)