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: ion-button with [routerLink] will display a border if being activated #20632

Closed
wilk-polarny opened this issue Feb 27, 2020 · 3 comments · Fixed by #29744 · May be fixed by gramirez-vic/prueba1_alcaldia#1
Closed

bug: ion-button with [routerLink] will display a border if being activated #20632

wilk-polarny opened this issue Feb 27, 2020 · 3 comments · Fixed by #29744 · May be fixed by gramirez-vic/prueba1_alcaldia#1
Labels
package: angular @ionic/angular package type: bug a confirmed bug report

Comments

@wilk-polarny
Copy link

wilk-polarny commented Feb 27, 2020

Bug Report

Ionic version:

[x] 4.11.9

Current behavior:
When activating an <ion-button> element with a [routerLink] directive being set, the button will display show a border.

image

Expected behavior:
No border should be displayed when a button gets activated.

Steps to reproduce:

  • Use an empty template to create an ion-button like below.
  • Either click the element to briefly see the border or cycle through the elements with tab

Related code:

<ion-button expand="block" fill="clear" [routerLink]="['/tabs/tab2']">Some button</ion-button>

https://stackblitz.com/edit/ionic-v4-angular-tabs-9a3req

Other information:
I'm not sure whether buttons are supposed to work like this, but this does not happen with a href being set.
I would only expect such behaviour in a real browser app, where anchors are being colored accordingly - but not within a Hybrid App on iOS :)

Ionic info:


Ionic:

   Ionic CLI                     : 6.1.0 (/Users/Wilk/.nvm/versions/node/v10.16.3/lib/node_modules/@ionic/cli)
   Ionic Framework               : @ionic/angular 4.11.9
   @angular-devkit/build-angular : 0.801.3
   @angular-devkit/schematics    : 8.1.3
   @angular/cli                  : 8.1.3
   @ionic/angular-toolkit        : 2.1.2

Cordova:

   Cordova CLI       : 9.0.0 ([email protected])
   Cordova Platforms : ios 5.1.1
   Cordova Plugins   : cordova-plugin-ionic-keyboard 2.2.0, cordova-plugin-ionic-webview 4.1.3, (and 4 other plugins)

Utility:

   cordova-res (update available: 0.9.0) : 0.8.1
   native-run                            : 0.3.0

System:

   ios-deploy : 1.9.4
   ios-sim    : 8.0.2
   NodeJS     : v10.16.3 (/Users/Wilk/.nvm/versions/node/v10.16.3/bin/node)
   npm        : 6.9.0
   OS         : macOS Catalina
   Xcode      : Xcode 11.3.1 Build version 11C504
@ionitron-bot ionitron-bot bot added the triage label Feb 27, 2020
@brandyscarney brandyscarney added package: angular @ionic/angular package type: bug a confirmed bug report labels Feb 27, 2020
@ionitron-bot ionitron-bot bot removed the triage label Feb 27, 2020
@brandyscarney
Copy link
Member

Thanks for the issue! This is actually an issue due to Angular adding tabindex=0 to the ion-button component. The outline can be hidden using:

ion-button:focus {
  outline: none;
}

However, the real problem is that by adding the tabindex both the ion-button and the native a element are able to be tabbed to, so you have to tab twice on the same element to go to the next one.

I left a comment on an issue on Angular's repo about this: angular/angular#28345 (comment)

You should be able to work around this issue by adding tabindex="null" to your ion-button. Unfortunately when I tried adding that in our actual component in Ionic, Angular still overrode it to 0, but I tested it out in an Angular app using Ionic and it is working there. I believe this is something that will need to be fixed on Angular's side, but we are going to reach out to the team directly about it.

@CrashBrennan
Copy link

However, the real problem is that by adding the tabindex both the ion-button and the native a element are able to be tabbed to, so you have to tab twice on the same element to go to the next one.

Has there been any progress on this issue? Or has angular ignored it for over 4 years?

github-merge-queue bot pushed a commit that referenced this issue Aug 8, 2024
…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`
Copy link

ionitron-bot bot commented Sep 7, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.