[SVG] Support "focusable" attribute

[Kureev]

Currently, if I supply a focusable attribute to the SVG, it shrinks it out. Seems it happens because there is no focusable key in SVGDOMPropertyConfig.js.

So is there any chance you merge a PR which adds this property to the list?

[gaearon]

React 15 removed whitelisting attributes for SVG completely in #5714. The names left in the whitelist are only there to print deprecation warnings. You will find that any arbitrary attribute, including focused, should get passed correctly in in any version >= 15 RC1.

React 15 RC1 was shipped yesterday so I would suggest you to check it out. I would expect 15 to arrive shortly within a few weeks.

[Kureev]

Thanks 👍

[gaearon]

Seems like we’re going to go back on that change (see #6243).

Are there any browsers that actually support focusable?
We can add it if that’s the case but from a quick look it doesn’t appear to be implemented.

[alexlande]

IE10, IE11, and Edge support focusable, unfortunately. If you don't specify focusable="false", SVG elements are added to the tab order by default in those browsers, with no way of removing them (tabindex is ignored).

It's a pretty annoying accessibility issue for keyboard users in those browsers.

[jimfb]

cc @tomocchino whitelists... :P

[KittyGiraudel]

Hey there. It seems that focusable needs to be passed false as a string and not a boolean so React (16) renders it in the DOM. Is that expected behaviour?

<svg
-    focusable={false}
+    focusable="false"

I find it rather surprising considering other boolean attributes such as aria-hidden appear to behave differently (passing false as a boolean properly renders it in the DOM).

Maybe I missed something?

[gaearon]

AFAIK it's expected because focusable is a string attribute with "true", "false" and "auto" values per spec, and not a boolean attribute. Yes, the spec is confusing.

[nhunzaker]

Yes. I can't remember if we achieved consensus on the best way to handle boolean attributes but, from what I remember, it follows:

• Attributes that require string html attribute values (like focusable="true") should be passed as strings
• Attributes that should either be present or absent (like disabled or novalidate) accept true and false. React adds/removes the attribute based on the boolean

I believe we have some inconsistency to retain backwards compatibility, though I'm not sure of a specific example off-hand.

---

• Attributes React treats as boolean

[nhunzaker]

Looks like we have a "boolean-ish" section for some SVG:
https://github.com/facebook/react/blob/master/packages/react-dom/src/shared/DOMProperty.js#L255-L267

Does it make sense to add focusable here?

[KittyGiraudel]

AFAIK it's expected because focusable is a string attribute with "true", "false" and "auto" values per spec, and not a boolean attribute. Yes, the spec is confusing.

That makes sense. I’m not sure accepting both a string or a boolean would be dramatically better. It would certainly make things less confusing from an authoring stand point, but introduces a bit of a code smell (expecting multiple types).

• Attributes that should either be present or absent (like disabled or novalidate) accept true and false. React adds/removes the attribute based on the boolean
• Attributes that require string html attribute values (like focusable="true") should be passed as strings

This is what confused me: aria-hidden doesn’t work this way for instance. As per the specifications, it should be passed as true or false (string), not just present (like disabled). But React is completely fine with it being passed without a value, or with a boolean.

It seems to me that we have an opportunity to make things a bit more consistent. Maybe by making focusable behave more like aria-hidden and similar props (especially since they are usually used together) by accepting a boolean value as well as "auto"? But then we’re back to my first comment: 2 different types.

[gaearon]

I think we're open to changes to this in case you want to write up a proposal.

[KittyGiraudel]

Yes, my colleagues and I can surely come up with something. Is there a specific format you’re expecting for this or shall it just be what we think would be a good improvement on the current implementation?

[gaearon]

Just file an issue with a free form description of how it should work and why

[aweary]

It's kind of a messy situation, since we're dealing with three separate specifications (HTML, SVG, ARIA) and they don't always use consistent terms when describing attribute behavior.

Ideally, every specification would define the attributes as either boolean attributes or enumerated attributes and we could just conform to those specs and only allow true/false for boolean attributes.

This would be the easiest approach from an implementation standpoint, but I do think there's value in making it work how the user expects it to. If an attribute is an enumerated attribute with "true" and "false" as keywords (a really unfortunate design) then taking the pit-of-success approach and making it just work feels like a better user experience.

I don't think having the attributes be polymorphic is that bad of a code smell. It just results in some simple coercion that React does anyways.

[KittyGiraudel]

Moved to #12481.