Repository: marigold-ui/marigold
Pull request #2463 (2022-09-15 14:37:53)

refa: Improve behavior of `<Stack>` and remove option to render it as a list (use `<List>` instead)

sebald: @lauraNawrotzki @michael-p this should make your life much easier 🙃🤞

Patch:
diff --git a/.changeset/strong-buses-jog.md b/.changeset/strong-buses-jog.md
new file mode 100644
index 000000000..16b900234
--- /dev/null
+++ b/.changeset/strong-buses-jog.md
@@ -0,0 +1,9 @@
+---
+"@marigold/docs": patch
+"@marigold/components": major
+---
+
+refa: Improve behavior of `<Stack>` and remove option to render it as a list (use `<List>` instead)
+
+- `<Stack>` will no longer align items by default, since this will cause children to not take all the available space
+- `<Stack as="ul|ol">` will no longer work, we have a `<List>` comopnent for that
diff --git a/docs/src/pages/components/stack.mdx b/docs/src/pages/components/stack.mdx
index 53d85d8f9..7f26ac9b0 100644
--- a/docs/src/pages/components/stack.mdx
+++ b/docs/src/pages/components/stack.mdx
@@ -23,15 +23,15 @@ Layout component to stack elements. This component uses the spaces from the give
     },
     {
       property: 'alignX',
-      type: '`left, right, center`',
+      type: '`none, left, right, center`',
       description: '',
-      default: 'left',
+      default: 'none',
     },
     {
       property: 'alignY',
-      type: '`top, bottom, center`',
+      type: '`none, top, bottom, center`',
       description: '',
-      default: 'center',
+      default: 'none',
     },
     {
       property: 'stretch',
diff --git a/packages/components/src/Stack/Stack.stories.tsx b/packages/components/src/Stack/Stack.stories.tsx
index 3d399ac8e..15f065f47 100644
--- a/packages/components/src/Stack/Stack.stories.tsx
+++ b/packages/components/src/Stack/Stack.stories.tsx
@@ -1,7 +1,6 @@
 import React, { ReactNode } from 'react';
 import type { Meta, ComponentStory } from '@storybook/react';
 import { Box } from '@marigold/system';
-import { shadow } from '@marigold/tokens';
 
 import { Headline } from '../Headline';
 import { Text } from '../Text';
@@ -30,14 +29,14 @@ export default {
       control: {
         type: 'select',
       },
-      options: ['left', 'center', 'right'],
+      options: ['none', 'left', 'center', 'right'],
       description: 'Vertical Alignment',
     },
     alignY: {
       control: {
         type: 'select',
       },
-      options: ['top', 'center', 'bottom'],
+      options: ['none', 'top', 'center', 'bottom'],
       description: 'Vertical Alignment',
     },
     stretch: {
@@ -45,11 +44,24 @@ export default {
         type: 'boolean',
       },
       description:
-        'Stretch to fill space (useful if you want to change y alignment)',
+        'Stretch to fill space (vertical AND horizontal, useful if you want to change y alignment)',
     },
   },
 } as Meta;
 
+const Block = ({ children }: { children: ReactNode }) => (
+  <Box
+    css={{
+      background: 'hsla(218 16% 77% / 50%)',
+      border: '1px solid hsla(218 16% 70% / 50%)',
+      borderRadius: 12,
+      p: 12,
+    }}
+  >
+    {children}
+  </Box>
+);
+
 export const Basic: ComponentStory<typeof Stack> = args => (
   <Stack {...args}>
     <Headline level="2">Heading</Headline>
@@ -69,77 +81,62 @@ export const Basic: ComponentStory<typeof Stack> = args => (
 );
 
 export const Nested: ComponentStory<typeof Stack> = args => (
-  <Stack {...args}>
-    <Stack space="xsmall">
-      <Headline level="2">Heading</Headline>
-      <Text>
-        Part 1: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-        Suspendisse dignissim dapibus elit, vel egestas felis pharetra non. Cras
-        malesuada, massa nec ultricies efficitur, lectus ante consequat magna, a
-        porttitor massa ex ut quam.
-      </Text>
-      <Text>
-        Part 2: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-        Suspendisse dignissim dapibus elit, vel egestas felis pharetra non. Cras
-        malesuada, massa nec ultricies efficitur, lectus ante consequat magna, a
-        porttitor massa ex ut quam.
-      </Text>
-    </Stack>
-    <Stack space="xsmall">
-      <Headline level="2">Heading</Headline>
-      <Text>
-        Part 1: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-        Suspendisse dignissim dapibus elit, vel egestas felis pharetra non. Cras
-        malesuada, massa nec ultricies efficitur, lectus ante consequat magna, a
-        porttitor massa ex ut quam.
-      </Text>
-      <Text>
-        Part 2: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
-        Suspendisse dignissim dapibus elit, vel egestas felis pharetra non. Cras
-        malesuada, massa nec ultricies efficitur, lectus ante consequat magna, a
-        porttitor massa ex ut quam.
-      </Text>
-    </Stack>
-  </Stack>
-);
-
-export const List: ComponentStory<typeof Stack> = args => (
-  <Stack space="large">
-    <Stack as="ol" {...args}>
-      <li>one</li>
-      <li>two</li>
-      <li>three</li>
-    </Stack>
-    <Stack as="ul" {...args}>
-      <li>one</li>
-      <li>two</li>
-      <li>three</li>
+  <Block>
+    <Stack {...args}>
+      <Stack space="xsmall">
+        <Block>
+          <Headline level="2">With xsmall spacing</Headline>
+        </Block>
+        <Block>
+          <Text>
+            Part 1: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
+            Suspendisse dignissim dapibus elit, vel egestas felis pharetra non.
+            Cras malesuada, massa nec ultricies efficitur, lectus ante consequat
+            magna, a porttitor massa ex ut quam.
+          </Text>
+        </Block>
+        <Block>
+          <Text>
+            Part 2: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
+            Suspendisse dignissim dapibus elit, vel egestas felis pharetra non.
+            Cras malesuada, massa nec ultricies efficitur, lectus ante consequat
+            magna, a porttitor massa ex ut quam.
+          </Text>
+        </Block>
+      </Stack>
+      <Stack space="medium">
+        <Block>
+          <Headline level="2">With Medium Spacing</Headline>
+        </Block>
+        <Block>
+          <Text>
+            Part 1: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
+            Suspendisse dignissim dapibus elit, vel egestas felis pharetra non.
+            Cras malesuada, massa nec ultricies efficitur, lectus ante consequat
+            magna, a porttitor massa ex ut quam.
+          </Text>
+        </Block>
+        <Block>
+          <Text>
+            Part 2: Lorem ipsum dolor sit amet, consectetur adipiscing elit.
+            Suspendisse dignissim dapibus elit, vel egestas felis pharetra non.
+            Cras malesuada, massa nec ultricies efficitur, lectus ante consequat
+            magna, a porttitor massa ex ut quam.
+          </Text>
+        </Block>
+      </Stack>
     </Stack>
-  </Stack>
-);
-
-const Block = ({ children }: { children: ReactNode }) => (
-  <Box
-    css={{
-      border: '1px solid #364fc7',
-      borderRadius: 16,
-      bg: '#4263eb',
-      color: '#edf2ff',
-      px: 32,
-      py: 12,
-      boxShadow: shadow['medium-1'],
-    }}
-  >
-    {children}
-  </Box>
+  </Block>
 );
 
 export const Stretch: ComponentStory<typeof Stack> = args => (
-  <Box css={{ height: 300 }}>
-    <Stack {...args}>
-      <Block>Lirum</Block>
-      <Block>Larum</Block>
-      <Block>Löffelstiel!</Block>
-    </Stack>
-  </Box>
+  <Block>
+    <Box css={{ height: 300 }}>
+      <Stack {...args}>
+        <Block>Lirum</Block>
+        <Block>Larum</Block>
+        <Block>Löffelstiel!</Block>
+      </Stack>
+    </Box>
+  </Block>
 );
diff --git a/packages/components/src/Stack/Stack.tsx b/packages/components/src/Stack/Stack.tsx
index 8266dc9e0..69caab6b8 100755
--- a/packages/components/src/Stack/Stack.tsx
+++ b/packages/components/src/Stack/Stack.tsx
@@ -2,13 +2,16 @@ import React, { ReactNode } from 'react';
 import { ResponsiveStyleValue } from '@marigold/system';
 
 import { Box } from '../Box';
+
 const ALIGNMENT_X = {
+  none: 'initial',
   left: 'flex-start',
   center: 'center',
   right: 'flex-end',
 };
 
 const ALIGNMENT_Y = {
+  none: 'initial',
   top: 'flex-start',
   center: 'center',
   bottom: 'flex-end',
@@ -17,7 +20,6 @@ const ALIGNMENT_Y = {
 // Props
 // ---------------
 export interface StackProps {
-  as?: 'div' | 'ul' | 'ol';
   children?: ReactNode;
   space?: ResponsiveStyleValue<string>;
   alignX?: keyof typeof ALIGNMENT_X;
@@ -30,8 +32,8 @@ export interface StackProps {
 export const Stack = ({
   children,
   space = 'none',
-  alignX = 'left',
-  alignY = 'top',
+  alignX = 'none',
+  alignY = 'none',
   stretch = false,
   ...props
 }: StackProps) => (
@@ -43,7 +45,7 @@ export const Stack = ({
       gap: space,
       alignItems: ALIGNMENT_X[alignX],
       justifyContent: ALIGNMENT_Y[alignY],
-      blockSize: stretch ? '100%' : 'auto',
+      blockSize: stretch ? '100%' : 'initial',
     }}
     {...props}
   >


Test patch:
diff --git a/packages/components/src/Stack/Stack.test.tsx b/packages/components/src/Stack/Stack.test.tsx
index d7bb8f347..f0f263698 100644
--- a/packages/components/src/Stack/Stack.test.tsx
+++ b/packages/components/src/Stack/Stack.test.tsx
@@ -30,7 +30,7 @@ test('default space is "none"', () => {
   expect(first).toHaveStyle(`gap: 0`);
 });
 
-test('accepts and uses spacing from theme', () => {
+test('uses spacing from theme', () => {
   render(
     <ThemeProvider theme={theme}>
       <Stack space="small">
@@ -43,13 +43,23 @@ test('accepts and uses spacing from theme', () => {
   expect(first).toHaveStyle(`gap: 2px`);
 });
 
-test('align children left by default', () => {
+test('children are not aligned by default', () => {
   render(
     <Stack data-testid="stack">
       <Text>first</Text>
     </Stack>
   );
   const stack = screen.getByTestId('stack');
+  expect(stack).toHaveStyle(`align-items: initial`);
+});
+
+test('allows to align children to the left', () => {
+  render(
+    <Stack alignX="left" data-testid="stack">
+      <Text>first</Text>
+    </Stack>
+  );
+  const stack = screen.getByTestId('stack');
   expect(stack).toHaveStyle(`align-items: flex-start`);
 });
 
@@ -73,13 +83,23 @@ test('allows to align children to the right', () => {
   expect(stack).toHaveStyle(`align-items: flex-end`);
 });
 
-test('children to the top by default', () => {
+test('children are not aligned vertically by default', () => {
   render(
     <Stack data-testid="stack">
       <Text>first</Text>
     </Stack>
   );
   const stack = screen.getByTestId('stack');
+  expect(stack).toHaveStyle(`justify-content: initial`);
+});
+
+test('allows to align children to the vertical top', () => {
+  render(
+    <Stack alignY="top" data-testid="stack">
+      <Text>first</Text>
+    </Stack>
+  );
+  const stack = screen.getByTestId('stack');
   expect(stack).toHaveStyle(`justify-content: flex-start`);
 });
 
@@ -110,7 +130,7 @@ test('behaves as inline be default', () => {
     </Stack>
   );
   const stack = screen.getByTestId('stack');
-  expect(stack).toHaveStyle(`blockSize: auto`);
+  expect(stack).toHaveStyle(`blockSize: initial`);
 });
 
 test('allows to fill space', () => {
@@ -162,31 +182,3 @@ test('renders as div per default', () => {
   const stack = screen.getByTestId('stack');
   expect(stack instanceof HTMLDivElement).toBeTruthy();
 });
-
-test('can render as "ul"', () => {
-  render(
-    <ThemeProvider theme={theme}>
-      <Stack as="ul" data-testid="stack">
-        <Text>first</Text>
-        <Text>second</Text>
-      </Stack>
-    </ThemeProvider>
-  );
-
-  const stack = screen.getByTestId('stack');
-  expect(stack instanceof HTMLUListElement).toBeTruthy();
-});
-
-test('can render as "ol"', () => {
-  render(
-    <ThemeProvider theme={theme}>
-      <Stack as="ol" data-testid="stack">
-        <Text>first</Text>
-        <Text>second</Text>
-      </Stack>
-    </ThemeProvider>
-  );
-
-  const stack = screen.getByTestId('stack');
-  expect(stack instanceof HTMLOListElement).toBeTruthy();
-});