Skip to content

Commit

Permalink
avoid confusion
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Sep 11, 2020
1 parent 182a290 commit 40c9c5a
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 23 deletions.
2 changes: 1 addition & 1 deletion docs/src/pages/guides/migration-v4/migration-v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ For a smoother transition, the `adaptV4Theme` helper allows you to iteratively u
```

- `theme.spacing` now returns single values with px units by default.
This improves the usage with styled-components.
This change improves the integration with styled-components & emotion.

Before:

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Grid/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function generateGutter(theme, breakpoint) {
SPACINGS.forEach((spacing) => {
const themeSpacing = theme.spacing(spacing);

if (themeSpacing === 0) {
if (themeSpacing === '0px') {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/styles/adaptV4Theme.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import createBreakpoints from './createBreakpoints';
import createSpacing from './createV4Spacing';
import createV4Spacing from './createV4Spacing';

export default function adaptV4Theme(inputTheme) {
if (process.env.NODE_ENV !== 'production') {
Expand Down Expand Up @@ -52,7 +52,7 @@ export default function adaptV4Theme(inputTheme) {
});

// theme.spacing
theme.spacing = createSpacing(inputTheme.spacing);
theme.spacing = createV4Spacing(inputTheme.spacing);

// theme.mixins.gutters
const breakpoints = createBreakpoints(inputTheme.breakpoints || {});
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui/src/styles/createSpacing.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { createUnarySpacing } from '@material-ui/system';

let warnOnce;

export default function createSpacing(spacingInput = 8) {
// Already transformed.
if (spacingInput.mui) {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/styles/createV4Spacing.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createUnarySpacing } from '@material-ui/system';

export default function createSpacing(spacingInput = 8) {
export default function createV4Spacing(spacingInput = 8) {
// Already transformed.
if (spacingInput.mui) {
return spacingInput;
Expand Down
32 changes: 16 additions & 16 deletions packages/material-ui/src/styles/createV4Spacing.test.js
Original file line number Diff line number Diff line change
@@ -1,54 +1,54 @@
import { expect } from 'chai';
import createSpacing from './createV4Spacing';
import createV4Spacing from './createV4Spacing';

describe('createSpacing', () => {
describe('createV4Spacing', () => {
it('should work as expected', () => {
let spacing;
spacing = createSpacing();
spacing = createV4Spacing();
expect(spacing(1)).to.equal(8);
spacing = createSpacing(10);
spacing = createV4Spacing(10);
expect(spacing(1)).to.equal(10);
spacing = createSpacing([0, 8, 16]);
spacing = createV4Spacing([0, 8, 16]);
expect(spacing(2)).to.equal(16);
spacing = createSpacing((factor) => factor ** 2);
spacing = createV4Spacing((factor) => factor ** 2);
expect(spacing(2)).to.equal(4);
spacing = createSpacing((factor) => `${0.25 * factor}rem`);
spacing = createV4Spacing((factor) => `${0.25 * factor}rem`);
expect(spacing(2)).to.equal('0.5rem');
});

it('should support recursion', () => {
const spacing = createSpacing();
createSpacing(spacing);
const spacing = createV4Spacing();
createV4Spacing(spacing);
});

it('should support a default value when no arguments are provided', () => {
let spacing;
spacing = createSpacing();
spacing = createV4Spacing();
expect(spacing()).to.equal(8);
spacing = createSpacing((factor) => `${0.25 * factor}rem`);
spacing = createV4Spacing((factor) => `${0.25 * factor}rem`);
expect(spacing()).to.equal('0.25rem');
});

it('should support multiple arguments', () => {
let spacing;
spacing = createSpacing();
spacing = createV4Spacing();
expect(spacing(1, 2)).to.equal('8px 16px');
spacing = createSpacing((factor) => `${0.25 * factor}rem`);
spacing = createV4Spacing((factor) => `${0.25 * factor}rem`);
expect(spacing(1, 2)).to.equal('0.25rem 0.5rem');
});

it('should support string arguments', () => {
let spacing;
spacing = createSpacing();
spacing = createV4Spacing();
expect(spacing(1, 'auto')).to.equal('8px auto');
spacing = createSpacing((factor) => `${0.25 * factor}rem`);
spacing = createV4Spacing((factor) => `${0.25 * factor}rem`);
expect(spacing(1, 'auto', 2, 3)).to.equal('0.25rem auto 0.5rem 0.75rem');
});

describe('warnings', () => {
it('should warn for wrong input', () => {
expect(() => {
createSpacing({
createV4Spacing({
unit: 4,
});
}).toErrorDev('Material-UI: The `theme.spacing` value ([object Object]) is invalid');
Expand Down

0 comments on commit 40c9c5a

Please sign in to comment.