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

Consider <title> for the name of its <svg> element. #324

Merged
merged 5 commits into from
Jul 13, 2020
Merged

Consider <title> for the name of its <svg> element. #324

merged 5 commits into from
Jul 13, 2020

Conversation

juanca
Copy link
Contributor

@juanca juanca commented Jul 12, 2020

<svg>
  <title>My SVG</title>
</svg>

Computing the name for this svg would have returned an empty string previously. It now correctly computes My SVG following the accessible object in the accessibility tree for rendered SVG elements.

Closes testing-library/dom-testing-library#685

```html
<svg>
  <title>My SVG</title>
</svg>
```

Computing the name for this svg would have returned an empty string previously. It now correctly computes `My SVG` following the [accessible object in the accessibility tree for rendered SVG elements](https://www.w3.org/TR/svg-aam-1.0/#include_elements).
@changeset-bot
Copy link

changeset-bot bot commented Jul 12, 2020

🦋 Changeset is good to go

Latest commit: 07e9bf3

We got this.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Owner

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Appreciate the PR and tests! I have some minor nitpicks about the name of these new methods and nuances about SVGElement and HTMLElement. They're likely not important but may cause problems in the future.

sources/util.ts Outdated Show resolved Hide resolved
sources/util.ts Outdated Show resolved Hide resolved
sources/util.ts Outdated Show resolved Hide resolved
sources/util.ts Outdated
@@ -56,6 +56,18 @@ export function isHTMLLegendElement(
return isElement(node) && node.tagName === "LEGEND";
}

export function isSVGElement(node: Node | null): node is SVGElement {
return isElement(node) && node.ownerSVGElement !== undefined;
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 was getting a ts error in my IDE:

Property 'ownerSVGElement' does not exist on type 'Element'.ts(2339)

I am not sure if this is the best way to look at the defined properties -- but on SVG elements, it is either defined or null. I figured elsewhere it would be undefined.

@juanca juanca requested a review from eps1lon July 13, 2020 19:13
sources/util.ts Outdated Show resolved Hide resolved
@eps1lon eps1lon added the enhancement New feature or request label Jul 13, 2020
@eps1lon eps1lon merged commit 85f0032 into eps1lon:main Jul 13, 2020
@eps1lon
Copy link
Owner

eps1lon commented Jul 13, 2020

@juanca Great work, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SVG nested <title> should be an accessible name
2 participants