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

feat(eslint-plugin-template): [accessibility-interactive-supports-focus] add rule #1134

Conversation

sandikbarr
Copy link
Contributor

@sandikbarr sandikbarr commented Sep 16, 2022

  • Adds a new accessibility rule to check that interactive elements are focussable.
    • Ported from eslint-plugin-jsx-a11y interactive-supports-focus
    • This rule ensures that DOM elements with interaction handlers (click, keydown, keyup, keypress) can receive focus either because the element is interactive by default (button, input, etc.) or by adding a tabindex.
  • This rule is intended to be used in conjunction with the template rule click-events-have-key-events
    • click-events-have-key-events ensures that when DOM elements are given click handlers that are not interactive by default that they also have a corresponding key event; however, elements that are not interactive by default also do not receive keyboard focus without tabindex.
    • Also, see my fix for click-events-have-key-events

The accessibility-interactive-supports-focus rule works as follows:

  • Checks only native HTML DOM elements
  • Looks for a tabindex attribute or input
  • Looks for interactive outputs, click, keyup, keydown, or key press
  • If the element has no interactive outputs, is disabled, aria-disabled, has a presentation role, or is hidden from the screen reader, it returns as valid
  • If the element has interactive outputs, does not have a tab index, is not contenteditable, and is not an interactive element by default or is not explicitly assigned a non interactive role, it returns an error that the element must be able to receive focus
    • The fix is to either use a DOM element that is interactive by default, or
    • Add a tabindex to the element to make it focusable. However, it still may lack semantic meaning, so using semantic elements should be preferred.

@codecov
Copy link

codecov bot commented Sep 18, 2022

Codecov Report

Merging #1134 (486a6f7) into main (6c39138) will increase coverage by 0.27%.
The diff coverage is 94.28%.

@@            Coverage Diff             @@
##             main    #1134      +/-   ##
==========================================
+ Coverage   87.40%   87.68%   +0.27%     
==========================================
  Files         150      154       +4     
  Lines        2843     2906      +63     
  Branches      459      467       +8     
==========================================
+ Hits         2485     2548      +63     
- Misses        252      253       +1     
+ Partials      106      105       -1     
Flag Coverage Δ
unittest 87.68% <94.28%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/eslint-plugin-template/src/index.ts 67.34% <0.00%> (+0.68%) ⬆️
.../rules/accessibility-interactive-supports-focus.ts 91.66% <91.66%> (ø)
...t-plugin-template/src/utils/is-content-editable.ts 100.00% <100.00%> (ø)
...t-plugin-template/src/utils/is-disabled-element.ts 100.00% <100.00%> (ø)
...lement/get-non-interactive-element-role-schemas.ts 100.00% <100.00%> (ø)
...template/src/utils/is-interactive-element/index.ts 100.00% <100.00%> (ø)
.../accessibility-interactive-supports-focus/cases.ts 100.00% <100.00%> (ø)
... and 2 more

@sandikbarr sandikbarr force-pushed the feat/template/interactive-supports-focus branch from f10937a to 486a6f7 Compare September 18, 2022 22:35
!tabIndex &&
!isInteractiveElement(node) &&
!isNonInteractiveRole(node) &&
!isContentEditable(node)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to support contenteditable: I was realizing I had used only (click) events in my test cases and updated some to use keyup, keydown, and keypress events and realized those key events might be used for contenteditable which is focusable and interactive by default.

@@ -3,6 +3,7 @@
"rules": {
"@angular-eslint/template/accessibility-alt-text": "error",
"@angular-eslint/template/accessibility-elements-content": "error",
"@angular-eslint/template/accessibility-interactive-supports-focus": "error",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the rule name to follow the accessibility- name prefix convention.

@sandikbarr sandikbarr changed the title feat(eslint-plugin-template): [interactive-supports-focus] add rule feat(eslint-plugin-template): [accessibility-interactive-supports-focus] add rule Sep 18, 2022
@JamesHenry
Copy link
Member

@adbaran1 are you happy with the current state of this PR?

@abaran30
Copy link
Contributor

@adbaran1 are you happy with the current state of this PR?

Looks good to me! Thank you for making the adjustments @sandikbarr!

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sandikbarr and @adbaran1!

@JamesHenry JamesHenry merged commit d99d8c1 into angular-eslint:main Sep 26, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Nov 18, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular-eslint/builder](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`14.1.2` -> `14.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2fbuilder/14.1.2/14.2.0) |
| [@angular-eslint/eslint-plugin](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`14.1.2` -> `14.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2feslint-plugin/14.1.2/14.2.0) |
| [@angular-eslint/eslint-plugin-template](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`14.1.2` -> `14.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2feslint-plugin-template/14.1.2/14.2.0) |
| [@angular-eslint/schematics](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`14.1.2` -> `14.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2fschematics/14.1.2/14.2.0) |
| [@angular-eslint/template-parser](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`14.1.2` -> `14.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2ftemplate-parser/14.1.2/14.2.0) |

---

### Release Notes

<details>
<summary>angular-eslint/angular-eslint (@&#8203;angular-eslint/builder)</summary>

### [`v14.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/builder/CHANGELOG.md#&#8203;1420-httpsgithubcomangular-eslintangular-eslintcomparev1412v1420-2022-11-15)

[Compare Source](angular-eslint/angular-eslint@v14.1.2...v14.2.0)

**Note:** Version bump only for package [@&#8203;angular-eslint/builder](https://github.com/angular-eslint/builder)

#### [14.1.2](angular-eslint/angular-eslint@v14.1.1...v14.1.2) (2022-09-21)

##### Bug Fixes

-   **builder:** remove nrwl/devkit from builder implementation ([#&#8203;1142](angular-eslint/angular-eslint#1142)) ([9d5a7fc](angular-eslint/angular-eslint@9d5a7fc))

#### [14.1.1](angular-eslint/angular-eslint@v14.1.0...v14.1.1) (2022-09-18)

**Note:** Version bump only for package [@&#8203;angular-eslint/builder](https://github.com/angular-eslint/builder)

</details>

<details>
<summary>angular-eslint/angular-eslint (@&#8203;angular-eslint/eslint-plugin)</summary>

### [`v14.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;1420-httpsgithubcomangular-eslintangular-eslintcomparev1412v1420-2022-11-15)

[Compare Source](angular-eslint/angular-eslint@v14.1.2...v14.2.0)

##### Bug Fixes

-   update typescript-eslint packages to v5.38.1 ([#&#8203;1152](angular-eslint/angular-eslint#1152)) ([8f6d0ef](angular-eslint/angular-eslint@8f6d0ef))
-   update typescript-eslint packages to v5.43.0 ([#&#8203;1190](angular-eslint/angular-eslint#1190)) ([2a4716a](angular-eslint/angular-eslint@2a4716a))

##### Features

-   update typescript-eslint packages to v5.38.0 ([#&#8203;1140](angular-eslint/angular-eslint#1140)) ([85b4b47](angular-eslint/angular-eslint@85b4b47))

#### [14.1.2](angular-eslint/angular-eslint@v14.1.1...v14.1.2) (2022-09-21)

**Note:** Version bump only for package [@&#8203;angular-eslint/eslint-plugin](https://github.com/angular-eslint/eslint-plugin)

#### [14.1.1](angular-eslint/angular-eslint@v14.1.0...v14.1.1) (2022-09-18)

##### Bug Fixes

-   **eslint-plugin:** \[sort-ngmodule-metadata-arrays]: add intl support ([#&#8203;1099](angular-eslint/angular-eslint#1099)) ([30d133b](angular-eslint/angular-eslint@30d133b))

</details>

<details>
<summary>angular-eslint/angular-eslint (@&#8203;angular-eslint/eslint-plugin-template)</summary>

### [`v14.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/eslint-plugin-template/CHANGELOG.md#&#8203;1420-httpsgithubcomangular-eslintangular-eslintcomparev1412v1420-2022-11-15)

[Compare Source](angular-eslint/angular-eslint@v14.1.2...v14.2.0)

##### Bug Fixes

-   update dependency aria-query to v5.1.3 ([#&#8203;1183](angular-eslint/angular-eslint#1183)) ([7c5b299](angular-eslint/angular-eslint@7c5b299))
-   update dependency axobject-query to v3.1.1 ([#&#8203;1184](angular-eslint/angular-eslint#1184)) ([dcfd43d](angular-eslint/angular-eslint@dcfd43d))
-   update typescript-eslint packages to v5.38.1 ([#&#8203;1152](angular-eslint/angular-eslint#1152)) ([8f6d0ef](angular-eslint/angular-eslint@8f6d0ef))
-   update typescript-eslint packages to v5.43.0 ([#&#8203;1190](angular-eslint/angular-eslint#1190)) ([2a4716a](angular-eslint/angular-eslint@2a4716a))

##### Features

-   **eslint-plugin-template:** \[accessibility-interactive-supports-focus] add rule ([#&#8203;1134](angular-eslint/angular-eslint#1134)) ([d99d8c1](angular-eslint/angular-eslint@d99d8c1))
-   **eslint-plugin-template:** \[accessibility-role-has-required-aria] add rule ([#&#8203;1100](angular-eslint/angular-eslint#1100)) ([f684df0](angular-eslint/angular-eslint@f684df0))
-   **eslint-plugin-template:** \[attributes-order] add rule with fixer ([#&#8203;1066](angular-eslint/angular-eslint#1066)) ([4c789c7](angular-eslint/angular-eslint@4c789c7))
-   **eslint-plugin-template:** \[no-duplicate-attributes] Add option to ignore properties ([#&#8203;1104](angular-eslint/angular-eslint#1104)) ([018d390](angular-eslint/angular-eslint@018d390))
-   update typescript-eslint packages to v5.38.0 ([#&#8203;1140](angular-eslint/angular-eslint#1140)) ([85b4b47](angular-eslint/angular-eslint@85b4b47))

#### [14.1.2](angular-eslint/angular-eslint@v14.1.1...v14.1.2) (2022-09-21)

**Note:** Version bump only for package [@&#8203;angular-eslint/eslint-plugin-template](https://github.com/angular-eslint/eslint-plugin-template)

#### [14.1.1](angular-eslint/angular-eslint@v14.1.0...v14.1.1) (2022-09-18)

##### Bug Fixes

-   **eslint-plugin-template:** \[click-events-have-key-events]: handle additional outputs ([#&#8203;1101](angular-eslint/angular-eslint#1101)) ([c608cdb](angular-eslint/angular-eslint@c608cdb))

</details>

<details>
<summary>angular-eslint/angular-eslint (@&#8203;angular-eslint/schematics)</summary>

### [`v14.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/schematics/CHANGELOG.md#&#8203;1420-httpsgithubcomangular-eslintangular-eslintcomparev1412v1420-2022-11-15)

[Compare Source](angular-eslint/angular-eslint@v14.1.2...v14.2.0)

##### Bug Fixes

-   update dependency eslint to v8.27.0 ([#&#8203;1189](angular-eslint/angular-eslint#1189)) ([d2ae95a](angular-eslint/angular-eslint@d2ae95a))
-   update typescript-eslint packages to v5.38.1 ([#&#8203;1152](angular-eslint/angular-eslint#1152)) ([8f6d0ef](angular-eslint/angular-eslint@8f6d0ef))
-   update typescript-eslint packages to v5.43.0 ([#&#8203;1190](angular-eslint/angular-eslint#1190)) ([2a4716a](angular-eslint/angular-eslint@2a4716a))

##### Features

-   update dependency eslint to v8.24.0 ([#&#8203;1148](angular-eslint/angular-eslint#1148)) ([5f30b2d](angular-eslint/angular-eslint@5f30b2d))
-   update typescript-eslint packages to v5.38.0 ([#&#8203;1140](angular-eslint/angular-eslint#1140)) ([85b4b47](angular-eslint/angular-eslint@85b4b47))

#### [14.1.2](angular-eslint/angular-eslint@v14.1.1...v14.1.2) (2022-09-21)

**Note:** Version bump only for package [@&#8203;angular-eslint/schematics](https://github.com/angular-eslint/schematics)

#### [14.1.1](angular-eslint/angular-eslint@v14.1.0...v14.1.1) (2022-09-18)

**Note:** Version bump only for package [@&#8203;angular-eslint/schematics](https://github.com/angular-eslint/schematics)

</details>

<details>
<summary>angular-eslint/angular-eslint (@&#8203;angular-eslint/template-parser)</summary>

### [`v14.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/template-parser/CHANGELOG.md#&#8203;1420-httpsgithubcomangular-eslintangular-eslintcomparev1412v1420-2022-11-15)

[Compare Source](angular-eslint/angular-eslint@v14.1.2...v14.2.0)

##### Bug Fixes

-   update dependency eslint-scope to v7 ([#&#8203;1156](angular-eslint/angular-eslint#1156)) ([05bd9e6](angular-eslint/angular-eslint@05bd9e6))

#### [14.1.2](angular-eslint/angular-eslint@v14.1.1...v14.1.2) (2022-09-21)

**Note:** Version bump only for package [@&#8203;angular-eslint/template-parser](https://github.com/angular-eslint/template-parser)

#### [14.1.1](angular-eslint/angular-eslint@v14.1.0...v14.1.1) (2022-09-18)

**Note:** Version bump only for package [@&#8203;angular-eslint/template-parser](https://github.com/angular-eslint/template-parser)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4xIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMSJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1641
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants