Fixing React.Fragment wrapping StackItems inside a Stack. (#21090)

Co-authored-by: Angel Duran <aduran@microsoft.com>
microsoft · Dec 22, 2021 · 36d59e3 · 36d59e3
1 parent cbf184e
commit 36d59e3
Show file tree

Hide file tree

Showing 5 changed files with 176 additions and 22 deletions.
diff --git a/change/@fluentui-react-6185695e-90a7-4334-a2c0-b2f29e60efc6.json b/change/@fluentui-react-6185695e-90a7-4334-a2c0-b2f29e60efc6.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Fixing Stack behavior when a React.Fragment wraps StackItems, it should be ignored.",
+  "packageName": "@fluentui/react",
+  "email": "aduran@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-examples/src/react/__snapshots__/Separator.Basic.Example.tsx.shot b/packages/react-examples/src/react/__snapshots__/Separator.Basic.Example.tsx.shot
@@ -443,7 +443,7 @@ exports[`Component Examples renders Separator.Basic.Example.tsx correctly 1`] =
             {
               -moz-osx-font-smoothing: grayscale;
               -webkit-font-smoothing: antialiased;
-              flex-shrink: 0;
+              flex-shrink: 1;
               font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
               font-size: 14px;
               font-weight: 400;
@@ -552,7 +552,7 @@ exports[`Component Examples renders Separator.Basic.Example.tsx correctly 1`] =
             {
               -moz-osx-font-smoothing: grayscale;
               -webkit-font-smoothing: antialiased;
-              flex-shrink: 0;
+              flex-shrink: 1;
               font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
               font-size: 14px;
               font-weight: 400;
@@ -661,7 +661,7 @@ exports[`Component Examples renders Separator.Basic.Example.tsx correctly 1`] =
             {
               -moz-osx-font-smoothing: grayscale;
               -webkit-font-smoothing: antialiased;
-              flex-shrink: 0;
+              flex-shrink: 1;
               font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
               font-size: 14px;
               font-weight: 400;
@@ -770,7 +770,7 @@ exports[`Component Examples renders Separator.Basic.Example.tsx correctly 1`] =
             {
               -moz-osx-font-smoothing: grayscale;
               -webkit-font-smoothing: antialiased;
-              flex-shrink: 0;
+              flex-shrink: 1;
               font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
               font-size: 14px;
               font-weight: 400;

diff --git a/packages/react/src/components/Stack/Stack.test.tsx b/packages/react/src/components/Stack/Stack.test.tsx
@@ -224,6 +224,19 @@ describe('Stack', () => {
     expect(tree).toMatchSnapshot();
   });
 
+  it('renders vertical Stack with StackItems inside a React.Fragment correctly', () => {
+    const component = renderer.create(
+      <Stack>
+        <>
+          <Stack.Item>Item 1</Stack.Item>
+          <Stack.Item>Item 2</Stack.Item>
+        </>
+      </Stack>,
+    );
+    const tree = component.toJSON();
+    expect(tree).toMatchSnapshot();
+  });
+
   it('renders horizontal Stack with shrinking StackItems correctly', () => {
     const component = renderer.create(
       <Stack horizontal>
@@ -235,6 +248,19 @@ describe('Stack', () => {
     expect(tree).toMatchSnapshot();
   });
 
+  it('renders horizontal Stack with StackItems inside a React.Fragment correctly', () => {
+    const component = renderer.create(
+      <Stack horizontal>
+        <>
+          <Stack.Item>Item 1</Stack.Item>
+          <Stack.Item>Item 2</Stack.Item>
+        </>
+      </Stack>,
+    );
+    const tree = component.toJSON();
+    expect(tree).toMatchSnapshot();
+  });
+
   it('renders vertical Stack with shrinking StackItems correctly', () => {
     const component = renderer.create(
       <Stack>

diff --git a/packages/react/src/components/Stack/Stack.tsx b/packages/react/src/components/Stack/Stack.tsx
@@ -18,27 +18,34 @@ const StackView: IStackComponent['view'] = props => {
     padding: 'tokens.padding',
   });
 
-  const stackChildren: (React.ReactChild | null)[] | null | undefined = React.Children.map(
-    props.children,
-    (child: React.ReactElement<IStackItemProps>, index: number) => {
-      if (!child) {
-        return null;
-      }
+  // React.Fragment needs to be ignored before checking for Stack's children
+  let stackChildren = React.Children.toArray(props.children);
+  if (
+    stackChildren.length === 1 &&
+    React.isValidElement(stackChildren[0]) &&
+    stackChildren[0].type === React.Fragment
+  ) {
+    stackChildren = stackChildren[0].props.children;
+  }
 
-      if (_isStackItem(child)) {
-        const defaultItemProps: IStackItemProps = {
-          shrink: !disableShrink,
-        };
+  stackChildren = React.Children.map(stackChildren, (child: React.ReactElement<IStackItemProps>, index: number) => {
+    if (!child) {
+      return null;
+    }
 
-        return React.cloneElement(child, {
-          ...defaultItemProps,
-          ...child.props,
-        });
-      }
+    if (_isStackItem(child)) {
+      const defaultItemProps: IStackItemProps = {
+        shrink: !disableShrink,
+      };
 
-      return child;
-    },
-  );
+      return React.cloneElement(child, {
+        ...defaultItemProps,
+        ...child.props,
+      });
+    }
+
+    return child;
+  });
 
   const nativeProps = getNativeProps<React.HTMLAttributes<HTMLDivElement>>(rest, htmlElementProperties);
 

diff --git a/packages/react/src/components/Stack/__snapshots__/Stack.test.tsx.snap b/packages/react/src/components/Stack/__snapshots__/Stack.test.tsx.snap
@@ -297,6 +297,63 @@ exports[`Stack renders horizontal Stack with StackItems correctly 1`] = `
 </div>
 `;
 
+exports[`Stack renders horizontal Stack with StackItems inside a React.Fragment correctly 1`] = `
+<div
+  className=
+      ms-Stack
+      {
+        box-sizing: border-box;
+        display: flex;
+        flex-direction: row;
+        flex-wrap: nowrap;
+        height: auto;
+        width: auto;
+      }
+      & > * {
+        text-overflow: ellipsis;
+      }
+      & > *:not(:first-child) {
+        margin-left: 0px;
+      }
+      & > *:not(.ms-StackItem) {
+        flex-shrink: 1;
+      }
+>
+  <div
+    className=
+        ms-StackItem
+        {
+          -moz-osx-font-smoothing: grayscale;
+          -webkit-font-smoothing: antialiased;
+          flex-shrink: 1;
+          font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
+          font-size: 14px;
+          font-weight: 400;
+          height: auto;
+          width: auto;
+        }
+  >
+    Item 1
+  </div>
+  <div
+    className=
+        ms-StackItem
+        {
+          -moz-osx-font-smoothing: grayscale;
+          -webkit-font-smoothing: antialiased;
+          flex-shrink: 1;
+          font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
+          font-size: 14px;
+          font-weight: 400;
+          height: auto;
+          width: auto;
+        }
+  >
+    Item 2
+  </div>
+</div>
+`;
+
 exports[`Stack renders horizontal Stack with a gap correctly 1`] = `
 <div
   className=
@@ -828,6 +885,63 @@ exports[`Stack renders vertical Stack with StackItems correctly 1`] = `
 </div>
 `;
 
+exports[`Stack renders vertical Stack with StackItems inside a React.Fragment correctly 1`] = `
+<div
+  className=
+      ms-Stack
+      {
+        box-sizing: border-box;
+        display: flex;
+        flex-direction: column;
+        flex-wrap: nowrap;
+        height: auto;
+        width: auto;
+      }
+      & > * {
+        text-overflow: ellipsis;
+      }
+      & > *:not(:first-child) {
+        margin-top: 0px;
+      }
+      & > *:not(.ms-StackItem) {
+        flex-shrink: 1;
+      }
+>
+  <div
+    className=
+        ms-StackItem
+        {
+          -moz-osx-font-smoothing: grayscale;
+          -webkit-font-smoothing: antialiased;
+          flex-shrink: 1;
+          font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
+          font-size: 14px;
+          font-weight: 400;
+          height: auto;
+          width: auto;
+        }
+  >
+    Item 1
+  </div>
+  <div
+    className=
+        ms-StackItem
+        {
+          -moz-osx-font-smoothing: grayscale;
+          -webkit-font-smoothing: antialiased;
+          flex-shrink: 1;
+          font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
+          font-size: 14px;
+          font-weight: 400;
+          height: auto;
+          width: auto;
+        }
+  >
+    Item 2
+  </div>
+</div>
+`;
+
 exports[`Stack renders vertical Stack with a gap correctly 1`] = `
 <div
   className=