Skip to content

Commit

Permalink
[Grid] Fix columnSpacing and rowSpacing props ignore higher break…
Browse files Browse the repository at this point in the history
…points with 0 (#33480)

* add test cases

* add logic

* improve logic
  • Loading branch information
ZeeshanTamboli committed Jul 26, 2022
1 parent 4507574 commit 904410d
Show file tree
Hide file tree
Showing 2 changed files with 312 additions and 4 deletions.
68 changes: 64 additions & 4 deletions packages/mui-material/src/Grid/Grid.js
Expand Up @@ -126,6 +126,31 @@ export function generateDirection({ theme, ownerState }) {
});
}

/**
* Extracts zero value breakpoint keys before a non-zero value breakpoint key.
* @example { xs: 0, sm: 0, md: 2, lg: 0, xl: 0 } or [0, 0, 2, 0, 0]
* @returns [xs, sm]
*/
function extractZeroValueBreakpointKeys({ breakpoints, values }) {
let nonZeroKey = '';

Object.keys(values).forEach((key) => {
if (nonZeroKey !== '') {
return;
}

if (values[key] !== 0) {
nonZeroKey = key;
}
});

const sortedBreakpointKeysByValue = Object.keys(breakpoints).sort((a, b) => {
return breakpoints[a] - breakpoints[b];
});

return sortedBreakpointKeysByValue.slice(0, sortedBreakpointKeysByValue.indexOf(nonZeroKey));
}

export function generateRowGap({ theme, ownerState }) {
const { container, rowSpacing } = ownerState;
let styles = {};
Expand All @@ -136,7 +161,15 @@ export function generateRowGap({ theme, ownerState }) {
breakpoints: theme.breakpoints.values,
});

styles = handleBreakpoints({ theme }, rowSpacingValues, (propValue) => {
let zeroValueBreakpointKeys;
if (typeof rowSpacingValues === 'object') {
zeroValueBreakpointKeys = extractZeroValueBreakpointKeys({
breakpoints: theme.breakpoints.values,
values: rowSpacingValues,
});
}

styles = handleBreakpoints({ theme }, rowSpacingValues, (propValue, breakpoint) => {
const themeSpacing = theme.spacing(propValue);

if (themeSpacing !== '0px') {
Expand All @@ -148,7 +181,16 @@ export function generateRowGap({ theme, ownerState }) {
};
}

return {};
if (zeroValueBreakpointKeys.includes(breakpoint)) {
return {};
}

return {
marginTop: 0,
[`& > .${gridClasses.item}`]: {
paddingTop: 0,
},
};
});
}

Expand All @@ -165,7 +207,15 @@ export function generateColumnGap({ theme, ownerState }) {
breakpoints: theme.breakpoints.values,
});

styles = handleBreakpoints({ theme }, columnSpacingValues, (propValue) => {
let zeroValueBreakpointKeys;
if (typeof columnSpacingValues === 'object') {
zeroValueBreakpointKeys = extractZeroValueBreakpointKeys({
breakpoints: theme.breakpoints.values,
values: columnSpacingValues,
});
}

styles = handleBreakpoints({ theme }, columnSpacingValues, (propValue, breakpoint) => {
const themeSpacing = theme.spacing(propValue);
if (themeSpacing !== '0px') {
return {
Expand All @@ -177,7 +227,17 @@ export function generateColumnGap({ theme, ownerState }) {
};
}

return {};
if (zeroValueBreakpointKeys.includes(breakpoint)) {
return {};
}

return {
width: '100%',
marginLeft: 0,
[`& > .${gridClasses.item}`]: {
paddingLeft: 0,
},
};
});
}

Expand Down
248 changes: 248 additions & 0 deletions packages/mui-material/src/Grid/Grid.test.js
Expand Up @@ -689,7 +689,9 @@ describe('Material UI <Grid />', () => {
'Warning: Failed prop type: The prop `spacing` of `Grid` can only be used together with the `container` prop.',
);
});
});

describe('prop: rowSpacing, columnSpacing', () => {
it('should generate correct responsive styles', () => {
const theme = createTheme();
expect(
Expand Down Expand Up @@ -1049,6 +1051,252 @@ describe('Material UI <Grid />', () => {
},
});
});

it('should generate correct responsive styles for overriding with zero value styles for higher breakpoints', () => {
const theme = createTheme({
breakpoints: {
values: {
mobile: 0,
desktop: 1200,
tablet: 640,
},
},
});
expect(
generateRowGap({
ownerState: {
container: true,
rowSpacing: { mobile: 1.5, desktop: 0, tablet: 0 },
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {
'& > .MuiGrid-item': {
paddingTop: '12px',
},
marginTop: '-12px',
},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingTop: 0,
},
marginTop: 0,
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingTop: 0,
},
marginTop: 0,
},
});

expect(
generateColumnGap({
ownerState: {
container: true,
columnSpacing: { mobile: 1.5, desktop: 0, tablet: 0 },
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {
'& > .MuiGrid-item': {
paddingLeft: '12px',
},
marginLeft: '-12px',
width: 'calc(100% + 12px)',
},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingLeft: 0,
},
marginLeft: 0,
width: '100%',
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingLeft: 0,
},
marginLeft: 0,
width: '100%',
},
});

// Array input
expect(
generateRowGap({
ownerState: {
container: true,
rowSpacing: [1.5, 0, 0],
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {
'& > .MuiGrid-item': {
paddingTop: '12px',
},
marginTop: '-12px',
},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingTop: 0,
},
marginTop: 0,
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingTop: 0,
},
marginTop: 0,
},
});

expect(
generateColumnGap({
ownerState: {
container: true,
columnSpacing: [1.5, 0, 0],
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {
'& > .MuiGrid-item': {
paddingLeft: '12px',
},
marginLeft: '-12px',
width: 'calc(100% + 12px)',
},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingLeft: 0,
},
marginLeft: 0,
width: '100%',
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingLeft: 0,
},
marginLeft: 0,
width: '100%',
},
});
});

it('should not generate responsive styles for lower breakpoints below a given non-zero breakpoint', () => {
const theme = createTheme({
breakpoints: {
values: {
mobile: 0,
desktop: 1200,
tablet: 640,
},
},
});
expect(
generateRowGap({
ownerState: {
container: true,
rowSpacing: { mobile: 0, desktop: 0, tablet: 1.5 },
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingTop: '12px',
},
marginTop: '-12px',
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingTop: 0,
},
marginTop: 0,
},
});

expect(
generateColumnGap({
ownerState: {
container: true,
columnSpacing: { mobile: 0, desktop: 0, tablet: 1.5 },
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingLeft: '12px',
},
marginLeft: '-12px',
width: 'calc(100% + 12px)',
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingLeft: 0,
},
marginLeft: 0,
width: '100%',
},
});

// Array input
expect(
generateRowGap({
ownerState: {
container: true,
rowSpacing: [0, 1.5, 0],
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingTop: '12px',
},
marginTop: '-12px',
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingTop: 0,
},
marginTop: 0,
},
});

expect(
generateColumnGap({
ownerState: {
container: true,
columnSpacing: [0, 1.5, 0],
},
theme,
}),
).to.deep.equal({
'@media (min-width:0px)': {},
'@media (min-width:640px)': {
'& > .MuiGrid-item': {
paddingLeft: '12px',
},
marginLeft: '-12px',
width: 'calc(100% + 12px)',
},
'@media (min-width:1200px)': {
'& > .MuiGrid-item': {
paddingLeft: 0,
},
marginLeft: 0,
width: '100%',
},
});
});
});

describe('prop: columns', () => {
Expand Down

0 comments on commit 904410d

Please sign in to comment.