angular-eslint / angular-eslint

:sparkles: Monorepo for all the tooling related to using ESLint with Angular
MIT License
1.64k stars 223 forks source link

[@angular-eslint/template/accessibility-elements-content] add allow list for directives that guarantee content #1169

Closed EliHowey closed 1 year ago

EliHowey commented 2 years ago

Description and reproduction of the issue

@angular-eslint/template/accessibility-elements-content is intelligent enough to check whether elements have attributes that would provide an accessible name, like aria-label. But it currently doesn't provide a way to specify that custom Angular directives can guarantee the existence of one of those attributes. For example, if I have a directive that always sets aria-label, the rule will still complain:

{
  "rules": {
    "@angular-eslint/template/accessibility-elements-content": "error"
  }
}
@Directive({
  selector: 'button[appTooltipLabel]'
})
export class TooltipLabelDirective implements OnInit {
  @Input('appTooltipLabel') @HostBinding('attr.aria-label') label!: string;

  ngOnInit(): void {
    if (!this.label) {
      throw new Error('appTooltipLabel must have a value');
    }
  }

  // Omitted: Logic to render a tooltip UI
}
     <button appTooltipLabel="This label gets assigned as the button's aria-label"></button>
<!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<!-- error: <button> should have content (@angular-eslint/template/accessibility-elements-content) -->

Suggested Change

Add configuration to @angular-eslint/template/accessibility-elements-content to ignore certain selectors that guarantee the appropriate element content at runtime, e.g.:

{
  "rules": {
    "@angular-eslint/template/accessibility-elements-content": [
      "error",
      { "allowSelectors": ["button[appTooltipLabel]"] }
    ]
  }
}

Versions

package version
@angular-eslint/eslint-plugin-template 12.7.0
@angular-eslint/template-parser 12.7.0
@typescript-eslint/parser 4.28.2
ESLint 7.32.0
node 14.20.1
# Please run `npx ng version` in your project and paste the full output here:

Angular CLI: 12.2.18
Node: 14.20.1
Package Manager: npm 6.14.17
OS: darwin x64

Angular: 12.2.16
... animations, common, compiler, compiler-cli, core, elements
... forms, language-service, platform-browser
... platform-browser-dynamic, platform-server, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1202.18
@angular-devkit/build-angular   12.2.18
@angular-devkit/core            12.2.18
@angular-devkit/schematics      12.2.18
@angular/cdk                    12.2.13
@angular/cli                    12.2.18
@nguniversal/builders           12.1.3
@nguniversal/express-engine     12.1.3
@schematics/angular             12.2.18
ng-packagr                      12.2.7
rxjs                            6.5.5
typescript                      4.3.5
webpack                         5.74.0
sandikbarr commented 1 year ago

Supporting CSS style selectors like button[appTooltipLabel] as suggested is more of a challenge, but I have added an allowList option in PR #1201 that would let you configure a list of additional attributes, inputs, or attribute directives that would satisfy the content criteria. @EliHowey Does this work for your use case?

EliHowey commented 1 year ago

@sandikbarr Yes, that should work for us, thank you!