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(MatButtonModule): Disabled mat-button with routerLink still works #19428

Closed
scopchanov opened this issue May 23, 2020 · 8 comments
Closed
Labels
area: material/button needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions
Projects

Comments

@scopchanov
Copy link

scopchanov commented May 23, 2020

Reproduction

The issue is reproduced on StackBlitz: https://stackblitz.com/edit/inactive-links-problem

Steps to reproduce:

  1. Use the following code:
    <a mat-button routerLink="/foo" [disabled]="true">Foo</a>

Expected Behavior

The button is visually disabled and clicking it does not activate the link.

Actual Behavior

The button is indeed visually disabled, but the link still works.

Environment

  • Angular: 9.1.9
  • CDK/Material: 9.2.4
  • Browser(s): Chrome, Mozilla, Safari
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows, iOS
@scopchanov scopchanov added the needs triage This issue needs to be triaged by the team label May 23, 2020
@scopchanov scopchanov changed the title bug([MatButtonModule]): [Disabled mat-button with routerLink still works] bug(MatButtonModule): Disabled mat-button with routerLink still works May 23, 2020
@devversion devversion added this to Triaged in triage #1 via automation May 25, 2020
@devversion devversion added area: material/button and removed needs triage This issue needs to be triaged by the team labels May 25, 2020
@devversion
Copy link
Member

devversion commented May 25, 2020

I was able to reproduce this. Usually anchor elements cannot be disabled (natively), but our MatAnchor implementation tries to support a disabled binding by intercepting the click event and preventing default / stopping propagation. This does not work nicely as the Angular routerLink directive registered its click event before MatAnchor.

One could argue this is working as expected, but then we shouldn't support the disabled input for anchors. Alternative option is that we set pointer-events: none, but that would then break tooltips.

Related post: https://css-tricks.com/how-to-disable-links/

@devversion devversion added needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions labels May 25, 2020
@scopchanov
Copy link
Author

I hope the following info could help fixing the issue:

  1. It does work with the original example from Angular Material + added [disabled]="true", i.e.

<a mat-button href="https://www.google.com/" target="_blank" [disabled]="true">Link</a>

However, in this case the attribute is href instead of routerLink.

  1. It also works as expected with MatListItem, i.e.
<mat-nav-list>
    <a mat-list-item routerLink="/foo" [disabled]="true">Foo</a>
    <a mat-list-item routerLink="/moo" [disabled]="true">Moo</a>
</mat-nav-list>
  1. Only the combination of mat-button and routerLink does not work well with disabled.

Could MatListItem be used as an example to fix MatButton?

@MaxDZ8
Copy link

MaxDZ8 commented Oct 26, 2020

Just to ensure this gets some minor activity and does not get closed I confirm I have stumbled on this today.

Since routerLink is "base angular" directive while disabled comes from mat-button, I suspect the thing could get involved.

For the time being, I will be filtering the (click) event myself but I hope this gets some attention.

@Ishou
Copy link

Ishou commented Nov 6, 2020

I also reproduce this issue. It seems like this is a side effect of c2abcad, it is supposed to avoid "actually" disabling the button so it may have a tooltip, but I guess it also lets it keep the href.
I don't see any actual change in the commit that could break things but the intent seems to match the issue...

Here is a workaround with a global style that works for my case (it disables any tooltip too):

.mat-button-disabled {
  pointer-events: none;
}

@ronaldhoek
Copy link

This has been reported before: #12920

@filarrro
Copy link

+1 on this one, if the attribute is supported then it should be working as expected

@zarend
Copy link
Contributor

zarend commented Jan 13, 2023

Cannot reproduce. I believe this has been fixed.

@zarend zarend closed this as completed Jan 13, 2023
@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 Feb 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/button needs: discussion Further discussion with the team is needed before proceeding P4 A relatively minor issue that is not relevant to core functions
Projects
No open projects
triage #1
  
Triaged
Development

Successfully merging a pull request may close this issue.

7 participants