Repository: react-materialize/react-materialize
Pull request #1168 (2020-12-15 10:12:10)

Allow for Dropdown children to also be Fragments.
# Description

This is a followup to #1164 which addresses the issue with Fragments in dropdowns, as discussed in #1167. Now, the code will recurse on fragments, rendering each properly into their own `<li>`. This retains the functionality of allowing null and undefined components.

The previous PR broke on Fragments because in the example in the issue, it was presented as a single-item child and the code assumed `children` would only ever be an array. **That PR will also break on other single-item dropdowns!**

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)

# How Has This Been Tested?

Test with components similar to these:

```jsx
// Multi-element with boolean values, Divider, and li test:
<Dropdown>
  { false && <a>Do Not Show Me</a> }
  <a>Show Me</a>
  <Divider/>
  <li><a>No Additional Wrapper</a></li>
</Dropdown>

// Single element render:
<Dropdown>
  <Divider/>
</Dropdown>

// Fragment, multi-element:
<Dropdown>
  <React.Fragment>
    <a>One</a>
    <a>Two</a>
  </React.Fragment>
  <a>Three</a>
</Dropdown>

// Fragment, only child:
<Dropdown>
  <React.Fragment>
    <a>Fragment</a>
  </React.Fragment>
</Dropdown>
```

# Checklist:

- [x] I have commented my code, particularly in hard-to-understand areas
- [x] My changes generate no new warnings
- [x] I have not generated a new package version. (the maintainers will handle that)



Patch:
diff --git a/src/Dropdown.js b/src/Dropdown.js
index 4e2acd5..14d80f7 100644
--- a/src/Dropdown.js
+++ b/src/Dropdown.js
@@ -29,16 +29,22 @@ const Dropdown = ({ children, className, trigger, options, ...props }) => {
       className: cx(trigger.props.className, 'dropdown-trigger')
     });
 
-  const renderItems = () =>
-    Children.map((children || []).filter(Boolean), element => {
-      if (element.type === 'li') {
+  const renderItems = c =>
+    Children.map(c, element => {
+      if (!element || element.type === 'li') {
+        // Skip null, undefined, or li children
         return element;
+      } else if (element.type === Fragment) {
+        // Recurse here for Fragments
+        return renderItems(element.props.children);
       } else if (
         element.type.name === 'Divider' ||
         element.type.displayName === 'Divider'
       ) {
+        // Replace <Divider/> component with proper semantics
         return <li key={idgen()} className="divider" tabIndex="-1" />;
       } else {
+        // Wrap child element in <li/>
         return <li key={idgen()}>{element}</li>;
       }
     });
@@ -51,7 +57,7 @@ const Dropdown = ({ children, className, trigger, options, ...props }) => {
         className={cx('dropdown-content', className)}
         ref={_dropdownContent}
       >
-        {renderItems()}
+        {renderItems(children)}
       </ul>
     </Fragment>
   );


Test patch:
diff --git a/test/DropdownButton.spec.js b/test/DropdownButton.spec.js
index 0552e68..9569a98 100644
--- a/test/DropdownButton.spec.js
+++ b/test/DropdownButton.spec.js
@@ -24,6 +24,54 @@ describe('<Dropdown />', () => {
     expect(wrapper.contains(<li className="divider" tanIndex="-1" />));
   });
 
+  test('renders inline conditionals, normal links, and li nodes', () => {
+    // noinspection PointlessBooleanExpressionJS
+    wrapper = shallow(
+      <Dropdown trigger={<span>hi</span>}>
+        {false && <a href={'#'}>Do Not Show Me</a>}
+        <a href={'#'}>Show Me</a>
+        <Divider />
+        <li>
+          <a href={'#'}>No Additional Wrapper</a>
+        </li>
+      </Dropdown>
+    );
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  test('renders a single element', () => {
+    wrapper = shallow(
+      <Dropdown trigger={<span>hi</span>}>
+        <a href={'#'}>I'm a Link</a>
+      </Dropdown>
+    );
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  test('renders fragments with other children', () => {
+    wrapper = shallow(
+      <Dropdown trigger={<span>hi</span>}>
+        <React.Fragment>
+          <a href={'#'}>One</a>
+          <a href={'#'}>Two</a>
+        </React.Fragment>
+        <a href={'#'}>Three</a>
+      </Dropdown>
+    );
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  test('renders a single-child fragment', () => {
+    wrapper = shallow(
+      <Dropdown trigger={<span>hi</span>}>
+        <React.Fragment>
+          <a href={'#'}>Fragment</a>
+        </React.Fragment>
+      </Dropdown>
+    );
+    expect(wrapper).toMatchSnapshot();
+  });
+
   test('deletes unwatend props', () => {
     const options = { hover: true };
     wrapper = shallow(<Dropdown trigger={<span>hi</span>} options={options} />);
diff --git a/test/__snapshots__/DropdownButton.spec.js.snap b/test/__snapshots__/DropdownButton.spec.js.snap
index c0d330e..27c4928 100644
--- a/test/__snapshots__/DropdownButton.spec.js.snap
+++ b/test/__snapshots__/DropdownButton.spec.js.snap
@@ -14,3 +14,135 @@ exports[`<Dropdown /> renders 1`] = `
   />
 </Fragment>
 `;
+
+exports[`<Dropdown /> renders a single element 1`] = `
+<Fragment>
+  <span
+    className="dropdown-trigger"
+    data-target="Dropdown_mockId"
+  >
+    hi
+  </span>
+  <ul
+    className="dropdown-content"
+    id="Dropdown_mockId"
+  >
+    <li
+      key="mockId/.0"
+    >
+      <a
+        href="#"
+      >
+        I'm a Link
+      </a>
+    </li>
+  </ul>
+</Fragment>
+`;
+
+exports[`<Dropdown /> renders a single-child fragment 1`] = `
+<Fragment>
+  <span
+    className="dropdown-trigger"
+    data-target="Dropdown_mockId"
+  >
+    hi
+  </span>
+  <ul
+    className="dropdown-content"
+    id="Dropdown_mockId"
+  >
+    <li
+      key=".0/.$mockId/.0"
+    >
+      <a
+        href="#"
+      >
+        Fragment
+      </a>
+    </li>
+  </ul>
+</Fragment>
+`;
+
+exports[`<Dropdown /> renders fragments with other children 1`] = `
+<Fragment>
+  <span
+    className="dropdown-trigger"
+    data-target="Dropdown_mockId"
+  >
+    hi
+  </span>
+  <ul
+    className="dropdown-content"
+    id="Dropdown_mockId"
+  >
+    <li
+      key=".0/.$mockId/.0"
+    >
+      <a
+        href="#"
+      >
+        One
+      </a>
+    </li>
+    <li
+      key=".0/.$mockId/.1"
+    >
+      <a
+        href="#"
+      >
+        Two
+      </a>
+    </li>
+    <li
+      key="mockId/.1"
+    >
+      <a
+        href="#"
+      >
+        Three
+      </a>
+    </li>
+  </ul>
+</Fragment>
+`;
+
+exports[`<Dropdown /> renders inline conditionals, normal links, and li nodes 1`] = `
+<Fragment>
+  <span
+    className="dropdown-trigger"
+    data-target="Dropdown_mockId"
+  >
+    hi
+  </span>
+  <ul
+    className="dropdown-content"
+    id="Dropdown_mockId"
+  >
+    <li
+      key="mockId/.1"
+    >
+      <a
+        href="#"
+      >
+        Show Me
+      </a>
+    </li>
+    <li
+      className="divider"
+      key="mockId/.2"
+      tabIndex="-1"
+    />
+    <li
+      key=".3"
+    >
+      <a
+        href="#"
+      >
+        No Additional Wrapper
+      </a>
+    </li>
+  </ul>
+</Fragment>
+`;