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

[system] Array reject on spacing transformation fixed #19900

Merged
merged 3 commits into from
Mar 1, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/pages/customization/spacing/spacing.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ theme.spacing(2); // = 0.25 * 2rem = 0.5rem = 8px

```js
const theme = createMuiTheme({
spacing: factor => [0, 4, 8, 16, 32, 64][factor],
spacing: [0, 4, 8, 16, 32, 64],
});

theme.spacing(2); // = 8
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui-system/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export { default as shadows } from './shadows';
export { default as sizing } from './sizing';
export * from './sizing';
export { default as spacing } from './spacing';
export * from './spacing';
export { default as style } from './style';
export { default as typography } from './typography';
export * from './typography';
15 changes: 12 additions & 3 deletions packages/material-ui-system/src/spacing.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,20 @@ const spacingKeys = [
'paddingY',
];

function getTransformer(theme) {
export function createUnarySpacing(theme) {
const themeSpacing = theme.spacing || 8;

if (typeof themeSpacing === 'number') {
return abs => themeSpacing * abs;
return abs => {
if (process.env.NODE_ENV !== 'production') {
if (typeof abs !== 'number') {
console.error(
`@material-ui/system: expected spacing argument to be a number, got ${abs}.`,
);
}
}
return themeSpacing * abs;
};
}

if (Array.isArray(themeSpacing)) {
Expand Down Expand Up @@ -144,7 +153,7 @@ function getStyleFromPropValue(cssProperties, transformer) {

function spacing(props) {
const theme = props.theme;
const transformer = getTransformer(theme);
const transformer = createUnarySpacing(theme);

return Object.keys(props)
.map(prop => {
Expand Down
34 changes: 8 additions & 26 deletions packages/material-ui/src/styles/createSpacing.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { createUnarySpacing } from '@material-ui/system';

let warnOnce;

export default function createSpacing(spacingInput = 8) {
Expand All @@ -6,32 +8,12 @@ export default function createSpacing(spacingInput = 8) {
return spacingInput;
}

// All components align to an 8dp square baseline grid for mobile, tablet, and desktop.
// https://material.io/design/layout/understanding-layout.html#pixel-density
let transform;

if (typeof spacingInput === 'function') {
transform = spacingInput;
} else {
if (process.env.NODE_ENV !== 'production') {
if (typeof spacingInput !== 'number') {
console.error(
[
`Material-UI: the \`theme.spacing\` value (${spacingInput}) is invalid.`,
'It should be a number or a function.',
].join('\n'),
);
}
}
transform = factor => {
if (process.env.NODE_ENV !== 'production') {
if (typeof factor !== 'number') {
console.error(`Expected spacing argument to be a number, got ${factor}`);
}
}
return spacingInput * factor;
};
}
// Material Design layouts are visually balanced. Most measurements align to an 8dp grid applied, which aligns both spacing and the overall layout.
// Smaller components, such as icons and type, can align to a 4dp grid.
// https://material.io/design/layout/understanding-layout.html#usage
const transform = createUnarySpacing({
spacing: spacingInput,
});

const spacing = (...args) => {
if (process.env.NODE_ENV !== 'production') {
Expand Down