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

bug(tabs): mat-tab-nav-bar tab working with space but not with enter key #27843

Closed
1 task done
Simona25 opened this issue Sep 25, 2023 · 7 comments · Fixed by #27862
Closed
1 task done

bug(tabs): mat-tab-nav-bar tab working with space but not with enter key #27843

Simona25 opened this issue Sep 25, 2023 · 7 comments · Fixed by #27862
Assignees
Labels
Accessibility This issue is related to accessibility (a11y) area: material/tabs P2 The issue is important to a large percentage of users, with a workaround

Comments

@Simona25
Copy link

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

Previously the mat-tab-nav-bar tabs could be activated both with space and enter, while now the space key still works but the enter doesn't trigger the action anymore.

Reproduction

Link: https://material.angular.io/components/tabs/examples official material example (Basic tab nav bar)
Steps to reproduce:

  1. Use the keyboard to move the focus inside the tab nav bar,
  2. Pressing Enter nothing happen,
  3. Pressing the Space the tab is correctly activated.

Expected Behavior

Pressing Enter the tab should be activated like when using hte Space.

Actual Behavior

Pressing Space works, while pressing Enter doesn't and the tab link isn't activated.

Environment

  • Angular: 16.2.5 (but also on the 15.2.5 for example)
  • CDK/Material: 16.2.5 (but also on the 15.2.5 for example)
  • Browser(s): Chrome
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows
@Simona25 Simona25 added the needs triage This issue needs to be triaged by the team label Sep 25, 2023
@mmalerba mmalerba added P2 The issue is important to a large percentage of users, with a workaround Accessibility This issue is related to accessibility (a11y) area: material/tabs and removed needs triage This issue needs to be triaged by the team labels Sep 25, 2023
@zarend zarend self-assigned this Sep 25, 2023
@zarend
Copy link
Contributor

zarend commented Sep 25, 2023

Hello folks,

Thank you for reporting this issue. I agree with the expected behavior, I would expect Enter or Space to perform the same action as clicking. We explicitly say that we support Enter and Space in the keyboard interaction documentation, it's needed to meet accessibility requirements

I investigated this issue. To summarize, both changes to the example code and implemented of MatTabNav are needed – they both seem to be handling Enter/Space incorrectly.

image The example code listens for click event to update the model of the active tab. We should not be using click to update selection because it doesn't include keyboard. It would be better to listen for an output or rely on the key manager to maintain the selected tab.

There are two ways I would expect the api of MatTabNav to work.

API designs of MatTabNav

Option A, not binding to active tab and relying on KeyManager

I would expect this to look simliar to tab-group-basic-example with uses MatTabGroup and MatTab component. Binding the active or selected tab is not necessary since the key manager handles this.

<mat-tab-group>
  <mat-tab label="First"> Content 1 </mat-tab>
  <mat-tab label="Second"> Content 2 </mat-tab>
  <mat-tab label="Third"> Content 3 </mat-tab>
</mat-tab-group>

This currently doesn't work because MatTabNav seems to implement the key manager for LEFT/RIGHT arrows but not for ENTER/SPACE (selection).

I would expect MatTabNav to implement key manager for all the keyboard interactions documented in keyboard interaction.

Option B

Bind to active tab

<nav mat-tab-nav-bar [backgroundColor]="background" [tabPanel]="tabPanel" <nav mat-tab-nav-bar [backgroundColor]="background" [tabPanel]="tabPanel">
  <a mat-tab-link *ngFor="let link of links"> {{link}} </a>
  <a mat-tab-link disabled>Disabled Link</a>
</nav>>
  <a mat-tab-link *ngFor="let link of links"
     [active]="activeLink == link"> {{link}} </a>
  <a mat-tab-link disabled>Disabled Link</a>
</nav>

This currently doesn't work because MatTabNav doesn't implement @Input selectedIndex and @Output selectedIndexChange

work-around

Here's a work-around until this issue and desired API of tabs can be worked-out.

Application code can listen for (keydown) and update selection on Space or Enter.

  <a mat-tab-link *ngFor="let link of links"
     (click)="activeLink = link"
     (keydown)="setActiveLinkOnKeypress($event, link)"
     [active]="activeLink == link"> {{link}} </a>
  <a mat-tab-link disabled>Disabled Link</a>

@zarend
Copy link
Contributor

zarend commented Sep 25, 2023

Hello @mmalerba or @crisbeto (or anyone who is more familiar with tabs than me)

I was wondering if you have more information on the high level design and intention of MatTabNav. That way I can better understanding what a correct outcome would look like.

I'm confused by the API design and intentions of MatTabNav. That's because it seems to be different than our other tab components, such as MatTabHeader. MatTabHeader seems to only handle presentation and not behaviors. MatTabGroup handles more of the behaviors.

Is the intention that using MatTabNav with MatTabLink has parity with using MatTAbGroup with MatTab?

Best regards,

Zach

@Simona25
Copy link
Author

Thank you for your answer! ^_^

@crisbeto
Copy link
Member

crisbeto commented Sep 27, 2023

@zarend the idea with MatTabNav is that it should look like a Material tab header, but have links instead of <button role="tab"></button>. It sounds like a bug that navigation doesn't work with the enter key since native links support it.

@zarend
Copy link
Contributor

zarend commented Sep 27, 2023

Yes, requirement is that end user can click link with space or enter keys. To fix this, I suggest two things:

  • implementing @input selectedIndex and @output selectedIndexChange so that binding to the selected tab work for parity with MatTabGroup. Making selection model work in Option A above.
  • implementing the keyboard manager so that enter and space go to the keyboard manager for parity with MatTabGroup

@crisbeto crisbeto assigned crisbeto and unassigned zarend Sep 28, 2023
crisbeto added a commit to crisbeto/material2 that referenced this issue Sep 28, 2023
Fixes that the tab nav bar wasn't navigating when pressing the enter key.

Fixes angular#27843.
@crisbeto
Copy link
Member

implementing @input selectedIndex and @output selectedIndexChange so that binding to the selected tab work for parity with MatTabGroup. Making selection model work in Option A above.

The nav bar intentionally doesn't implement selectedIndex, because the selection state is controlled through the active input on MatTabLink. If we were to add selectedIndex, we can end up with two conflicting sources of information.

As for supporting ENTER key presses, I've opened #27862 to fix it.

crisbeto added a commit that referenced this issue Sep 29, 2023
Fixes that the tab nav bar wasn't navigating when pressing the enter key.

Fixes #27843.
crisbeto added a commit that referenced this issue Sep 29, 2023
Fixes that the tab nav bar wasn't navigating when pressing the enter key.

Fixes #27843.

(cherry picked from commit 5dd614a)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) area: material/tabs P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants