Skip to content

✨ Allow SVG 'focusable' attribute from SVG Tiny 1.2#19128

Merged
Gregable merged 1 commit intoampproject:masterfrom
westonruter:add/svg-focusable-attribute
Nov 6, 2018
Merged

✨ Allow SVG 'focusable' attribute from SVG Tiny 1.2#19128
Gregable merged 1 commit intoampproject:masterfrom
westonruter:add/svg-focusable-attribute

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Nov 3, 2018

In working on adding AMP compatibility to the next default WordPress theme Twenty Nineteen (ampproject/amp-wp#1587), I found that it was outputting a focusable attribute in an svg element like so:

<svg class="svg-icon" width="24" height="24" aria-hidden="true" role="img" focusable="false" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
    <path d="M7.41 8.59L12 13.17l4.59-4.58L18 10l-6 6-6-6 1.41-1.41z"></path>
    <path fill="none" d="M0 0h24v24H0V0z"></path>
</svg>

The focusable attribute is defined in SVG Tiny 1.2: https://www.w3.org/TR/SVGTiny12/interact.html#focusable-attr

Fixes #19107

/cc @Gregable

Copy link
Copy Markdown
Contributor

@aghassemi aghassemi left a comment

Choose a reason for hiding this comment

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

LGTM, deferring to @Gregable for final approval

@aghassemi
Copy link
Copy Markdown
Contributor

(added Fixes https://github.com/ampproject/amphtml/issues/19107 to description)

@Gregable
Copy link
Copy Markdown
Member

Gregable commented Nov 6, 2018

LGTM.

@ChrisBAshton
Copy link
Copy Markdown

Thanks for fixing. Do we know when the new version is likely to be published?

Last release was 9 months ago: https://www.npmjs.com/package/amphtml-validator

@aghassemi
Copy link
Copy Markdown
Contributor

@ChrisBAshton about 1-2 weeks. The publish date of https://www.npmjs.com/package/amphtml-validator is not representative since rules are fetched dynamically.

alin04 pushed a commit to alin04/amphtml that referenced this pull request Nov 13, 2018
@alin04 alin04 mentioned this pull request Nov 13, 2018
alin04 added a commit that referenced this pull request Nov 13, 2018
* cl/220306609 Revision bump for #17907

* cl/220307253 Revision bump for #19128

* cl/220310523 Revision bump for #19129

* cl/220399983 Revision bump for #19167

* cl/221145203 n/a

* cl/221159765 Revision bump for #19214

* cl/221164382 Invalidate `<amp-script>` tag as well

* cl/221176616 Revision bump for #17939

* cl/221181356 Revision bump for #19171
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
* cl/220306609 Revision bump for ampproject#17907

* cl/220307253 Revision bump for ampproject#19128

* cl/220310523 Revision bump for ampproject#19129

* cl/220399983 Revision bump for ampproject#19167

* cl/221145203 n/a

* cl/221159765 Revision bump for ampproject#19214

* cl/221164382 Invalidate `<amp-script>` tag as well

* cl/221176616 Revision bump for ampproject#17939

* cl/221181356 Revision bump for ampproject#19171
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants