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

bugfix: map Stack children in div and remove prop as to render ol/ul #1451

Merged
merged 7 commits into from
Nov 24, 2021
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
6 changes: 6 additions & 0 deletions .changeset/itchy-seas-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"docs": patch
"@marigold/components": patch
---

bugfix: change Stack that pt has higher priority than base
42 changes: 26 additions & 16 deletions docs/content/components/stack.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ Layout component to stack elements. This component uses the spaces from the give
import { Stack } from '@marigold/components';
```

## Live-Code Example

```tsx live
<Stack space="large">
<Heading>Header</Heading>
<Text>Some text.</Text>
<Text>Some more code.</Text>
</Stack>
```

## Usage

### Spacing
Expand All @@ -43,22 +53,6 @@ import { Stack } from '@marigold/components';
</Stack>
```

### Alignment

```tsx
<>
<Stack space="medium">
<Text>Left</Text>
</Stack>
<Stack space="medium" align="center">
<Text>Center</Text>
</Stack>
<Stack space="medium" align="right">
<Text>Right</Text>
</Stack>
</>
```

### Nested

```tsx
Expand All @@ -80,6 +74,22 @@ import { Stack } from '@marigold/components';
</Stack>
```

### Alignment

```tsx
<>
<Stack space="medium">
<Text>Left</Text>
</Stack>
<Stack space="medium" align="center">
<Text>Center</Text>
</Stack>
<Stack space="medium" align="right">
<Text>Right</Text>
</Stack>
</>
```

### Usage for Lists

```tsx
Expand Down
40 changes: 20 additions & 20 deletions packages/components/src/Stack/Stack.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,39 +44,39 @@ test('accepts and uses spacing from theme', () => {
const first = screen.getByText(/first/);
const second = screen.getByText(/second/);

expect(first).toHaveStyle(`padding-top: 0px`);
expect(second).toHaveStyle(`padding-top: 2px`);
expect(first.parentElement).toHaveStyle(`padding-top: 0px`);
expect(second.parentElement).toHaveStyle(`padding-top: 2px`);
});

test('aligns children left by default', () => {
render(
<Stack>
<Stack data-testid="stack">
<Text>first</Text>
</Stack>
);
const stack = screen.getByText(/first/).parentElement;
const stack = screen.getByTestId('stack');

expect(stack).toHaveStyle(`align-items: flex-start`);
});

test('allows to aligns children to the center', () => {
render(
<Stack align="center">
<Stack align="center" data-testid="stack">
<Text>first</Text>
</Stack>
);
const stack = screen.getByText(/first/).parentElement;
const stack = screen.getByTestId('stack');

expect(stack).toHaveStyle(`align-items: center`);
});

test('allows to aligns children to the right', () => {
render(
<Stack align="right">
<Stack align="right" data-testid="stack">
<Text>first</Text>
</Stack>
);
const stack = screen.getByText(/first/).parentElement;
const stack = screen.getByTestId('stack');

expect(stack).toHaveStyle(`align-items: flex-end`);
});
Expand All @@ -85,11 +85,11 @@ test('supports nesting', () => {
render(
<ThemeProvider theme={theme}>
<Stack space="large">
<Stack space="small">
<Stack space="small" data-testid="upperStack">
<Text>first</Text>
<Text>second</Text>
</Stack>
<Stack space="small">
<Stack space="small" data-testid="lowerStack">
<Text>third</Text>
<Text>fourth</Text>
</Stack>
Expand All @@ -98,33 +98,33 @@ test('supports nesting', () => {
);
const first = screen.getByText(/first/);
const second = screen.getByText(/second/);
const upperStack = first.parentElement;
const upperStack = screen.getByTestId('upperStack');

const third = screen.getByText(/third/);
const fourth = screen.getByText(/fourth/);
const lowerStack = third.parentElement;
const lowerStack = screen.getByTestId('lowerStack');

expect(upperStack).toHaveStyle(`padding-top: 0px`);
expect(lowerStack).toHaveStyle(`padding-top: 8px`);
expect(upperStack.parentElement).toHaveStyle(`padding-top: 0px`);
expect(lowerStack.parentElement).toHaveStyle(`padding-top: 8px`);

expect(first).toHaveStyle(`padding-top: 0px`);
expect(second).toHaveStyle(`padding-top: 2px`);
expect(first.parentElement).toHaveStyle(`padding-top: 0px`);
expect(second.parentElement).toHaveStyle(`padding-top: 2px`);

expect(third).toHaveStyle(`padding-top: 0px`);
expect(fourth).toHaveStyle(`padding-top: 2px`);
expect(third.parentElement).toHaveStyle(`padding-top: 0px`);
expect(fourth.parentElement).toHaveStyle(`padding-top: 2px`);
});

test('renders as div per default', () => {
render(
<ThemeProvider theme={theme}>
<Stack>
<Stack data-testid="stack">
<Text>first</Text>
<Text>second</Text>
</Stack>
</ThemeProvider>
);

const stack = screen.getByText(/first/).parentElement;
const stack = screen.getByTestId('stack');
expect(stack instanceof HTMLDivElement).toBeTruthy();
});

Expand Down
40 changes: 25 additions & 15 deletions packages/components/src/Stack/Stack.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import React from 'react';
import React, { Children } from 'react';
import flattenChildren from 'react-keyed-flatten-children';

import { ResponsiveStyleValue } from '@marigold/system';

import { Box } from '../Box';
Expand All @@ -21,17 +23,25 @@ export const Stack: React.FC<StackProps> = ({
align = 'left',
children,
...props
}) => {
return (
<Box
{...props}
as={as}
display="flex"
flexDirection="column"
alignItems={ALIGNMENT[align]}
css={{ '> * + *': { pt: space } }}
>
{children}
</Box>
);
};
}) => (
<Box
{...props}
as={as}
display="flex"
flexDirection="column"
alignItems={ALIGNMENT[align]}
css={{ '> * + *': { pt: space } }}
>
{Children.map(
flattenChildren(children) as unknown as React.ReactElement,
(child: React.ReactElement) => {
if (as === 'div') {
ti10le marked this conversation as resolved.
Show resolved Hide resolved
return (
<Box>{React.cloneElement(child, {}, child.props.children)}</Box>
);
}
return React.cloneElement(child, {}, child.props.children);
}
)}
</Box>
);