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

Tabs is passing className to both tab-panel and tab-list #3317

Open
alecf opened this issue Jan 25, 2019 · 4 comments
Open

Tabs is passing className to both tab-panel and tab-list #3317

alecf opened this issue Jan 25, 2019 · 4 comments

Comments

@alecf
Copy link
Contributor

alecf commented Jan 25, 2019

Environment

  • Package version(s): 3.
  • Browser and OS versions: Chrome

Steps to reproduce

  1. Create a tabs component with some kind of class:
<Tabs >
  <Tab className="my-tab-component1" title="Tab 1" panel={<div>Panel 1</div>} />
  <Tab className="my-tab-component2" title="Tab 2" panel={<div>Panel 2</div>} />
  <Tab className="my-tab-component3" title="Tab 3" panel={<div>Panel 3</div>} />
</Tabs>
  1. Look at the generated DOM:
<div class="bp3-tabs my-tab-component">
  <div class="bp3-tab-list" role="tablist">
    <div class="bp3-tab-indicator-wrapper">..</div>
    <div class="bp3-tab my-tab-component1"...></div>
    <div class="bp3-tab my-tab-component2"...></div>
    <div class="bp3-tab my-tab-component3"...></div>
  </div>
  <div class="bp3-tab-panel my-tab-component1">...</div>
  <div class="bp3-tab-panel my-tab-component2">...</div>
  <div class="bp3-tab-panel my-tab-component3">...</div>
</div>

Actual behavior

className is repeated in both each tab, as well as each panel. There is no way to target just a tab, or just a panel.

This matters especially where parent/child relationships are important, such as with flexbox. In my case I want to flex the panel vertically but NOT flex the tabs...

Expected behavior

Separate className properties for the tab/panel

Possible solution

Separate className properties for the tab/panel

While it's true you could target this with CSS, I feel like other components' support for classNames of subcomponents gives you the control you want without relying quite as much on the internal structure. For example, see wrapperTagName, targetClassName and targetTagName in Popover

@giladgray
Copy link
Contributor

giladgray commented Jan 28, 2019

@alecf oof I agree with you that this is not ideal but fixing it fully would require a breaking change (to, say, replace className with panelClassName on the TabPanel) 😕

the best we can do in the short term is add a panelClassName prop in addition to the existing className behavior.

@alecf
Copy link
Contributor Author

alecf commented Jan 30, 2019

yeah, totally get the breaking change... panelClassName sounds great to me..

@n1313
Copy link
Contributor

n1313 commented Feb 7, 2019

There is no way to target just a tab, or just a panel.

@alecf I have been bitten by this before, and had to use attributes in addition to class name to target one of the two "tabs":

.tab[role='tab'] {
  /* styles for the tab */
}

.tab[role='tabpanel'] {
  /* styles for tab panel */
}

@adidahiya
Copy link
Contributor

I like @n1313's solution here. Still open to PRs to add a panelClassName prop, but it's low priority because of the reasonable workaround.

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

No branches or pull requests

4 participants