Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Adding data-id to tab component #278

Merged
merged 2 commits into from
Jun 15, 2023
Merged

Adding data-id to tab component #278

merged 2 commits into from
Jun 15, 2023

Conversation

tarzen-chugh
Copy link
Collaborator

What this PR does

Adds support for adding data-id prop to tab component.
Usage
<Tabs.Panel
label={translate('Sharing')}
data-id="privacy_drawer_sharing_tab">

How it does that

Testing

All unit tests are passing in repository.

Copy link
Contributor

@Boughtmanatee5 Boughtmanatee5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need this. data attributes are passed in the props and since we're spreading props at the bottom there you should be able to pass any data attributes you want on the component Button itself. Same with aria attributes.

@juliewongbandue
Copy link
Collaborator

You shouldn't need this. data attributes are passed in the props and since we're spreading props at the bottom there you should be able to pass any data attributes you want on the component Button itself. Same with aria attributes.

Oh yes, @Boughtmanatee5 is right. Props are spread here: https://github.com/vimeo/iris/pull/278/files#diff-ce687691fe458ef8d9a85cc00e06bf778a4272a39b854ce5c89e598eecb531e3L93, so you can pass data-id as expected to the Tabs.Panel:

      <Tabs.Panel
        label="Tab 0"
        data-id="Tab-0-id" // data-id (and any aria attributes..) can be passed directly to the component
        onOpen={() => console.log('Clicked Tab 0')}
      >
        <Header size="2">I am Tab 0</Header>
        <Paragraph size="2">Lorem ipsum dolor sit amet.</Paragraph>
        <img src="http://placekitten.com/1000/400" width="100%" />
      </Tabs.Panel>

@tarzen-chugh
Copy link
Collaborator Author

tarzen-chugh commented Jun 12, 2023

@juliewongbandue This is what we have done currently but data-id gets passed to tab content instead of anchor tag.

image

image

Ref code

@Boughtmanatee5 Can you look at the comment above.

@tarzen-chugh tarzen-chugh marked this pull request as ready for review June 13, 2023 09:04
@tarzen-chugh tarzen-chugh requested review from a team as code owners June 13, 2023 09:04
@Boughtmanatee5 Boughtmanatee5 self-requested a review June 14, 2023 13:22
src/components/Tabs/Tabs.tsx Outdated Show resolved Hide resolved
src/components/Button/Button.tsx Outdated Show resolved Hide resolved
@juliewongbandue juliewongbandue enabled auto-merge (squash) June 15, 2023 15:43
@juliewongbandue juliewongbandue merged commit abdc5d8 into main Jun 15, 2023
@juliewongbandue juliewongbandue deleted the tabs-tagging branch June 15, 2023 15:43
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants