feat(theme-classic): allow passing additional attributes to tab headings

[Drylozu]

Motivation

It's nice that we can customize the tabs of Tabs component with CSS or just to be able to differentiate them (in CSS you can't select elements by their inner HTML). And I think this is the best way to be able to do this because if we use className attribute, Tabs and TabItem components already have one on it and the one from TabItem component places it to it's content and not it's tab.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

This adds a data-value attribute to the Tabs component, knowing that they must be unique there should be no problem.

We should be able to style a specific tab whether it's active or not, using the aria-selected attribute or tabs__item--active class:

There are infinite possibilities, designs and styles. For example, being able to place a red color in the Apples tab and an orange color in the Oranges tab of the Tabs example :)!

[netlify]

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 10e8e85

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61bc6394e9cfd70007f9fc49

😎 Browse the preview: https://deploy-preview-6082--docusaurus-2.netlify.app

[github-actions]

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟢 Performance	94
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

Lighthouse ran on https://deploy-preview-6082--docusaurus-2.netlify.app/

[Josh-Cena]

That adds one extra level of indirection IMO. What are the benefits of this approach compared to, say, we letting you pass custom props to the tab headings?

@armano2 May I know what's your point of objection?

[slorber]

I don't like it much, we'd rather let you provide custom attributes instead and build this yourself, something like:

<Tabs
  defaultValue="apple"
  values={[
    {label: 'Apple 1', value: 'apple',data-value: 'xyz', className: 'myClassName'},
  ]}>

Maybe we can also add something so that using <TabItem> would allow to apply props to the parent?

[Drylozu]

I don't like it much, we'd rather let you provide custom attributes instead and build this yourself.
Maybe we can also add something so that using <TabItem> would allow to apply props to the parent?

I like this idea, should I close this PR and do a new one? How will we provide the custom props to <TabItem>?

[Josh-Cena]

I like this idea, should I close this PR and do a new one?

Just edit on the current PR. At least for me personally I don't like PRs closed unmerged in my timeline...

How will we provide the custom props to <TabItem>?

Spreading the rest props. But I think we are spreading them to the tab headings, not Tab items.

[Drylozu]

I think it's ready. I created a prop called attributes in the Tabs values/TabItem because if we use the spread operator directly, some props like default or mdxType (in .mdx files) would be added, and they would be things that we don't want I guess

[Josh-Cena]

👍I see, had just been wondering why :P LGTM

[slorber]

That seems fine

Wonder if you could add an example in one of our docs for dogfooding and easier review: see website/_dogfooding (coloring tabs with CSS looks nice)

I also agree to use a custom prop instead of spreading everything, we should allow to pass props to both the parent and the child in the end, it allows more use-cases

Is there a better name that we can find instead of attributes? I find it not very intuitive that it will be props to the parent? Maybe tabAttributes? 🤷‍♂️ I don't have better for now

[Drylozu]

Wonder if you could add an example in one of our docs for dogfooding and easier review: see website/_dogfooding (coloring tabs with CSS looks nice)

I'll try but I don't know how to do that

Is there a better name that we can find instead of attributes? I find it not very intuitive that it will be props to the parent? Maybe tabAttributes? 🤷‍♂️ I don't have better for now

I think tabAttributes is fine but what do you think about headerAttributes?

---

Should I add something like allow functions to know if the current tab is active?

[Josh-Cena]

I don't find it very unintuitive that attributes apply to the heading... In the end everything we can customize are the headings anyways and the TabItem themselves are just dummy elements

[slorber]

Thanks, that looks nice.

In the end everything we can customize is the headings anyways and the TabItem themselves are just dummy elements

TabItem is not a dummy element, it's a div[role="tabpanel]. By providing props here, we might want to either customize the tab item, or that tab panel. Current API looks good to me.