Skip to content
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

Fixing grid spacing issue #24299

Closed
wants to merge 8 commits into from
5 changes: 5 additions & 0 deletions packages/material-ui/src/Grid/Grid.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ export interface GridTypeMap<P = {}, D extends React.ElementType = 'div'> {
* You should be wrapping *items* with a *container*.
* @default false
*/
/**
* If `true`, items are full-width, independent of whether or not they have spacing.
* @default false
*/
fullWidth?: boolean;
item?: boolean;
/**
* Defines the `justify-content` style property.
Expand Down
146 changes: 106 additions & 40 deletions packages/material-ui/src/Grid/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,60 @@ import requirePropFactory from '../utils/requirePropFactory';
const SPACINGS = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
const GRID_SIZES = ['auto', true, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];

function generateGrid(globalStyles, theme, breakpoint) {
function getOffset(val, div = 1) {
const parse = parseFloat(val);
return `${parse / div}${String(val).replace(String(parse), '') || 'px'}`;
}

function generateGrid(globalStyles, theme, breakpoint, fullWidth) {
const styles = {};

GRID_SIZES.forEach((size) => {
const key = `grid-${breakpoint}-${size}`;
const spacings = fullWidth ? SPACINGS.slice(1) : [undefined];

if (size === true) {
// For the auto layouting
styles[key] = {
flexBasis: 0,
flexGrow: 1,
maxWidth: '100%',
};
spacings.forEach((spacing) => {
const key = `grid-${breakpoint}-${size}${
fullWidth && spacing !== undefined ? `-fullWidth-spacing-${spacing}` : ''
}`;

return;
}
if (size === true) {
// For the auto layouting
styles[key] = {
flexBasis: 0,
flexGrow: 1,
maxWidth: '100%',
};

if (size === 'auto') {
styles[key] = {
flexBasis: 'auto',
flexGrow: 0,
maxWidth: 'none',
};
return;
}

return;
}
if (size === 'auto') {
styles[key] = {
flexBasis: 'auto',
flexGrow: 0,
maxWidth: 'none',
};

// Keep 7 significant numbers.
const width = `${Math.round((size / 12) * 10e7) / 10e5}%`;
return;
}

// Close to the bootstrap implementation:
// https://github.com/twbs/bootstrap/blob/8fccaa2439e97ec72a4b7dc42ccc1f649790adb0/scss/mixins/_grid.scss#L41
styles[key] = {
flexBasis: width,
flexGrow: 0,
maxWidth: width,
};
// Keep 7 significant numbers.
let width = `${Math.round((size / 12) * 10e7) / 10e5}%`;

if (fullWidth && spacing > 0) {
const themeSpacing = theme.spacing(spacing);

width = `calc(${width} + ${getOffset(themeSpacing)})`;
}

// Close to the bootstrap implementation:
// https://github.com/twbs/bootstrap/blob/8fccaa2439e97ec72a4b7dc42ccc1f649790adb0/scss/mixins/_grid.scss#L41
styles[key] = {
flexBasis: width,
flexGrow: 0,
maxWidth: width,
};
});
});

// No need for a media query for the first size.
Expand All @@ -65,11 +82,6 @@ function generateGrid(globalStyles, theme, breakpoint) {
}
}

function getOffset(val, div = 1) {
const parse = parseFloat(val);
return `${parse / div}${String(val).replace(String(parse), '') || 'px'}`;
}

function generateGutter(theme, breakpoint) {
const styles = {};

Expand All @@ -81,17 +93,28 @@ function generateGutter(theme, breakpoint) {
}

styles[`spacing-${breakpoint}-${spacing}`] = {
margin: `-${getOffset(themeSpacing, 2)}`,
marginTop: `-${getOffset(themeSpacing)}`,
marginLeft: `-${getOffset(themeSpacing)}`,
width: `calc(100% + ${getOffset(themeSpacing)})`,
'& > $item': {
padding: getOffset(themeSpacing, 2),
paddingTop: getOffset(themeSpacing),
paddingLeft: getOffset(themeSpacing),
},
};
});

return styles;
}

function removeKeys(obj, filter) {
const newObj = {};
Object.assign(newObj, obj);
for (let i = 0; i < filter.length; i += 1) {
delete newObj[filter[i]];
}
return newObj;
}

// Default CSS values
// flex: '0 1 auto',
// flexDirection: 'row',
Expand Down Expand Up @@ -200,11 +223,37 @@ export const styles = (theme) => ({
justifyContent: 'space-evenly',
},
...generateGutter(theme, 'xs'),
...theme.breakpoints.keys.reduce((accumulator, key) => {
// Use side effect over immutability for better performance.
generateGrid(accumulator, theme, key);
return accumulator;
}, {}),
...(() => {
const finalStyles = {};

const mediaQueries = theme.breakpoints.keys.map((breakpoint) =>
theme.breakpoints.up(breakpoint),
);

const stylesWithoutFullWidth = theme.breakpoints.keys.reduce((accumulator, key) => {
// Use side effect over immutability for better performance.
generateGrid(accumulator, theme, key);
return accumulator;
}, {});
const stylesFullWidth = theme.breakpoints.keys.reduce((accumulator, key) => {
// Use side effect over immutability for better performance.
generateGrid(accumulator, theme, key, true);
return accumulator;
}, {});
Object.assign(finalStyles, {
...removeKeys(stylesWithoutFullWidth, mediaQueries),
...removeKeys(stylesFullWidth, mediaQueries),
});
mediaQueries.forEach((query) => {
Object.assign(finalStyles, {
[query]: {
...stylesWithoutFullWidth[query],
...stylesFullWidth[query],
},
});
});
return finalStyles;
})(),
});

const Grid = React.forwardRef(function Grid(props, ref) {
Expand All @@ -226,6 +275,7 @@ const Grid = React.forwardRef(function Grid(props, ref) {
xl = false,
xs = false,
zeroMinWidth = false,
fullWidth = false,
...other
} = props;

Expand All @@ -246,6 +296,16 @@ const Grid = React.forwardRef(function Grid(props, ref) {
[classes[`grid-md-${String(md)}`]]: md !== false,
[classes[`grid-lg-${String(lg)}`]]: lg !== false,
[classes[`grid-xl-${String(xl)}`]]: xl !== false,
[classes[`grid-xs-${String(xs)}-fullWidth-spacing-${String(spacing)}`]]:
container && fullWidth && spacing !== 0 && xs !== false,
[classes[`grid-sm-${String(sm)}-fullWidth-spacing-${String(spacing)}`]]:
container && fullWidth && spacing !== 0 && sm !== false,
[classes[`grid-md-${String(md)}-fullWidth-spacing-${String(spacing)}`]]:
container && fullWidth && spacing !== 0 && md !== false,
[classes[`grid-lg-${String(lg)}-fullWidth-spacing-${String(spacing)}`]]:
container && fullWidth && spacing !== 0 && lg !== false,
[classes[`grid-xl-${String(xl)}-fullWidth-spacing-${String(spacing)}`]]:
container && fullWidth && spacing !== 0 && xl !== false,
},
classNameProp,
);
Expand Down Expand Up @@ -306,6 +366,11 @@ Grid.propTypes = {
* @default 'row'
*/
direction: PropTypes.oneOf(['column-reverse', 'column', 'row-reverse', 'row']),
/**
* If `true`, items are full-width, independent of whether or not they have spacing.
* @default false
*/
fullWidth: PropTypes.bool,
/**
* If `true`, the component will have the flex *item* behavior.
* You should be wrapping *items* with a *container*.
Expand Down Expand Up @@ -399,6 +464,7 @@ if (process.env.NODE_ENV !== 'production') {
alignContent: requireProp('container'),
alignItems: requireProp('container'),
direction: requireProp('container'),
fullWidth: requireProp('container'),
justifyContent: requireProp('container'),
lg: requireProp('item'),
md: requireProp('item'),
Expand Down