Components: mat-button click handlers on anchor tags do not respect the disabled attribute

[daotoad]

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Given this template: <a mat-button disabled (click)="someHandler()">Foo</a> the someHandler() method should not be called when the button is clicked.

What is the current behavior?

The someHandler() method is called when the button is clicked.

What are the steps to reproduce?

https://stackblitz.com/edit/angular-ucd9ew

I've included samples with each combination of button/anchor tag and enabled/disabled state.
Clicking the button in each will increment a counter if the handler is called. When clicked, the disabled anchor tag based button in the lower right triggers its handler erroneously.

What is the use-case or motivation for changing an existing behavior?

Disabled buttons shouldn't do anything, even when they're made from anchor tags.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Only tested with Angular 6.1.6, Material 6.4.7, and TypeScript 2.7.2.
Tested in Chrome for MacOS Version 68.0.3440.106 (Official Build) (64-bit) and in Safari Version 11.1.1 (13605.2.8).

Is there anything else we should know?

Not that I am aware of.

[josephperrott]

This is working as expected as disabled is not an attribute of the anchor tag.

Additionally is not feasible to do so if we wanted to because MatButton can't prevent a click event being listened to for a binding placed by the user.

[daotoad]

While disabled is not an attribute of the anchor tag, neither is mat-button or ngForOf which are defined by Directives. Angular and Angular Material 2 honor those attributes, why not disabled on an anchor tag?

Perhaps you mean to say that supporting the disabled attribute on an anchor tag is likely to break screen readers and damage accessibility because it is non-standard. That is a good reason not to support the attribute.

Let us assume that the attribute is and shall remain unsupported, for whatever reason. Then you have four bugs:

1. The documentation does not indicate that the anchor tag does not honor the disabled attribute.
2. The anchor tag visually indicates that it is disabled by accepting the CSS styling of an anchor tag.
3. The anchor tag sets the aria-disabled attribute when disabled is set, further confusing screen readers.
4. We should either get an error message when disabled is set on an anchor tag OR it should be silently ignored. Not somewhere in the middle.

Whether the attribute is to be supported or not, please handle it correctly.

[daotoad]

Now lets look at the other side of the issue, "is the intent of the code to disable click events on anchor tags when the disabled attribute is set?" Examining the code we see:

@Component({
  moduleId: module.id,
  selector: `a[mat-button], a[mat-raised-button], a[mat-icon-button], a[mat-fab],
             a[mat-mini-fab], a[mat-stroked-button], a[mat-flat-button]`,
  exportAs: 'matButton, matAnchor',
  host: {
    // Note that we ignore the user-specified tabindex when it's disabled for
    // consistency with the `mat-button` applied on native buttons where even
    // though they have an index, they're not tabbable.
    '[attr.tabindex]': 'disabled ? -1 : (tabIndex || 0)',
    '[attr.disabled]': 'disabled || null',
    '[attr.aria-disabled]': 'disabled.toString()',
    '(click)': '_haltDisabledEvents($event)',
    '[class._mat-animation-noopable]': '_animationMode === "NoopAnimations"',
  },  
  inputs: ['disabled', 'disableRipple', 'color'],
  templateUrl: 'button.html',
  styleUrls: ['button.css'],
  encapsulation: ViewEncapsulation.None,
  changeDetection: ChangeDetectionStrategy.OnPush,
})
export class MatAnchor extends MatButton {
  /** Tabindex of the button. */
  @Input() tabIndex: number;

  constructor(
    platform: Platform,
    focusMonitor: FocusMonitor,
    elementRef: ElementRef,
    // @breaking-change 7.0.0 `animationMode` parameter to be made required.
    @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string) {
    super(elementRef, platform, focusMonitor, animationMode);
  }

  _haltDisabledEvents(event: Event) {
    // A disabled button shouldn't apply any actions
    if (this.disabled) {
      event.preventDefault();
      event.stopImmediatePropagation();
    }   
  }

Well, _haltDisabledEvents is bound to the click event in the host entry of the Component decorator.

Draw your own conclusions.

[angular-automatic-lock-bot[bot]]

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._}