fix(Stack): flex prop by default false

kiwicom · Oct 25, 2023 · e68297b · e68297b
1 parent e5c2d65
commit e68297b
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 17 deletions.
diff --git a/packages/orbit-components/src/Stack/README.md b/packages/orbit-components/src/Stack/README.md
@@ -28,9 +28,9 @@ The table below contains all types of props available in the Stack component.
 | dataTest     | `string`                   |            | Optional prop for testing purposes.                                                                                                                            |
 | desktop      | [`Object`](#media-queries) |            | Object for setting up properties for the desktop viewport. [See Media queries](#media-queries)                                                                 |
 | direction    | [`enum`](#enum)            |            | The `flex-direction` of the Stack. [See Functional specs](#functional-specs)                                                                                   |
-| flex         | `boolean`                  | `true`     | If `true` or you specify some flex attribute, the Stack will use flex attributes.                                                                              |
+| flex         | `boolean`                  | `false`    | If `true` or you specify some flex attribute, the Stack will use flex attributes.                                                                              |
 | grow         | `boolean`                  | `true`     | If `true`, the Stack will have `flex-grow` set to `1`, otherwise it will be `0`.                                                                               |
-| inline       | `boolean`                  | `false`    | If `true`, the Stack will have `display` set to `inline-flex`, otherwise it will be `flex`.                                                                    |
+| inline       | `boolean`                  | `false`    | If `true`, the Stack will have `display` set to `inline-flex`.                                                                                                 |
 | justify      | [`enum`](#enum)            | `"start"`  | The `justify-content` of the Stack.                                                                                                                            |
 | largeDesktop | [`Object`](#media-queries) |            | Object for setting up properties for the largeDesktop viewport. [See Media queries](#media-queries)                                                            |
 | largeMobile  | [`Object`](#media-queries) |            | Object for setting up properties for the largeMobile viewport. [See Media queries](#media-queries)                                                             |

diff --git a/packages/orbit-components/src/Stack/__tests__/index.test.tsx b/packages/orbit-components/src/Stack/__tests__/index.test.tsx
@@ -42,7 +42,7 @@ describe("Stack", () => {
 
   it("should turn off grow", () => {
     render(
-      <Stack grow={false} dataTest="test">
+      <Stack flex grow={false} dataTest="test">
         <div>kek</div>
         <div>bur</div>
       </Stack>,
@@ -51,9 +51,21 @@ describe("Stack", () => {
     expect(screen.getByTestId("test")).toHaveStyle(`flex-grow: 0`);
   });
 
+  it("should be block with spacing", () => {
+    render(
+      <Stack spacing="XLarge" dataTest="test">
+        <div>1</div>
+        <div>2</div>
+      </Stack>,
+    );
+
+    expect(screen.getByTestId("test")).toHaveStyle(`display: block`);
+    expect(screen.getByTestId("test")).toHaveClass("space-y-xl space-x-none block w-full");
+  });
+
   it("should turn on shrink", () => {
     render(
-      <Stack shrink dataTest="test">
+      <Stack flex shrink dataTest="test">
         <div>kek</div>
         <div>bur</div>
       </Stack>,

diff --git a/packages/orbit-components/src/Stack/index.tsx b/packages/orbit-components/src/Stack/index.tsx
@@ -6,7 +6,7 @@ import cx from "clsx";
 import type { Props, CommonProps, Direction, Spacing } from "./types";
 import type * as Common from "../common/types";
 import {
-  getDisplayInlineClass,
+  getDisplayClasses,
   getDirectionClasses,
   getAlignItemsClasses,
   getWrapClasses,
@@ -29,6 +29,13 @@ const shouldUseFlex = (props: CommonProps & Common.SpaceAfter) =>
     .map(prop => ["spacing", "spaceAfter", "dataTest", "children"].includes(prop))
     .includes(false);
 
+// use margins instead of gap, works with block
+const shouldUseLegacy = (props: CommonProps & Common.SpaceAfter) => {
+  if (props.legacy) return true;
+  if (!shouldUseFlex(props) && Boolean(props.spacing)) return true;
+  return false;
+};
+
 const Stack = (props: Props) => {
   const {
     children,
@@ -45,7 +52,7 @@ const Stack = (props: Props) => {
   const viewportProps = { mediumMobile, largeMobile, tablet, desktop, largeDesktop };
 
   const defaultMediaProps = React.useMemo(() => {
-    const isFlex = shouldUseFlex(props) || props.inline;
+    const isFlex = shouldUseFlex(props);
 
     const {
       spacing = SPACING.medium,
@@ -55,8 +62,8 @@ const Stack = (props: Props) => {
       justify = JUSTIFY.START,
       shrink = false,
       wrap = false,
-      flex = true,
-      legacy = false,
+      flex,
+      legacy = shouldUseLegacy({ ...props, spacing }),
       align = ALIGN.START,
       inline = false,
     } = props;
@@ -132,17 +139,20 @@ const Stack = (props: Props) => {
 
     return cx(
       typeof spaceAfter !== "undefined" && getSpaceAfterClasses(spaceAfter, viewport),
-      typeof align !== "undefined" && getAlignItemsClasses(align, viewport),
-      typeof align !== "undefined" && getAlignContentClasses(align, viewport),
-      typeof wrap !== "undefined" && getWrapClasses(wrap, viewport),
-      typeof grow !== "undefined" && getGrowClasses(grow, viewport),
-      typeof shrink !== "undefined" && getShrinkClasses(shrink, viewport),
-      typeof justify !== "undefined" && getJustifyClasses(justify, viewport),
-      getDirectionClasses(direction, viewport),
+      flex || inline
+        ? [
+            getDisplayClasses(inline ? "inline-flex" : "flex", viewport),
+            typeof align !== "undefined" && getAlignItemsClasses(align, viewport),
+            typeof align !== "undefined" && getAlignContentClasses(align, viewport),
+            typeof wrap !== "undefined" && getWrapClasses(wrap, viewport),
+            typeof grow !== "undefined" && getGrowClasses(grow, viewport),
+            typeof shrink !== "undefined" && getShrinkClasses(shrink, viewport),
+            typeof justify !== "undefined" && getJustifyClasses(justify, viewport),
+            getDirectionClasses(direction, viewport),
+          ]
+        : "block",
       getSpacingClasses(spacing, viewport, direction, legacy),
-      inline && getDisplayInlineClass(inline, viewport),
       inline === false && "w-full",
-      flex && "flex",
     );
   };