-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add tabs component #341
Changes from all commits
Commits
Show all changes
72 commits
Select commit
Hold shift + click to select a range
9a304b9
feat(tabs): initial implementation
mtsvyatkova 16dfafd
Merge branch 'master' into mtsvyatkova/tabs-component
mtsvyatkova 42899b0
feat(tabs): update scrolling
mtsvyatkova 263c920
Merge branch 'mtsvyatkova/tabs-component' of https://github.com/Ignit…
mtsvyatkova 6f37677
feat(tabs): update indicator and panel styles
mtsvyatkova d37938c
chore: fix lint
mtsvyatkova 1554c4c
feat(tabs): initial styling
SisIvanova 3cbae96
fix lint error
SisIvanova 9c5dba5
feat(tab): add themes
SisIvanova 96cd2aa
feat(tabs):add keyboard events and activation prop
mtsvyatkova 5a4b94f
feat(tabs): add igcChange event and select method
mtsvyatkova 08da984
light & dark themes improvements
SisIvanova dae461c
add fluent theme
SisIvanova c1816b3
finish bootstrap focus styles
SisIvanova 097b369
remove unnecessary changes
SisIvanova 366e04a
remove panel div and update selected prop
mtsvyatkova 85da81f
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov 452e1d6
feat(tabs): add RTL support
mtsvyatkova 72dc15c
Merge branch 'mtsvyatkova/tabs-component' of https://github.com/Ignit…
mtsvyatkova 3d65d9c
add docs, fix imports and cache DOM elements
mtsvyatkova c1458ac
apply requested styling changes
SisIvanova 40eeea6
add tests
mtsvyatkova 47e7dde
Merge branch 'mtsvyatkova/tabs-component' of https://github.com/Ignit…
mtsvyatkova 04c0b9f
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov 42c12e8
update bootstrap & fluent themes
SisIvanova 571cc2e
remove icon-button hover & focus styles
SisIvanova dd01331
update scroll logic
mtsvyatkova 54b1aa7
Merge branch 'master' into mtsvyatkova/tabs-component
SisIvanova 4572eae
add button renderer & get next tab on button click
mtsvyatkova 194593d
Merge branch 'master' into mtsvyatkova/tabs-component
mtsvyatkova e7328a5
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov 51d46df
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov 1853be7
update scroll buttons colors
SisIvanova a94741d
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov 6c0bbf9
update tabs typography
SisIvanova 86bfca4
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov c6e24f3
update scroll logic and add alignment tests
mtsvyatkova 7b6dd45
align selected indicator after scroll
mtsvyatkova 74a8253
Merge branch 'master' into mtsvyatkova/tabs-component
simeonoff bdc0062
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov d37509e
Merge branch 'master' into mtsvyatkova/tabs-component
simeonoff 3d22354
display both scroll buttons
mtsvyatkova 83e9716
Merge branch 'master' into mtsvyatkova/tabs-component
simeonoff 43ce217
fix(tabs): Various paper-cut fixes
rkaraivanov 183b538
update disabled header button color
SisIvanova 89f289a
fix bootstrap & material justify alignment
SisIvanova b3696b4
test(tabs): Wait for buttons update in scroll tests
rkaraivanov 7162b51
test(tabs): Added RTL scrolling tests
rkaraivanov a2991b5
test(tabs): Add RTL indicator&key selection tests
mtsvyatkova ed2fb51
Merge branch 'master' into mtsvyatkova/tabs-component
mtsvyatkova 4581887
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov a18196f
refactor(tabs): Slotchange behavior for tabs
rkaraivanov 267208e
fix(tabs): Scrolling when tab is partially visible
mtsvyatkova cfc0763
fix(tabs): set default scroll to nearest
mtsvyatkova 46bff71
fix(tabs): scroll buttons size
SisIvanova 82e3928
fix(tabs): improve tab alignment
simeonoff 65379e6
Merge branch 'master' into mtsvyatkova/tabs-component
simeonoff c1eb4ab
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov 77fcefb
fix(tabs): set aria attrs and remove panel's props
mtsvyatkova 7ffea4a
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov dcc14dd
fix(tabs): scroll doesn't work in safari
simeonoff b1c8517
Merge branch 'master' into mtsvyatkova/tabs-component
ChronosSF 8ef0fc7
fix(tabs): Scroll behavior
rkaraivanov 08f41b3
stories(tab): update icons
simeonoff a13ffa2
refactor(tab-panel): Implicit attributes
rkaraivanov 097e1e9
refactor: Selection logic
rkaraivanov 299b9cf
themes(tabs): fix prefix/suffix styling
simeonoff 905e2f6
fix(tabs): scrollIntoView options
mtsvyatkova 90c49d1
Merge branch 'master' into mtsvyatkova/tabs-component
rkaraivanov b40785d
refactor: Addressed PR comments
rkaraivanov 115e738
refactor: Auto increment handling and mutaiton observer callback
rkaraivanov f9d7e9c
fix: Automatic attribute wire-up
rkaraivanov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { html, LitElement } from 'lit'; | ||
import { createCounter } from '../common/util.js'; | ||
import { styles } from './themes/light/tab-panel.base.css.js'; | ||
|
||
/** | ||
* Represents the content of a tab | ||
* | ||
* @element igc-tab-panel | ||
* | ||
* @slot - Renders the content. | ||
*/ | ||
export default class IgcTabPanelComponent extends LitElement { | ||
public static readonly tagName = 'igc-tab-panel'; | ||
|
||
public static override styles = styles; | ||
|
||
private static readonly increment = createCounter(); | ||
|
||
public override connectedCallback() { | ||
this.setAttribute('role', 'tabpanel'); | ||
this.tabIndex = this.hasAttribute('tabindex') ? this.tabIndex : 0; | ||
this.slot = this.slot.length > 0 ? this.slot : 'panel'; | ||
this.id = | ||
this.id.length > 0 | ||
? this.id | ||
: `igc-tab-panel-${IgcTabPanelComponent.increment()}`; | ||
} | ||
|
||
protected override render() { | ||
return html`<slot></slot>`; | ||
} | ||
} | ||
|
||
declare global { | ||
interface HTMLElementTagNameMap { | ||
'igc-tab-panel': IgcTabPanelComponent; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import { html, LitElement } from 'lit'; | ||
import { property, query } from 'lit/decorators.js'; | ||
import { themes } from '../../theming/theming-decorator.js'; | ||
import { createCounter } from '../common/util.js'; | ||
import { styles } from './themes/light/tab.base.css.js'; | ||
import { styles as bootstrap } from './themes/light/tab.bootstrap.css.js'; | ||
import { styles as fluent } from './themes/light/tab.fluent.css.js'; | ||
import { styles as indigo } from './themes/light/tab.indigo.css.js'; | ||
|
||
/** | ||
* Represents the tab header. | ||
* | ||
* @element igc-tab | ||
* | ||
* @slot prefix - Renders before the tab header content. | ||
* @slot - Renders the tab header content. | ||
* @slot suffix - Renders after the tab header content. | ||
* | ||
* @csspart content - The content wrapper. | ||
* @csspart prefix - The prefix wrapper. | ||
* @csspart suffix - The suffix wrapper. | ||
*/ | ||
@themes({ bootstrap, fluent, indigo }) | ||
export default class IgcTabComponent extends LitElement { | ||
public static readonly tagName = 'igc-tab'; | ||
|
||
public static override styles = styles; | ||
|
||
private static readonly increment = createCounter(); | ||
|
||
@query('[part="base"]', true) | ||
private tab!: HTMLElement; | ||
|
||
/** The id of the tab panel which will be controlled by the tab. */ | ||
@property({ type: String }) | ||
public panel = ''; | ||
|
||
/** Determines whether the tab is selected. */ | ||
@property({ type: Boolean, reflect: true }) | ||
public selected = false; | ||
|
||
/** Determines whether the tab is disabled. */ | ||
@property({ type: Boolean, reflect: true }) | ||
public disabled = false; | ||
|
||
public override connectedCallback(): void { | ||
super.connectedCallback(); | ||
this.id = | ||
this.id.length > 0 ? this.id : `igc-tab-${IgcTabComponent.increment()}`; | ||
} | ||
|
||
/** Sets focus to the tab. */ | ||
public override focus(options?: FocusOptions) { | ||
this.tab.focus(options); | ||
} | ||
|
||
/** Removes focus from the tab. */ | ||
public override blur() { | ||
this.tab.blur(); | ||
} | ||
|
||
protected override render() { | ||
return html` | ||
<div | ||
part="base" | ||
role="tab" | ||
aria-disabled=${this.disabled ? 'true' : 'false'} | ||
aria-selected=${this.selected ? 'true' : 'false'} | ||
tabindex=${this.disabled || !this.selected ? -1 : 0} | ||
> | ||
<slot name="prefix" part="prefix"></slot> | ||
<div part="content"> | ||
<slot></slot> | ||
</div> | ||
<slot name="suffix" part="suffix"></slot> | ||
</div> | ||
`; | ||
} | ||
} | ||
|
||
declare global { | ||
interface HTMLElementTagNameMap { | ||
'igc-tab': IgcTabComponent; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see
aria-controls
linking to the panel the tab controls, the specs say 'should' but I think it might be needed to properly reflect the active tab when the focus is in the panel or its children.To that end - @rkaraivanov would it not make more sense to have
id
on the panel instead ofname
as the ARIA link is based on id-s anyway?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@damyanpetev
Fine by me. Feel free to update the specification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did though
aria-controls
is described as should have.Reading https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role we might also consider aria-owns list on the tabs themselves it seems as it's described as must
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tablist role is set for the parent element of the tabs so I believe we don't need the aria-owns attribute.