Allow other components inside TabList

[patrick91]

Allows to add additional elements inside a TabList.
I've disabled the check that was throwing a warning when an element different from Tab was added to the TabList. Seems working well.

I needed to do this since I need to add a fake tab at the end of the TabList, like this:

<Tabs>
  <TabList>
    <Tab>Foo</Tab>
    <div>+</div>
  </TabList>

  <TabPanel>Hello</TabPanel>
</Tabs>

Maybe we can do the same for TabPanels, if needed.
Let me know your thoughts :)

[danez]

Nice, I think we could also optimize this and if we don't need to not clone the element. Because with this approach right now it would clone the <span> or whatever is supplied besides <tab> although not really necessary. (Correct me if I'm wrong)

[patrick91]

Any idea on how to test this? I've tried with:

it('should not clone non tabs element', () => {
    const plus = <div>+</div>;

    const wrapper = mount(<Tabs>
    <TabList>
        <Tab>Foo</Tab>
        {plus}
    </TabList>

    <TabPanel>Hello Baz</TabPanel>
    </Tabs>);

    expect(wrapper.childAt(0).childAt(1) === plus).toBe(true);
});

but it is not working

[danez]

I'm not sure how to do this. childAt() returns a ReactWrapper and not the instance so they will be always different. And it doesn't seem that enzyme offers a way to retrieve the instance of children currently.

I asked on gitter, lets see if I can get an answer there.

[danez]

This would still clone everytime.

    if (child.type.displayName === 'Tab') {
      return React.cloneElement(child, {
        activeTabClassName: props.activeTabClassName,
        disabledTabClassName: props.disabledTabClassName,
      });
    }

    return child;

[patrick91]

Thanks! Don't know why I did that :D

Anyway, now it should be ok! In which gitter chat did you ask the question?

[danez]

It is this one: https://gitter.im/airbnb/enzyme

[patrick91]

@danez can you check the last commit?

I've been digging into the test suite of the cloneElement function and I got inspiration from the ref test: https://github.com/facebook/react/blob/2d74280679e17d33ac25d3e24c860bfffa9ae661/src/isomorphic/classic/element/__tests__/ReactElementClone-test.js#L102

Since we are cloning elements overriding the ref I've tried to make a test where I check that the original ref attribute is preserved. Seems working fine :)

EDIT: can I disable react/no-multi-comp for the test file?

[danez]

Awesome work, sorry for the late reply. Good to merge, just two more small things.

Thank you.

[danez]

Was there a reason to use the displayName. If not I think it would be better to do child.type !== Type and import Tab from './Tab' at the top.

[danez]

Same here: tab.type === Tab

[danez]

Nice to figure out how to test this 👍
You can just add /* eslint-disable react/no-multi-comp */ at the top of the test to disable the lint.

[patrick91]

Done!

[masad-frost]

Just FYI, you should avoid divs and other elements inside TabList, the HTML spec only permits li elements inside ul.

Thanks for this btw.