Skip to content

Commit

Permalink
[theme] Fix spacing string arguments (#23224)
Browse files Browse the repository at this point in the history
  • Loading branch information
GuilleDF authored Oct 26, 2020
1 parent 62ecf26 commit 42183a9
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 11 deletions.
4 changes: 2 additions & 2 deletions packages/material-ui-system/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ export type SpacingProps = PropsFor<typeof spacing>;
export function createUnarySpacing<Spacing>(theme: {
spacing: Spacing;
}): Spacing extends number
? (abs: number) => number
? (abs: number | string) => number | number
: Spacing extends any[]
? <Index extends number>(abs: Index) => Spacing[Index]
? <Index extends number>(abs: Index | string) => Spacing[Index] | string
: Spacing extends (...args: unknown[]) => unknown
? Spacing
: // warns in Dev
Expand Down
12 changes: 11 additions & 1 deletion packages/material-ui-system/src/spacing.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,15 @@ export function createUnarySpacing(theme) {

if (typeof themeSpacing === 'number') {
return (abs) => {
if (typeof abs === 'string') {
return abs;
}

if (process.env.NODE_ENV !== 'production') {
if (typeof abs !== 'number') {
console.error(`Material-UI: Expected spacing argument to be a number, got ${abs}.`);
console.error(
`Material-UI: Expected spacing argument to be a number or a string, got ${abs}.`,
);
}
}
return themeSpacing * abs;
Expand All @@ -90,6 +96,10 @@ export function createUnarySpacing(theme) {

if (Array.isArray(themeSpacing)) {
return (abs) => {
if (typeof abs === 'string') {
return abs;
}

if (process.env.NODE_ENV !== 'production') {
if (abs > themeSpacing.length - 1) {
console.error(
Expand Down
2 changes: 2 additions & 0 deletions packages/material-ui/src/styles/createSpacing.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export interface Spacing {

export type SpacingOptions =
| number
| Spacing
| ((factor: number) => string | number)
| ((factor: number | string) => string | number)
| Array<string | number>;

export default function createSpacing(spacing: SpacingOptions): Spacing;
12 changes: 7 additions & 5 deletions packages/material-ui/src/styles/createSpacing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ describe('createSpacing', () => {
expect(spacing(2)).to.equal('16px');
spacing = createSpacing(['0rem', '8rem', '16rem']);
expect(spacing(2)).to.equal('16rem');
spacing = createSpacing((factor) => factor ** 2);
spacing = createSpacing((factor: number) => factor ** 2);
expect(spacing(2)).to.equal('4px');
spacing = createSpacing((factor) => `${0.25 * factor}rem`);
spacing = createSpacing((factor: number) => `${0.25 * factor}rem`);
expect(spacing(2)).to.equal('0.5rem');
});

Expand All @@ -27,23 +27,25 @@ describe('createSpacing', () => {
let spacing;
spacing = createSpacing();
expect(spacing()).to.equal('8px');
spacing = createSpacing((factor) => `${0.25 * factor}rem`);
spacing = createSpacing((factor: number) => `${0.25 * factor}rem`);
expect(spacing()).to.equal('0.25rem');
});

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

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

Expand Down
4 changes: 1 addition & 3 deletions packages/material-ui/src/styles/createSpacing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export type SpacingOptions =
| number
| Spacing
| ((abs: number) => number | string)
| ((abs: number | string) => number | string)
| Array<string | number>;

export type SpacingArgument = number | string;
Expand Down Expand Up @@ -53,9 +54,6 @@ export default function createSpacing(spacingInput: SpacingOptions = 8): Spacing

return args
.map((argument) => {
if (typeof argument === 'string') {
return argument;
}
const output = transform(argument);
return typeof output === 'number' ? `${output}px` : output;
})
Expand Down

0 comments on commit 42183a9

Please sign in to comment.