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

Add tabs component #341

Merged
merged 72 commits into from
Jul 7, 2022
Merged

Add tabs component #341

merged 72 commits into from
Jul 7, 2022

Conversation

mtsvyatkova
Copy link
Contributor

Closes #185

@mtsvyatkova mtsvyatkova requested a review from rkaraivanov May 9, 2022 14:28
src/components/common/definitions/defineAllComponents.ts Outdated Show resolved Hide resolved
src/components/tabs/tab-panel.ts Outdated Show resolved Hide resolved
src/components/tabs/tab-panel.ts Outdated Show resolved Hide resolved
src/components/tabs/tab-panel.ts Outdated Show resolved Hide resolved
src/components/tabs/tab.ts Outdated Show resolved Hide resolved
src/components/tabs/tabs.ts Outdated Show resolved Hide resolved
src/components/tabs/tabs.ts Outdated Show resolved Hide resolved
src/components/tabs/tabs.ts Outdated Show resolved Hide resolved
src/components/tabs/tabs.ts Outdated Show resolved Hide resolved
src/components/tabs/tabs.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@simeonoff simeonoff left a comment

Choose a reason for hiding this comment

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

I can see a slight border leftover in the selected tab when using bootstrap.

Comment on lines 24 to 30
> * {
margin-inline-start: rem(12px);

&:first-child {
margin-inline-start: 0;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can replace all this with:

Suggested change
> * {
margin-inline-start: rem(12px);
&:first-child {
margin-inline-start: 0;
}
}
gap: rem(12px);

Comment on lines 72 to 73
width: rem(24px);
height: rem(24px);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use --size for sizing igc-icons.

simeonoff
simeonoff previously approved these changes Jun 27, 2022
@simeonoff simeonoff self-requested a review June 27, 2022 14:39
simeonoff
simeonoff previously approved these changes Jun 27, 2022
Support using tab as a tab strip
Support selection through attribute for tab children.
Mutation observer to sync DOM changes for ARIA and selection states.
Some fixed issues.
@simeonoff simeonoff self-requested a review July 5, 2022 07:33
simeonoff
simeonoff previously approved these changes Jul 5, 2022
damyanpetev
damyanpetev previously approved these changes Jul 5, 2022
rkaraivanov
rkaraivanov previously approved these changes Jul 6, 2022
@damyanpetev
Copy link
Member

@rkaraivanov I've noticed on the Strip sample since they have no panels the change event emits empty string detail for all - perhaps we should consider adding the entire tab element to the detail?

Also an idea related to that - perhaps we can add functionality to auto-associate tabs and panels based on index so explicit ids are not mandatory (will ease generating tabs from designs)

@rkaraivanov rkaraivanov dismissed stale reviews from damyanpetev, simeonoff, and themself via b40785d July 6, 2022 12:37
@rkaraivanov rkaraivanov requested a review from damyanpetev July 6, 2022 12:38
@damyanpetev damyanpetev merged commit 6fa846c into master Jul 7, 2022
@damyanpetev damyanpetev deleted the mtsvyatkova/tabs-component branch July 7, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab component
9 participants