Skip to content

Commit

Permalink
fix(grid): vertical grid should use gutter (#1685)
Browse files Browse the repository at this point in the history
  • Loading branch information
@andioneto
andioneto committed Aug 4, 2021
1 parent b6706e1 commit b02b0d6
Show file tree
Hide file tree
Showing 7 changed files with 337 additions and 49 deletions.
6 changes: 6 additions & 0 deletions .changeset/seven-baboons-report.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@twilio-paste/grid': patch
'@twilio-paste/core': patch
---

[Grid] Fix to apply gutter value to top/bottom padding for vertical grid.
44 changes: 30 additions & 14 deletions packages/paste-core/layout/grid/__tests__/__snapshots__/grid.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ exports[`Grid render should render a Column 1`] = `
<DocumentFragment>
.emotion-0 {
width: 8.333333333333332%;
padding-top: space0;
padding-bottom: space0;
padding-right: space0;
padding-left: space0;
}
<div
Expand Down Expand Up @@ -78,8 +82,8 @@ exports[`Grid render should render responsive css 1`] = `
-ms-flex-wrap: wrap;
flex-wrap: wrap;
display: flex;
margin-right: spaceNegative10;
margin-left: spaceNegative10;
margin-right: auto;
margin-left: auto;
min-width: size0;
}
Expand All @@ -88,8 +92,8 @@ exports[`Grid render should render responsive css 1`] = `
-webkit-flex-direction: column;
-ms-flex-direction: column;
flex-direction: column;
margin-right: spaceNegative20;
margin-left: spaceNegative20;
margin-right: auto;
margin-left: auto;
}
}
Expand All @@ -105,49 +109,61 @@ exports[`Grid render should render responsive css 1`] = `
.emotion-0 {
width: 16.666666666666664%;
padding-left: space10;
padding-right: space10;
padding-top: space10;
padding-bottom: space10;
padding-right: space0;
padding-left: space0;
margin-left: 66.66666666666666%;
}
@media screen and (min-width:40em) {
.emotion-0 {
width: 33.33333333333333%;
padding-left: space20;
padding-right: space20;
padding-top: space20;
padding-bottom: space20;
padding-right: space0;
padding-left: space0;
margin-left: 50%;
}
}
@media screen and (min-width:52em) {
.emotion-0 {
width: 16.666666666666664%;
padding-left: space30;
padding-top: space0;
padding-bottom: space0;
padding-right: space30;
padding-left: space30;
margin-left: 66.66666666666666%;
}
}
.emotion-1 {
width: 50%;
padding-left: space10;
padding-right: space10;
padding-top: space10;
padding-bottom: space10;
padding-right: space0;
padding-left: space0;
min-width: 100%;
margin-left: space0;
}
@media screen and (min-width:40em) {
.emotion-1 {
padding-left: space20;
padding-right: space20;
padding-top: space20;
padding-bottom: space20;
padding-right: space0;
padding-left: space0;
min-width: 100%;
}
}
@media screen and (min-width:52em) {
.emotion-1 {
padding-left: space30;
padding-top: space0;
padding-bottom: space0;
padding-right: space30;
padding-left: space30;
min-width: 0;
}
}
Expand Down
177 changes: 173 additions & 4 deletions packages/paste-core/layout/grid/__tests__/grid.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,128 @@
import * as React from 'react';
import {render} from '@testing-library/react';
import {Grid, Column} from '../src';
import {getOuterGutterPull, getStackedColumns, getColumnOffset, getColumnSpan} from '../src/utils';
import * as React from 'react';
import {Column, Grid} from '../src';
import {getColumnStyles} from '../src/Column';

import {
getColumnOffset,
getColumnSpan,
getOuterGutterPull,
getStackedColumns,
getSpacing,
getResponsiveSpacing,
} from '../src/utils';

describe('Grid', () => {
const BASE_PADDING = {
paddingBottom: 'space0',
paddingLeft: 'space0',
paddingRight: 'space0',
paddingTop: 'space0',
};
const BASE_MARGIN = {
marginBottom: 'auto',
marginLeft: 'auto',
marginRight: 'auto',
marginTop: 'auto',
};
describe('Utils', () => {
describe('getSpacing', () => {
it('should return the correct spacing when vertical is true and the prefix is "margin"', () => {
expect(getSpacing(true, 'margin', 'space20')).toEqual({
...BASE_MARGIN,
marginTop: 'space20',
marginBottom: 'space20',
});
});

it('should return the correct spacing when vertical is true and the prefix is "padding"', () => {
expect(getSpacing(true, 'padding', 'space20')).toEqual({
...BASE_PADDING,
paddingTop: 'space20',
paddingBottom: 'space20',
});
});

it('should return the correct spacing when vertical is false and the prefix is "margin"', () => {
expect(getSpacing(false, 'margin', 'space20')).toEqual({
...BASE_MARGIN,
marginRight: 'space20',
marginLeft: 'space20',
});
});

it('should return the correct spacing when vertical is false and the prefix is "padding"', () => {
expect(getSpacing(false, 'padding', 'space20')).toEqual({
...BASE_PADDING,
paddingRight: 'space20',
paddingLeft: 'space20',
});
});

it('should return the default spacing whenever spacing is undefined', () => {
// eslint-disable-next-line unicorn/no-useless-undefined
expect(getSpacing(true, 'padding', undefined)).toEqual({
...BASE_PADDING,
});

// eslint-disable-next-line unicorn/no-useless-undefined
expect(getSpacing(true, 'margin', undefined)).toEqual({
...BASE_MARGIN,
});
});
});

describe('getResponsiveSpacing', () => {
it('should return the default responsive spacing if spacing is undefined', () => {
// eslint-disable-next-line unicorn/no-useless-undefined
expect(getResponsiveSpacing([true, true, false], 'margin', undefined)).toEqual({
marginTop: ['auto', 'auto', 'auto'],
marginBottom: ['auto', 'auto', 'auto'],
marginLeft: ['auto', 'auto', 'auto'],
marginRight: ['auto', 'auto', 'auto'],
});
// eslint-disable-next-line unicorn/no-useless-undefined
expect(getResponsiveSpacing([true, true, false], 'padding', undefined)).toEqual({
paddingTop: ['space0', 'space0', 'space0'],
paddingBottom: ['space0', 'space0', 'space0'],
paddingLeft: ['space0', 'space0', 'space0'],
paddingRight: ['space0', 'space0', 'space0'],
});
});

it('should return the correct responsive spacing if spacing is a single token', () => {
expect(getResponsiveSpacing([true, true, false], 'margin', 'space20')).toEqual({
marginTop: ['space20', 'space20', 'auto'],
marginBottom: ['space20', 'space20', 'auto'],
marginLeft: ['auto', 'auto', 'space20'],
marginRight: ['auto', 'auto', 'space20'],
});

expect(getResponsiveSpacing([true, true, false], 'padding', 'space20')).toEqual({
paddingTop: ['space20', 'space20', 'space0'],
paddingBottom: ['space20', 'space20', 'space0'],
paddingLeft: ['space0', 'space0', 'space20'],
paddingRight: ['space0', 'space0', 'space20'],
});
});

it('should return the correct responsive spacing if spacing is a responsive array', () => {
expect(getResponsiveSpacing([true, true, false], 'margin', ['space10', 'space40', 'space80'])).toEqual({
marginTop: ['space10', 'space40', 'auto'],
marginBottom: ['space10', 'space40', 'auto'],
marginLeft: ['auto', 'auto', 'space80'],
marginRight: ['auto', 'auto', 'space80'],
});

expect(getResponsiveSpacing([true, true, false], 'padding', ['space10', 'space40', 'space80'])).toEqual({
paddingTop: ['space10', 'space40', 'space0'],
paddingBottom: ['space10', 'space40', 'space0'],
paddingLeft: ['space0', 'space0', 'space80'],
paddingRight: ['space0', 'space0', 'space80'],
});
});
});

describe('getOuterGutterPull', () => {
it('should return a negative token value', () => {
expect(getOuterGutterPull('space20')).toEqual('spaceNegative20');
Expand Down Expand Up @@ -100,31 +217,80 @@ describe('Grid', () => {

describe('Column', () => {
describe('getColumnStyles', () => {
it('should return column padding when passed a gutter', () => {
it('should return correct column padding when passed a gutter', () => {
expect(getColumnStyles({gutter: 'space20'})).toEqual({
...BASE_PADDING,
paddingLeft: 'space20',
paddingRight: 'space20',
width: '8.333333333333332%',
});
});

it('should return correct column padding when passed a gutter and vertical is true', () => {
expect(getColumnStyles({gutter: 'space20', vertical: true})).toEqual({
...BASE_PADDING,
marginLeft: 'space0',
minWidth: '100%',
paddingBottom: 'space20',
paddingTop: 'space20',
width: '8.333333333333332%',
});
});

it('should return correct column padding when passed no gutter and vertical is true', () => {
expect(getColumnStyles({vertical: true})).toEqual({
...BASE_PADDING,
marginLeft: 'space0',
minWidth: '100%',
width: '8.333333333333332%',
});
});

it('should return correct column padding when passed a gutter and vertical is a responsive array value', () => {
expect(getColumnStyles({gutter: 'space20', vertical: [true, true, false]})).toEqual({
marginLeft: 'space0',
minWidth: ['100%', '100%', '0'],
paddingBottom: ['space20', 'space20', 'space0'],
paddingLeft: ['space0', 'space0', 'space20'],
paddingRight: ['space0', 'space0', 'space20'],
paddingTop: ['space20', 'space20', 'space0'],
width: '8.333333333333332%',
});
});

it('should return correct column padding when passed a responsive array value for gutter and vertical', () => {
expect(getColumnStyles({gutter: ['space10', 'space30', 'space60'], vertical: [true, true, false]})).toEqual({
marginLeft: 'space0',
minWidth: ['100%', '100%', '0'],
paddingBottom: ['space10', 'space30', 'space0'],
paddingLeft: ['space0', 'space0', 'space60'],
paddingRight: ['space0', 'space0', 'space60'],
paddingTop: ['space10', 'space30', 'space0'],
width: '8.333333333333332%',
});
});

it('should return column margin left when passed a column offset based on size of offset', () => {
expect(getColumnStyles({offset: 2})).toEqual({
...BASE_PADDING,
marginLeft: '16.666666666666664%',
width: '8.333333333333332%',
});
expect(getColumnStyles({offset: 6})).toEqual({
...BASE_PADDING,
marginLeft: '50%',
width: '8.333333333333332%',
});
});

it('should set 100% width and zero offset when a column is vertical', () => {
expect(getColumnStyles({vertical: true, offset: 4})).toEqual({
...BASE_PADDING,
marginLeft: '33.33333333333333%',
width: '8.333333333333332%',
});
expect(getColumnStyles({vertical: true})).toEqual({
...BASE_PADDING,
marginLeft: 'space0',
minWidth: '100%',
width: '8.333333333333332%',
Expand All @@ -133,6 +299,7 @@ describe('Grid', () => {

it('should set the column to stretch content and display flex when setting stretch column content', () => {
expect(getColumnStyles({stretchColumnContent: true})).toEqual({
...BASE_PADDING,
alignContent: 'stretch',
display: 'flex',
width: '8.333333333333332%',
Expand Down Expand Up @@ -161,9 +328,11 @@ describe('Grid', () => {
const {asFragment} = render(
<Grid gutter={['space10', 'space20', 'space30']} vertical={[true, true, false]}>
<Column span={[2, 4, 2]} offset={[8, 6, 8]} />

<Column />
</Grid>
);

expect(asFragment()).toMatchSnapshot();
});
});
Expand Down
33 changes: 18 additions & 15 deletions packages/paste-core/layout/grid/src/Column.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,32 @@ import * as React from 'react';
import * as PropTypes from 'prop-types';
import {styled, compose, flexbox, layout, space} from '@twilio-paste/styling-library';
import {ResponsiveProp} from '@twilio-paste/style-props';
import {ColumnProps, ColumnStyleProps} from './types';
import {getStackedColumns, getColumnOffset, getColumnSpan} from './utils';

export const getColumnStyles = (props: ColumnProps): ColumnStyleProps => {
import type {ColumnProps, ColumnStyleProps} from './types';
import {getStackedColumns, getColumnOffset, getColumnSpan, getColumnPadding} from './utils';

export const getColumnStyles = ({
count,
span,
gutter,
offset,
vertical,
stretchColumnContent,
}: ColumnProps): ColumnStyleProps => {
const columnStyles: ColumnStyleProps = {
width: getColumnSpan(props),
width: getColumnSpan({count, span}),
...getColumnPadding({gutter, vertical}),
};

if (props.gutter) {
columnStyles.paddingLeft = props.gutter;
columnStyles.paddingRight = props.gutter;
}

if (props.offset) {
columnStyles.marginLeft = getColumnOffset(props.offset);
if (offset) {
columnStyles.marginLeft = getColumnOffset(offset);
}

if (props.vertical && !props.offset) {
columnStyles.minWidth = getStackedColumns(props.vertical);
if (vertical && !offset) {
columnStyles.minWidth = getStackedColumns(vertical);
columnStyles.marginLeft = 'space0';
}

if (props.stretchColumnContent) {
if (stretchColumnContent) {
columnStyles.alignContent = 'stretch';
columnStyles.display = 'flex';
}
Expand Down
Loading

0 comments on commit b02b0d6

Please sign in to comment.