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
fix a11y-svg-has-accessible-name
considering whitespace JSXText
#508
Conversation
👋 Hello and thanks for pinging us! You've entered our first responder queue. An accessibility first responder will review this soon.
|
Hi @nnmrts thanks for your contribution! I'll be able to review this in the next day or so and let you know. Thanks for raising that this is causing issues for you and we certainly want to get you unblocked as soon as possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good as a proposal. Can you add a test that validates for this whitespace scenario in tests/a11y-svg-has-accessible-name.js
and ensure the linters pass as well? I can get this approved after that.
Thanks for the suggestion!
Co-authored-by: Andri Alexandrou <andrialexandrou@github.com>
fix code style
@andrialexandrou Thanks for taking a look! Sorry that it took me 2 full days, I added a test now, which initially didn't pass until I fixed some prettier issues, so I had to change EDIT: I hope it's okay that I added the test as the second case, I felt like it's related to the first one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem at all, thanks for making these contributions. To your question, placement there seems just fine.
Approving!
@nnmrts your work is available in the latest release, which you can update via npm: |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint-plugin-github](https://togithub.com/github/eslint-plugin-github) | devDependencies | patch | [`4.10.1` -> `4.10.2`](https://renovatebot.com/diffs/npm/eslint-plugin-github/4.10.1/4.10.2) | --- ### Release Notes <details> <summary>github/eslint-plugin-github (eslint-plugin-github)</summary> ### [`v4.10.2`](https://togithub.com/github/eslint-plugin-github/releases/tag/v4.10.2) [Compare Source](https://togithub.com/github/eslint-plugin-github/compare/v4.10.1...v4.10.2) #### What's Changed ##### Functionality - Update no-then.md by [@​cefn](https://togithub.com/cefn) in [github/eslint-plugin-github#503 - fix `a11y-svg-has-accessible-name` considering whitespace JSXText by [@​nnmrts](https://togithub.com/nnmrts) in [github/eslint-plugin-github#508 ##### Dependancy updates - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#482 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#484 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#485 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#486 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#487 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#489 - chore(deps): bump actions/setup-node from 3 to 4 by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#488 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#490 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#491 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#492 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#493 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#494 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#495 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#496 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#498 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#499 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#500 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#501 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#502 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#504 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#507 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#509 #### New Contributors - [@​cefn](https://togithub.com/cefn) made their first contribution in [github/eslint-plugin-github#503 - [@​nnmrts](https://togithub.com/nnmrts) made their first contribution in [github/eslint-plugin-github#508 **Full Changelog**: github/eslint-plugin-github@v4.10.1...v4.10.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjIyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: Balvajs Renovate Bot <406530+balvajs-renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint-plugin-github](https://togithub.com/github/eslint-plugin-github) | devDependencies | patch | [`4.10.1` -> `4.10.2`](https://renovatebot.com/diffs/npm/eslint-plugin-github/4.10.1/4.10.2) | --- ### Release Notes <details> <summary>github/eslint-plugin-github (eslint-plugin-github)</summary> ### [`v4.10.2`](https://togithub.com/github/eslint-plugin-github/releases/tag/v4.10.2) [Compare Source](https://togithub.com/github/eslint-plugin-github/compare/v4.10.1...v4.10.2) #### What's Changed ##### Functionality - Update no-then.md by [@​cefn](https://togithub.com/cefn) in [github/eslint-plugin-github#503 - fix `a11y-svg-has-accessible-name` considering whitespace JSXText by [@​nnmrts](https://togithub.com/nnmrts) in [github/eslint-plugin-github#508 ##### Dependancy updates - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#482 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#484 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#485 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#486 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#487 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#489 - chore(deps): bump actions/setup-node from 3 to 4 by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#488 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#490 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#491 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#492 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#493 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#494 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#495 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#496 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#498 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#499 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#500 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#501 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#502 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#504 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#507 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#509 #### New Contributors - [@​cefn](https://togithub.com/cefn) made their first contribution in [github/eslint-plugin-github#503 - [@​nnmrts](https://togithub.com/nnmrts) made their first contribution in [github/eslint-plugin-github#508 **Full Changelog**: github/eslint-plugin-github@v4.10.1...v4.10.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjIyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: Balvajs Renovate Bot <406530+balvajs-renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [eslint-plugin-github](https://togithub.com/github/eslint-plugin-github) | [`4.10.1` -> `4.10.2`](https://renovatebot.com/diffs/npm/eslint-plugin-github/4.10.1/4.10.2) | [![age](https://developer.mend.io/api/mc/badges/age/npm/eslint-plugin-github/4.10.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/eslint-plugin-github/4.10.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/eslint-plugin-github/4.10.1/4.10.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/eslint-plugin-github/4.10.1/4.10.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>github/eslint-plugin-github (eslint-plugin-github)</summary> ### [`v4.10.2`](https://togithub.com/github/eslint-plugin-github/releases/tag/v4.10.2) [Compare Source](https://togithub.com/github/eslint-plugin-github/compare/v4.10.1...v4.10.2) #### What's Changed ##### Functionality - Update no-then.md by [@​cefn](https://togithub.com/cefn) in [github/eslint-plugin-github#503 - fix `a11y-svg-has-accessible-name` considering whitespace JSXText by [@​nnmrts](https://togithub.com/nnmrts) in [github/eslint-plugin-github#508 ##### Dependancy updates - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#482 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#484 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#485 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#486 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#487 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#489 - chore(deps): bump actions/setup-node from 3 to 4 by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#488 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#490 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#491 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#492 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#493 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#494 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#495 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#496 - chore(deps): bump the all-dependencies group with 4 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#498 - chore(deps): bump the all-dependencies group with 2 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#499 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#500 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#501 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#502 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#504 - chore(deps): bump the all-dependencies group with 5 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#507 - chore(deps): bump the all-dependencies group with 3 updates by [@​dependabot](https://togithub.com/dependabot) in [github/eslint-plugin-github#509 #### New Contributors - [@​cefn](https://togithub.com/cefn) made their first contribution in [github/eslint-plugin-github#503 - [@​nnmrts](https://togithub.com/nnmrts) made their first contribution in [github/eslint-plugin-github#508 **Full Changelog**: github/eslint-plugin-github@v4.10.1...v4.10.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 4am on Monday" in timezone Europe/Berlin, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/WtfJoke/setup-tectonic). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMjAuMiIsInVwZGF0ZWRJblZlciI6IjM3LjIyMC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Hello!
This PR fixes an issue I encountered with
a11y-svg-has-accessible-name
. I'm not sure if it's relevant to any "vanilla" use-cases of this plugin, eslint and/or react. My stack is probably very different (deno, fresh, preact) from many users of this plugin and the sole reason I encounter this issue (it's probably Preact, right?), but this PR shouldn't introduce any breaking change to "normal" users.In my case, the issue was the following:
Even though I added a title element here, this plugin errored
a11y-svg-has-accessible-name
. After inspecting how this rule is implemented, I saw that it checks if the first child is of typeJSXElement
andopeningElement?.name?.name === 'title'
.For some reason the JSX parser, whichever it is in my case, inserts a node of type
JSXText
with a value of\n\t\t\t
as the first child of thesvg
element. I get where it's coming from, since there is actual whitespace in my source code, but I feel like whichever parser does this, should just ignore whitespace here. Maybe this is an upstream issue, but this PR, which filters these empty nodes out before checking, seemed like a quicker fix.