Skip to content

Commit

Permalink
fix(angular): remove the tabindex set by routerLink from Ionic compon…
Browse files Browse the repository at this point in the history
…ents (#29744)

Issue number: resolves #20632

---------

## What is the current behavior?
When using the `routerLink` directive in Angular, it automatically adds
`tabindex="0"` to the element. This creates issues with Ionic components
that render native button or anchor elements, as they have their own
focus management. As a result, when navigating between list items with
`routerLink` using the `Tab` key, you need to press the `Tab` key twice
to move to the next item. This problem is illustrated in the following
demo:

[![Open in
StackBlitz](https://developer.stackblitz.com/img/open_in_stackblitz.svg)](https://stackblitz.com/edit/angular-blfa7h?file=src%2Fapp%2Fexample.component.html)

Related Angular issue: angular/angular#28345

## What is the new behavior?
Updated our `RouterLinkDelegateDirective` to check if the element using
`routerLink` is one of the following Ionic components:
`ion-back-button`, `ion-breadcrumb`, `ion-button`, `ion-card`,
`ion-fab-button`, `ion-item`, `ion-item-option`, `ion-menu-button`,
`ion-segment-button`, or `ion-tab-button`. If so, it removes the
`tabindex` attribute from the element. This allows these Ionic
components to let the native button or anchor element handle the focus.

This solution is demonstrated in the following demo:

[![Open in
StackBlitz](https://developer.stackblitz.com/img/open_in_stackblitz.svg)](https://stackblitz.com/edit/angular-blfa7h-svmguh?file=src%2Fapp%2Fexample.component.html)

> [!NOTE]
> I did not include the `ion-router-link` component in the list to
remove `tabindex` because [the router link
documentation](https://ionicframework.com/docs/api/router-link) does not
recommend using it with Angular:
>> Note: this component should only be used with vanilla and Stencil
JavaScript projects. For Angular projects, use an `<a>` and `routerLink`
with the Angular router.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

Dev build: `8.2.7-dev.11722448707.1e8c66e6`
  • Loading branch information
brandyscarney committed Aug 8, 2024
1 parent 7b16397 commit 20073e1
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,45 @@ export class RouterLinkDelegateDirective implements OnInit, OnChanges {

ngOnInit(): void {
this.updateTargetUrlAndHref();
this.updateTabindex();
}

ngOnChanges(): void {
this.updateTargetUrlAndHref();
}

/**
* The `tabindex` is set to `0` by default on the host element when
* the `routerLink` directive is used. This causes issues with Ionic
* components that wrap an `a` or `button` element, such as `ion-item`.
* See issue https://github.com/angular/angular/issues/28345
*
* This method removes the `tabindex` attribute from the host element
* to allow the Ionic component to manage the focus state correctly.
*/
private updateTabindex() {
// Ionic components that render a native anchor or button element
const ionicComponents = [
'ION-BACK-BUTTON',
'ION-BREADCRUMB',
'ION-BUTTON',
'ION-CARD',
'ION-FAB-BUTTON',
'ION-ITEM',
'ION-ITEM-OPTION',
'ION-MENU-BUTTON',
'ION-SEGMENT-BUTTON',
'ION-TAB-BUTTON',
];
const hostElement = this.elementRef.nativeElement;

if (ionicComponents.includes(hostElement.tagName)) {
if (hostElement.getAttribute('tabindex') === '0') {
hostElement.removeAttribute('tabindex');
}
}
}

private updateTargetUrlAndHref() {
if (this.routerLink?.urlTree) {
const href = this.locationStrategy.prepareExternalUrl(this.router.serializeUrl(this.routerLink.urlTree));
Expand Down
15 changes: 15 additions & 0 deletions packages/angular/test/base/e2e/src/lazy/router-link.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,21 @@ describe('Router Link', () => {
testBack();
});
});

// Angular sets the `tabindex` to `"0"` on any element that uses
// the `routerLink` directive. Ionic removes the `tabindex` from
// components that wrap an `a` or `button` element, so we are
// checking here that it is only removed from Ionic components.
// https://github.com/ionic-team/ionic-framework/issues/20632
describe('tabindex', () => {
it('should have tabindex="0" with a native span', () => {
cy.get('#span').should('have.attr', 'tabindex', '0');
});

it('should not have tabindex set with an ionic button', () => {
cy.get('#routerLink').should('not.have.attr', 'tabindex');
});
});
});

function testForward() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ describe('RouterLink', () => {
beforeEach(() => {
cy.visit('/standalone/router-link');
});

it('should mount the root component', () => {
cy.ionPageVisible('app-router-link');

Expand All @@ -21,4 +21,17 @@ describe('RouterLink', () => {

cy.url().should('include', '/standalone/popover');
});

// Angular sets the `tabindex` to `"0"` on any element that uses
// the `routerLink` directive. Ionic removes the `tabindex` from
// components that wrap an `a` or `button` element, so we are
// checking here that it is only removed from Ionic components.
// https://github.com/ionic-team/ionic-framework/issues/20632
it('should have tabindex="0" with a native span', () => {
cy.get('span').should('have.attr', 'tabindex', '0');
});

it('should not have tabindex set with an ionic button', () => {
cy.get('ion-button').should('not.have.attr', 'tabindex');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@
<p><button id="queryParamsFragment" routerLink="/lazy/router-link-page2/MyPageID==" [queryParams]="{ token: 'A&=#Y' }"
fragment="myDiv1">Query Params and Fragment</button></p>

<p><span routerLink="/lazy/router-link-page" id="span">span[routerLink]</span></p>
</ion-content>
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,9 @@
<button routerLink="/standalone/popover" routerDirection="root">
I'm a button
</button>
<span routerLink="/standalone/popover" routerDirection="root">
I'm a span
</span>
<ion-button routerLink="/standalone/popover" routerDirection="root">
I'm an ion-button
</ion-button>

0 comments on commit 20073e1

Please sign in to comment.