Skip to content

fix(tabs): ensure first tab is active when no active tab is provided#2072

Open
akashsonune wants to merge 1 commit into
mainfrom
fix/a11y-tabs-keyboard-entry-without-active-tab
Open

fix(tabs): ensure first tab is active when no active tab is provided#2072
akashsonune wants to merge 1 commit into
mainfrom
fix/a11y-tabs-keyboard-entry-without-active-tab

Conversation

@akashsonune

@akashsonune akashsonune commented May 15, 2026

Copy link
Copy Markdown
Member

Tablist is not reachable when there is no active tab


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@akashsonune akashsonune changed the title fix(tabs): ensure tablist is keyboard reachable when no active tab is… fix(tabs): ensure tablist is keyboard reachable when no active tab is defined May 15, 2026
@akashsonune akashsonune marked this pull request as ready for review May 15, 2026 14:59
@akashsonune akashsonune requested review from a team as code owners May 15, 2026 14:59

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a tabindexValue computed signal to manage the tabindex attribute within the SiTabBaseDirective. The review identifies that using a computed signal for this logic is problematic because it depends on a non-signal property from the focus key manager, which will not trigger necessary updates during keyboard navigation. Furthermore, the current implementation may result in multiple tabs having a zero tabindex. The feedback recommends replacing the signal with a method to ensure proper re-evaluation and correct roving tabindex behavior.

Comment thread projects/element-ng/tabs/si-tab-base.directive.ts Outdated
@akashsonune

akashsonune commented May 15, 2026

Copy link
Copy Markdown
Member Author

@panch1739 @dr-itz WDYT?

Alternatively as per- https://www.w3.org/WAI/ARIA/apg/patterns/tabs/ there should be at least one tab active. In that case, we can make the first non disabled tab as active if no active tab is provided. Shall we do this instead?

@akashsonune akashsonune force-pushed the fix/a11y-tabs-keyboard-entry-without-active-tab branch from 15a5ae0 to 05e7c36 Compare May 15, 2026 15:07

@chintankavathia chintankavathia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@akashsonune please rebase

@akashsonune akashsonune force-pushed the fix/a11y-tabs-keyboard-entry-without-active-tab branch 3 times, most recently from 7af8f17 to 157d20f Compare June 18, 2026 11:24
@akashsonune

Copy link
Copy Markdown
Member Author

@akashsonune please rebase

@chintankavathia can you check. I discussed it with @panch1739 and we decided that there should be at least one tab selected/active when nothing is provided, in this case the first one. this solves the issue of keyboard reachable tabs as there will always be a active tab

@akashsonune akashsonune force-pushed the fix/a11y-tabs-keyboard-entry-without-active-tab branch from 157d20f to 7a9190e Compare June 18, 2026 11:32
@akashsonune akashsonune changed the title fix(tabs): ensure tablist is keyboard reachable when no active tab is defined fix(tabs): ensure first tab is active when no active tab is provided Jun 18, 2026
Comment thread projects/element-ng/tabs/si-tabset.component.ts Outdated
Comment thread projects/element-ng/tabs/si-tabset.component.ts Outdated
Comment thread projects/element-ng/tabs/si-tabset.component.ts Outdated
@akashsonune akashsonune force-pushed the fix/a11y-tabs-keyboard-entry-without-active-tab branch from 7a9190e to dc3abe1 Compare June 19, 2026 18:35
@akashsonune

Copy link
Copy Markdown
Member Author

@chintankavathia updated, please check

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants