From 5cfbacc3577a198125fcca4ee5a1ed1c109e8e94 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 18:39:19 +0200 Subject: [PATCH 1/7] Spacer: move types to separate file --- packages/components/src/spacer/hook.ts | 72 +----------------------- packages/components/src/spacer/index.ts | 2 +- packages/components/src/spacer/types.ts | 75 +++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 70 deletions(-) create mode 100644 packages/components/src/spacer/types.ts diff --git a/packages/components/src/spacer/hook.ts b/packages/components/src/spacer/hook.ts index a3560aade106fe..ba306782039df5 100644 --- a/packages/components/src/spacer/hook.ts +++ b/packages/components/src/spacer/hook.ts @@ -9,80 +9,14 @@ import { css } from '@emotion/react'; import { useContextSystem } from '../ui/context'; // eslint-disable-next-line no-duplicate-imports import type { PolymorphicComponentProps } from '../ui/context'; -import { space, SpaceInput } from '../ui/utils/space'; +import { space } from '../ui/utils/space'; import { useCx } from '../utils/hooks/use-cx'; +import type { Props } from './types'; const isDefined = < T >( o: T ): o is Exclude< T, null | undefined > => typeof o !== 'undefined' && o !== null; -export interface SpacerProps { - /** - * Adjusts all margins. - */ - margin?: SpaceInput; - /** - * Adjusts top and bottom margins. - */ - marginY?: SpaceInput; - /** - * Adjusts left and right margins. - */ - marginX?: SpaceInput; - /** - * Adjusts top margins. - */ - marginTop?: SpaceInput; - /** - * Adjusts bottom margins. - * - * @default 2 - */ - marginBottom?: SpaceInput; - /** - * Adjusts left margins. - */ - marginLeft?: SpaceInput; - /** - * Adjusts right margins. - */ - marginRight?: SpaceInput; - /** - * Adjusts all padding. - */ - padding?: SpaceInput; - /** - * Adjusts top and bottom padding. - */ - paddingY?: SpaceInput; - /** - * Adjusts left and right padding. - */ - paddingX?: SpaceInput; - /** - * Adjusts top padding. - */ - paddingTop?: SpaceInput; - /** - * Adjusts bottom padding. - */ - paddingBottom?: SpaceInput; - /** - * Adjusts left padding. - */ - paddingLeft?: SpaceInput; - /** - * Adjusts right padding. - */ - paddingRight?: SpaceInput; - /** - * The children elements. - */ - children?: React.ReactNode; -} - -export function useSpacer( - props: PolymorphicComponentProps< SpacerProps, 'div' > -) { +export function useSpacer( props: PolymorphicComponentProps< Props, 'div' > ) { const { className, margin, diff --git a/packages/components/src/spacer/index.ts b/packages/components/src/spacer/index.ts index 765121f409b066..93662af4a7749b 100644 --- a/packages/components/src/spacer/index.ts +++ b/packages/components/src/spacer/index.ts @@ -1,3 +1,3 @@ export { default as Spacer } from './component'; export { useSpacer } from './hook'; -export type { SpacerProps } from './hook'; +export type { Props as SpacerProps } from './types'; diff --git a/packages/components/src/spacer/types.ts b/packages/components/src/spacer/types.ts new file mode 100644 index 00000000000000..01d45525a14d1a --- /dev/null +++ b/packages/components/src/spacer/types.ts @@ -0,0 +1,75 @@ +/** + * External dependencies + */ +// eslint-disable-next-line no-restricted-imports +import type { ReactNode } from 'react'; + +/** + * Internal dependencies + */ +import type { SpaceInput } from '../ui/utils/space'; + +export interface InnerProps { + /** + * Adjusts all margins. + */ + margin?: SpaceInput; + /** + * Adjusts top and bottom margins. + */ + marginY?: SpaceInput; + /** + * Adjusts left and right margins. + */ + marginX?: SpaceInput; + /** + * Adjusts top margins. + */ + marginTop?: SpaceInput; + /** + * Adjusts bottom margins. + * + * @default 2 + */ + marginBottom?: SpaceInput; + /** + * Adjusts left margins. + */ + marginLeft?: SpaceInput; + /** + * Adjusts right margins. + */ + marginRight?: SpaceInput; + /** + * Adjusts all padding. + */ + padding?: SpaceInput; + /** + * Adjusts top and bottom padding. + */ + paddingY?: SpaceInput; + /** + * Adjusts left and right padding. + */ + paddingX?: SpaceInput; + /** + * Adjusts top padding. + */ + paddingTop?: SpaceInput; + /** + * Adjusts bottom padding. + */ + paddingBottom?: SpaceInput; + /** + * Adjusts left padding. + */ + paddingLeft?: SpaceInput; + /** + * Adjusts right padding. + */ + paddingRight?: SpaceInput; +} + +export interface Props extends InnerProps { + children?: ReactNode; +} From 677afcee9163e5c3446aef95230d84f40133c3df Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 18:50:25 +0200 Subject: [PATCH 2/7] Spacer: add story --- .../components/src/spacer/stories/index.js | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 packages/components/src/spacer/stories/index.js diff --git a/packages/components/src/spacer/stories/index.js b/packages/components/src/spacer/stories/index.js new file mode 100644 index 00000000000000..585a7910d7ade6 --- /dev/null +++ b/packages/components/src/spacer/stories/index.js @@ -0,0 +1,56 @@ +/** + * External dependencies + */ +import { number } from '@storybook/addon-knobs'; + +/** + * Internal dependencies + */ +/** + * Internal dependencies + */ +import { Spacer } from '..'; + +export default { + component: Spacer, + title: 'Components (Experimental)/Spacer', +}; + +const PROPS = [ + 'marginTop', + 'marginBottom', + 'marginLeft', + 'marginRight', + 'marginX', + 'marginY', + 'margin', + + 'paddingTop', + 'paddingBottom', + 'paddingLeft', + 'paddingRight', + 'paddingX', + 'paddingY', + 'padding', +]; + +const BlackBox = () => ( +
+); + +export const _default = () => { + const props = PROPS.reduce( + ( acc, prop ) => ( { ...acc, [ prop ]: number( prop, undefined ) } ), + {} + ); + + return ( + <> + + This is the spacer + + + ); +}; From 6b8fe2ff02ca9630044b501d17bd3a2855e6e408 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 18:53:13 +0200 Subject: [PATCH 3/7] Spacer: swap margin and padding styles order to allow for better composition --- packages/components/src/spacer/hook.ts | 44 +++++++++---------- .../components/src/spacer/stories/index.js | 12 ++--- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/components/src/spacer/hook.ts b/packages/components/src/spacer/hook.ts index ba306782039df5..e228ecf883db14 100644 --- a/packages/components/src/spacer/hook.ts +++ b/packages/components/src/spacer/hook.ts @@ -39,6 +39,20 @@ export function useSpacer( props: PolymorphicComponentProps< Props, 'div' > ) { const cx = useCx(); const classes = cx( + isDefined( margin ) && + css` + margin: ${ space( margin ) }; + `, + isDefined( marginY ) && + css` + margin-bottom: ${ space( marginY ) }; + margin-top: ${ space( marginY ) }; + `, + isDefined( marginX ) && + css` + margin-left: ${ space( marginX ) }; + margin-right: ${ space( marginX ) }; + `, isDefined( marginTop ) && css` margin-top: ${ space( marginTop ) }; @@ -55,19 +69,19 @@ export function useSpacer( props: PolymorphicComponentProps< Props, 'div' > ) { css` margin-right: ${ space( marginRight ) }; `, - isDefined( marginX ) && + isDefined( padding ) && css` - margin-left: ${ space( marginX ) }; - margin-right: ${ space( marginX ) }; + padding: ${ space( padding ) }; `, - isDefined( marginY ) && + isDefined( paddingY ) && css` - margin-bottom: ${ space( marginY ) }; - margin-top: ${ space( marginY ) }; + padding-bottom: ${ space( paddingY ) }; + padding-top: ${ space( paddingY ) }; `, - isDefined( margin ) && + isDefined( paddingX ) && css` - margin: ${ space( margin ) }; + padding-left: ${ space( paddingX ) }; + padding-right: ${ space( paddingX ) }; `, isDefined( paddingTop ) && css` @@ -85,20 +99,6 @@ export function useSpacer( props: PolymorphicComponentProps< Props, 'div' > ) { css` padding-right: ${ space( paddingRight ) }; `, - isDefined( paddingX ) && - css` - padding-left: ${ space( paddingX ) }; - padding-right: ${ space( paddingX ) }; - `, - isDefined( paddingY ) && - css` - padding-bottom: ${ space( paddingY ) }; - padding-top: ${ space( paddingY ) }; - `, - isDefined( padding ) && - css` - padding: ${ space( padding ) }; - `, className ); diff --git a/packages/components/src/spacer/stories/index.js b/packages/components/src/spacer/stories/index.js index 585a7910d7ade6..230b5d1f298a8e 100644 --- a/packages/components/src/spacer/stories/index.js +++ b/packages/components/src/spacer/stories/index.js @@ -17,21 +17,21 @@ export default { }; const PROPS = [ + 'margin', + 'marginY', + 'marginX', 'marginTop', 'marginBottom', 'marginLeft', 'marginRight', - 'marginX', - 'marginY', - 'margin', + 'padding', + 'paddingY', + 'paddingX', 'paddingTop', 'paddingBottom', 'paddingLeft', 'paddingRight', - 'paddingX', - 'paddingY', - 'padding', ]; const BlackBox = () => ( From 888ab9e0b0329d15dcbb5a26aa050b07bd64dac8 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 19:03:53 +0200 Subject: [PATCH 4/7] Update README --- packages/components/src/spacer/README.md | 83 ++++++++++++------------ 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/packages/components/src/spacer/README.md b/packages/components/src/spacer/README.md index 7bc5b01abf42d3..01946b493c0343 100644 --- a/packages/components/src/spacer/README.md +++ b/packages/components/src/spacer/README.md @@ -33,86 +33,87 @@ function Example() { ## Props -##### margin - -**Type**: `number` +### `margin`: `number` Adjusts all margins. -##### marginBottom +- Required: No -**Type**: `number` +### `marginY`: `number` -Adjusts bottom margins. +Adjusts top and bottom margins, potentially overriding the value from the more generic `margin` prop. -##### marginLeft +- Required: No -**Type**: `number` +### `marginX`: `number` -Adjusts left margins. +Adjusts left and right margins, potentially overriding the value from the more generic `margin` prop. -##### marginRight +- Required: No -**Type**: `number` +### `marginTop`: `number` -Adjusts right margins. +Adjusts top margin, potentially overriding the value from the more generic `margin` and `marginY` props. -##### marginTop +- Required: No -**Type**: `number` +### `marginBottom`: `number` -Adjusts top margins. +Adjusts bottom margin, potentially overriding the value from the more generic `margin` and `marginY` props. -##### marginX +- Required: No +- Default: `2` -**Type**: `number` +### `marginLeft`: `number` -Adjusts left and right margins. +Adjusts left margin, potentially overriding the value from the more generic `margin` and `marginX` props. -##### marginY +- Required: No -**Type**: `number` +### `marginRight`: `number` -Adjusts top and bottom margins. +Adjusts right margin, potentially overriding the value from the more generic `margin` and `marginX` props. -##### padding +- Required: No -**Type**: `number` +### `padding`: `number` Adjusts all padding. -##### paddingBottom +- Required: No + +### `paddingY`: `number` -**Type**: `number` +Adjusts top and bottom padding, potentially overriding the value from the more generic `padding` prop. -Adjusts bottom padding. +- Required: No -##### paddingLeft +### `paddingX`: `number` -**Type**: `number` +Adjusts left and right padding, potentially overriding the value from the more generic `padding` prop. -Adjusts left padding. +- Required: No -##### paddingRight +### `paddingTop`: `number` -**Type**: `number` +Adjusts top padding, potentially overriding the value from the more generic `padding` and `paddingY` props. -Adjusts right padding. +- Required: No -##### paddingTop +### `paddingBottom`: `number` -**Type**: `number` +Adjusts bottom padding, potentially overriding the value from the more generic `padding` and `paddingY` props. -Adjusts top padding. +- Required: No -##### paddingX +### `paddingLeft`: `number` -**Type**: `number` +Adjusts left padding, potentially overriding the value from the more generic `padding` and `paddingX` props. -Adjusts left and right padding. +- Required: No -##### paddingY +### `paddingRight`: `number` -**Type**: `number` +Adjusts right padding, potentially overriding the value from the more generic `padding` and `paddingX` props. -Adjusts top and bottom padding. +- Required: No From 3d5aa00cabc5ea51ba0ebd505a8263d388b4d163 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 19:07:10 +0200 Subject: [PATCH 5/7] Update snapshots --- .../components/src/spacer/test/__snapshots__/index.js.snap | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/components/src/spacer/test/__snapshots__/index.js.snap b/packages/components/src/spacer/test/__snapshots__/index.js.snap index b11a8cc37124e9..309a2b10dbae66 100644 --- a/packages/components/src/spacer/test/__snapshots__/index.js.snap +++ b/packages/components/src/spacer/test/__snapshots__/index.js.snap @@ -98,9 +98,8 @@ Snapshot Diff: Array [ Object { -- "margin-bottom": "calc(4px * 5)", + "margin-bottom": "calc(4px * 2)", - "margin-top": "calc(4px * 5)", -+ "margin-bottom": "calc(4px * 2)", }, ] `; From 4e2b3a859109532e922512eca4f711579342804e Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 19:14:20 +0200 Subject: [PATCH 6/7] Add unit tests to check props override order --- .../spacer/test/__snapshots__/index.js.snap | 32 +++++++++++++++++++ packages/components/src/spacer/test/index.js | 28 ++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/packages/components/src/spacer/test/__snapshots__/index.js.snap b/packages/components/src/spacer/test/__snapshots__/index.js.snap index 309a2b10dbae66..1daea39bad4fa1 100644 --- a/packages/components/src/spacer/test/__snapshots__/index.js.snap +++ b/packages/components/src/spacer/test/__snapshots__/index.js.snap @@ -1,5 +1,37 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`props should override margin props from less to more specific 1`] = ` +Snapshot Diff: +- Received styles ++ Base styles + + Array [ + Object { +- "margin": "calc(4px * 10)", +- "margin-bottom": "calc(4px * 1)", +- "margin-left": "calc(4px * 3)", +- "margin-right": "calc(4px * 5)", ++ "margin-bottom": "calc(4px * 2)", + }, + ] +`; + +exports[`props should override padding props from less to more specific 1`] = ` +Snapshot Diff: +- Received styles ++ Base styles + + Array [ + Object { + "margin-bottom": "calc(4px * 2)", +- "padding": "calc(4px * 10)", +- "padding-bottom": "calc(4px * 2)", +- "padding-left": "calc(4px * 3)", +- "padding-top": "calc(4px * 5)", + }, + ] +`; + exports[`props should render correctly 1`] = ` .emotion-0 { margin-bottom: calc(4px * 2); diff --git a/packages/components/src/spacer/test/index.js b/packages/components/src/spacer/test/index.js index 47d9ab1833596c..33edeb22decd64 100644 --- a/packages/components/src/spacer/test/index.js +++ b/packages/components/src/spacer/test/index.js @@ -67,6 +67,20 @@ describe( 'props', () => { ); } ); + test( 'should override margin props from less to more specific', () => { + const { container } = render( + + ); + expect( container.firstChild ).toMatchStyleDiffSnapshot( + base.firstChild + ); + } ); + test( 'should render padding', () => { const { container } = render( ); expect( container.firstChild ).toMatchStyleDiffSnapshot( @@ -115,4 +129,18 @@ describe( 'props', () => { base.firstChild ); } ); + + test( 'should override padding props from less to more specific', () => { + const { container } = render( + + ); + expect( container.firstChild ).toMatchStyleDiffSnapshot( + base.firstChild + ); + } ); } ); From 2a0cf2bcaae967a9e18338eb8cce3a305d2de88b Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 19 Jul 2021 20:54:36 +0200 Subject: [PATCH 7/7] Delete unnecessary InnerProps type --- packages/components/src/spacer/types.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/components/src/spacer/types.ts b/packages/components/src/spacer/types.ts index 01d45525a14d1a..c5a6afee1ca2e0 100644 --- a/packages/components/src/spacer/types.ts +++ b/packages/components/src/spacer/types.ts @@ -9,7 +9,7 @@ import type { ReactNode } from 'react'; */ import type { SpaceInput } from '../ui/utils/space'; -export interface InnerProps { +export type Props = { /** * Adjusts all margins. */ @@ -68,8 +68,8 @@ export interface InnerProps { * Adjusts right padding. */ paddingRight?: SpaceInput; -} - -export interface Props extends InnerProps { + /** + * The children elements. + */ children?: ReactNode; -} +};