Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

styled-components don't work with Tab.js #148

Open
mschipperheyn opened this issue Dec 5, 2016 · 12 comments
Open

styled-components don't work with Tab.js #148

mschipperheyn opened this issue Dec 5, 2016 · 12 comments

Comments

@mschipperheyn
Copy link

It's not possible to customize Tab.js with styled-components (https://github.com/styled-components/styled-components) because of this line in Tab.js:

if (child.type !== _Tab2.default) {
  return child;
}

if you customize the styling with styled-components,

const StyleTab = styled(Tab)`
    background-color : ${props => props.selected? 'white' : 'transparent'};
`;

child.type will no longer be Tab but StyleTab

I also don't understand why this check is here?

@joepvl
Copy link
Collaborator

joepvl commented Dec 5, 2016

Hm, that's unfortunate. I believe the check is there is to facilitate using other types of component inside of <TabList>, and it could possibly have something to do with nesting <Tabs> inside <TabPanel>s (don't quote me on this though). To work around this, would it work for you to forego styling the <Tab> itself, and styling its contents instead? E.g.:

const TabContents = props => <div>{props.title}</div>;

const StyledTabContents = styled(TabContents)`
  background-color: ${props => props.selected? 'white' : 'transparent'};
`;

const ComponentWithStyledTab = props =>
  <Tabs>
    <TabList><Tab><StyledTabContents title="Tab 1" /></Tab></TabList>
    <TabPanel>TabPanel 1</TabPanel>
  </Tabs>
;

Off on a tangent here: I think @alexanderwallin's idea from his comment in #131 about providing composer functions to "wrap your custom components inside higher-order tab components" could help in solving issues like this, with "like this" meaning "related to the static nature of the current implementation & API". Maybe react-tabs will move in this direction at some point. OTOH, the API would be very different, and perhaps the difference in design goals means that a separate project that provides tab behaviour but doesn't prescribe tab markup or style would be warranted. In fact, going beyond that, one could build such a project and then build the current API on top of that!

I am left to wonder where my thoughts are going with this though, because part of what makes React attractive to me is the ease of colocating view template and view logic :). I guess in a library context (as opposed to userland code) this direction is warranted (or even to be encouraged at times) though.

@mschipperheyn
Copy link
Author

I ended up going for a solution where I overrode the classes in a central css file. Works also. Not quite as elegant. Styling the contents is not an option when you want adjust e.g. The border color

@danez
Copy link
Collaborator

danez commented Sep 19, 2017

This is fixed in 2.0.0 and should work out of the box now

@danez danez closed this as completed Sep 19, 2017
@pzmudzinski
Copy link

It does not seem to. StyledTab isn't recognized by TabList.

Example:

const StyledTab = styled(Tab)`
    padding: 18px;
   // ...
`;

render method:

<Tabs>
  <TabList>
     <StyledTab>Tab1</StyledTab>
     <StyledTab>Tab2</StyledTab>
   </TabList>

   <TabPanel>
      <h2>Any content 1</h2>
    </TabPanel>
   <TabPanel>
     <h2>Any content 2</h2>
    </TabPanel>
</Tabs>

causes "Failed prop type: There should be an equal number of 'Tab' and 'TabPanel' in Tabs.Received 0 'Tab' and 2 'TabPanel'."

Tabs aren't working (clicking on them do not cause any effect).

@danez ^

@danez danez reopened this Sep 20, 2017
@AlexHenkel
Copy link

@pzmudzinski I had the same problem and my workaround is to add tabsRole = 'Tab' to the component, like this:

const StyledTab = styled(Tab)`
  background: red;
`
StyledTab.tabsRole = 'Tab';

The only function that I could trace that fails, is this:

export function isTab(el) {
  return el.type && el.type.tabsRole === 'Tab';
}

So when you add the role, it should work as expected. Hope it helps

@pzmudzinski
Copy link

Thanks @AlexHenkel. Wondering why there's need to check type.tabsRole at all.

@NathanielHill
Copy link

This is still failing for me. @AlexHenkel work-around does the trick, however I still get a warning when styling the TabList:

Failed prop type: Found a 'Tab' component outside of the 'TabList' component.

@bbenezech
Copy link

I had great success with this:

import styled from 'styled-components';
import {
  Tab as UnstyledTab,
  TabList as UnstyledTabList,
  Tabs as UnstyledTabs,
  TabPanel as UnstyledTabPanel
} from 'react-tabs';

const Tabs = styled(UnstyledTabs)`
  border: 1px solid #e0e0e0;
  background: white;
`;

const TabList = styled(UnstyledTabList)`
  display: flex;
  justify-content: space-between;
  flex-wrap: wrap;
  padding: 0;
  margin: 0;
`;

const Tab = styled(UnstyledTab).attrs({
  selectedClassName: 'selected',
  disabledClassName: 'disabled'
})`
  flex-grow: 1;
  text-align: center;
  padding: 12px 0;
  list-style: none;
  cursor: pointer;
  color: #888;
  border-left: 1px solid #e0e0e0;
  border-bottom: 1px solid #e0e0e0;

  &:first-child {
    border-left: none;
  }

  &.selected {
    color: #0097ff;
    border-bottom: none;
  }

  &.disabled {
    color: #e0e0e0;
    cursor: not-allowed;
  }
`;

const TabPanel = styled(UnstyledTabPanel).attrs({ selectedClassName: 'selected' })`
  display: none;
  padding: 10px 20px;
  &.selected {
    display: block;
  }
`;

Tab.tabsRole = 'Tab';
Tabs.tabsRole = 'Tabs';
TabPanel.tabsRole = 'TabPanel';
TabList.tabsRole = 'TabList';

export { Tab, TabList, Tabs, TabPanel };

@Ericnr
Copy link

Ericnr commented Jun 27, 2019

The above solution seems to work until you try to add props to dynamically style the tabs. I've tried

const MiniTab = styled(Tab).attrs({
  selectedClassName: 'selected',
  disabledClassName: 'disabled'
})`
  display: flex;
  align-items: center;
  justify-content: center;
  height: 34px;
  font-size: 13px;
  text-align: center;
  border-bottom: 2px solid transparent;
  cursor: pointer;
  transition: border-color 200ms, color 200ms;
  color: gray;
  padding: 0 6px;

  &:hover {
    color: black;
  }

  &.selected {
    border-color: ${props => props.borderColor === "blue" ? "#1664a2 !important" : "#9e2d2d !important"}
    color: ${props => props.borderColor === "blue" ? "#1664a2 !important" : "#9e2d2d !important"}
  }

  -moz-user-select: none;
  -khtml-user-select: none;
  -webkit-user-select: none;
  -ms-user-select: none;
  user-select: none;
`;

MiniTab.tabsRole = 'Tab';

And I get the error React does not recognize the borderColor prop on a DOM element.
It seems like every prop passed to Tab is being passed down to the underlying li, which doesn't seem right?

@matviishyn
Copy link

@Ericnr Looks like it works now!

@woro83c
Copy link

woro83c commented Aug 16, 2019

I'm not sure if there's a styled-components equivalent, but I was able to get around this warning by using Emotion and its prop forwarding:

https://emotion.sh/docs/styled#customizing-prop-forwarding

So, basically:

// Only allow children to be passed down to the DOM
const MyTab = styled(Tab, { shouldForwardProp: prop => prop === 'children' })();

@kevinRoomies
Copy link

@pzmudzinski I had the same problem and my workaround is to add tabsRole = 'Tab' to the component, like this:

const StyledTab = styled(Tab)`
  background: red;
`
StyledTab.tabsRole = 'Tab';

The only function that I could trace that fails, is this:

export function isTab(el) {
  return el.type && el.type.tabsRole === 'Tab';
}

So when you add the role, it should work as expected. Hope it helps

I was using this until I updated the module 2 days ago and now it's broken and I can't get it to work anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests