From 4206bf164fbeb4a66b691befe089fec2ea24c1f1 Mon Sep 17 00:00:00 2001 From: Vince Speelman Date: Fri, 22 Jun 2018 23:14:55 -0400 Subject: [PATCH 1/4] add "container" prop to Cartesian Allow the consumer to provide an optional `container` prop. This prop is the same type as the `component` prop and behaves the same way. The `container` prop is a way for consumers to control the way each child of the Cartesian component may look. I anticipate this being used primarily for spacing and layout, when the product of the cartesian component does not have any opinions about its container. --- core/src/Cartesian.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/core/src/Cartesian.js b/core/src/Cartesian.js index fdfa9e14..4f09f5f7 100644 --- a/core/src/Cartesian.js +++ b/core/src/Cartesian.js @@ -1,10 +1,24 @@ import React, { Fragment } from 'react' import { cartesianProduct } from './util' -export default ({ component, ...props }) => { +export default ({ component, container, ...props }) => { const combinations = cartesianProduct(props) const Component = component + + if (container) { + const Container = container + return ( + + {combinations.map((props, i) => ( + + + + ))} + + ) + } + return ( {combinations.map((props, i) => )} From 22fe236ac85e6ad8c15b558ee32cefb5d5edf426 Mon Sep 17 00:00:00 2001 From: Vince Speelman Date: Fri, 22 Jun 2018 23:18:55 -0400 Subject: [PATCH 2/4] move key to the container since the container is now the outermost child, it needs the key. --- core/src/Cartesian.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/Cartesian.js b/core/src/Cartesian.js index 4f09f5f7..e4f8d817 100644 --- a/core/src/Cartesian.js +++ b/core/src/Cartesian.js @@ -11,8 +11,8 @@ export default ({ component, container, ...props }) => { return ( {combinations.map((props, i) => ( - - + + ))} From e1b04871224261238f814092350240504b44edae Mon Sep 17 00:00:00 2001 From: Vince Speelman Date: Sat, 23 Jun 2018 08:56:30 -0400 Subject: [PATCH 3/4] default container to `Fragment` by defaulting the container prop to `Fragment`, we eliminate a set of possibilities and a [branch in the code](https://github.com/c8r/kit/pull/197#discussion_r197610976). --- core/src/Cartesian.js | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/core/src/Cartesian.js b/core/src/Cartesian.js index e4f8d817..d4e68bb0 100644 --- a/core/src/Cartesian.js +++ b/core/src/Cartesian.js @@ -1,27 +1,18 @@ import React, { Fragment } from 'react' import { cartesianProduct } from './util' -export default ({ component, container, ...props }) => { +export default ({ component, container = Fragment, ...props }) => { const combinations = cartesianProduct(props) const Component = component - - if (container) { - const Container = container - - return ( - - {combinations.map((props, i) => ( - - - - ))} - - ) - } + const Container = container return ( - {combinations.map((props, i) => )} + {combinations.map((props, i) => ( + + + + ))} ) } From 10e67d6bc1ba13b957a57086ef22433b96d358a0 Mon Sep 17 00:00:00 2001 From: Vince Speelman Date: Sat, 23 Jun 2018 09:56:02 -0400 Subject: [PATCH 4/4] cover `Cartesian`'s `container` prop with a test --- core/__tests__/Cartesian.js | 42 +++++++--- .../__tests__/__snapshots__/Cartesian.js.snap | 77 +++++++++++++++++++ 2 files changed, 108 insertions(+), 11 deletions(-) diff --git a/core/__tests__/Cartesian.js b/core/__tests__/Cartesian.js index 6beae216..2e101fa3 100644 --- a/core/__tests__/Cartesian.js +++ b/core/__tests__/Cartesian.js @@ -1,25 +1,45 @@ -import React from 'react' -import styled from 'styled-components' -import { render } from 'react-testing-library' -import { color, space, fontSize } from 'styled-system' +import React from 'react'; +import styled from 'styled-components'; +import { render } from 'react-testing-library'; +import { color, space, fontSize } from 'styled-system'; -import { Cartesian } from '../src' +import { Cartesian } from '../src'; const buttonProps = { children: 'Hello, world!', fontSize: [1, 2, 3], px: [2, 3], - backgroundColor: ['pink', 'tomato', 'purple'] -} + backgroundColor: ['pink', 'tomato', 'purple'], +}; const Button = styled.a` ${color} ${fontSize} ${space} -` +`; + +const Container = styled.div` + padding: 1rem; +`; test('Cartesian renders all examples', () => { - const { container } = render() + const { container } = render( + , + ); + + expect(container).toMatchSnapshot(); +}); + +test('Cartesian renders all examples in a container', () => { + const { getByTestId, container } = render( + } + />, + ); - expect(container).toMatchSnapshot() -}) + const containerNode = getByTestId('container'); + expect(containerNode.nodeName).toBe('DIV'); + expect(container).toMatchSnapshot(); +}); diff --git a/core/__tests__/__snapshots__/Cartesian.js.snap b/core/__tests__/__snapshots__/Cartesian.js.snap index d95e704e..47e05a1a 100644 --- a/core/__tests__/__snapshots__/Cartesian.js.snap +++ b/core/__tests__/__snapshots__/Cartesian.js.snap @@ -112,3 +112,80 @@ exports[`Cartesian renders all examples 1`] = ` `; + +exports[`Cartesian renders all examples in a container 1`] = ` +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+`;