cannot add 'draggable' attr to img

[zhaoyao91]

IMPORTANT: If you have a question or an idea for a new feature use Github Discussions instead

Describe the bug

this is no effect when adding draggable attr to img.

To Reproduce

see https://playground.solidjs.com/#NobwRAlgJmBcYBYCuBTANhtBbAzlAVgAwAOAZgHYBsAFgOzlrWlgA0Y5AhlinGFhxHKswAFwCexHvBE4AHsJwB7JACcAxlMhZiilSIAEIfSpTkoKFfoC++0isVZ9AciVpoAWnw4A9AHcUAEZOANwAOuQQ2roGRmomHCIoAMoQAOacaNa29o4uim5Qnjgh4eGkSORqIhCK5PoAwsrkiSoAFACUhuH6xigiqnWt3T36ADyRqfo46gC8oWDUIiLEOLDe3sRoHGKp9hVQAHSu0F4Hag7eHDg4fT5oiqmKBwCMAEwAzAGECAFqRwBuqXm+l80BE1DmYFeCGB1BQaUWkOhwKgKg4qVSHACaBQMxApA4aBuNm8AD5hu0wuQrKVyCYzBZWh19DNSWNGhUWvoySx9FBFGokNxmgdUn0AKI44UiABCYgAklBWk4OMRiE52u1wmArABdIA

Expected behavior

cannot drag img

Reproduction

[ryansolid]

Yeah it's an annoying property: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/draggable

Basically it looks like boolean but isn't boolean. I'm not going to mess too much here. Just fix the types. I compile

<img draggable={false} />
// to
el.setAttribute("draggable", false)

Which doesn't work.. which IMHO just dumb. Know what does work according to the doc:

el.setAttribute("draggable", "false")
// or
<img draggable="false" />

I'm pretty sure this is why React toStrings everything but also why it completely breaks web components. Draggable isn't worth it. I'm going to fix the types to reflect that draggable takes strings.

[ryansolid]

No I found a better fix. Looked at what Svelte does. I will get this working in the next version.

[ryansolid]

Should be fixed in 0.25.0. Reopen if still an issue. (keep in mind playground isn't updated yet)

[zhaoyao91]

There is breaking change after upgrading from ^0.24.1 to 0.25.0

I have such code

  return <div  data-loading={props.isLoading} />;

if props.isLoading is false, before upgrading, there is no data-loading attr in div element, but after upgrading, there is a data-loading='false' on it. I am not sure which behavior is expected.

[ryansolid]

Oh right yes. This is caused by this very fix.

I followed suite with other frameworks. We set strings on attributes now on false rather than remove unless they are known boolean attributes. I was thinking of forcing end user to do strings but every framework I saw handled it the other way. So this fixes draggable, contenteditable, and a handful of other native dom booleans that aren't boolean attributes but causes this. If you want to remove the attribute it has to be set to null or undefined.

[zhaoyao91]

well, in my opinion, the previous behaviour is more nature: boolean to toggle the attr and string is what as it is.
so for draggable, the only fix is the type declaration changing to 'true' | 'false'.

I don't know if there is spec guiding other framework to do so.

[ryansolid]

The problem is it isn't just draggable. It's any boolean non-boolean. There are a number of these properties. Boolean attributes are treated differently, unfortunately. This means in terms of custom properties like data-loading you can't really expect boolean attribute behavior.

What I wrote earlier wasn't a hundred percent true. My helper checking for false and removing the attributes. But it was from a time before I was detecting boolean attributes properly. Calling setAttribute on anything forces string coercion. So el.setAttribute("draggable", false); does work and sets false as a the string "false". It takes special behavior to remove the attribute. Boolean attributes are still true when you set any value, like el.setAttribute("hidden", false) ends up being true. If anything boolean attributes are the weird ones since not being present is the criteria for being not set. But this is not reflective of the native DOM.

So from that perspective it seems really reasonable this is what other frameworks do and it basically reflects how the DOM works, and why it seems like the logical choice here.

[zhaoyao91]

I found lit-html creates a new syntax ?disabled to handle boolean case specifically. Maybe we could take similar approach?

[ryansolid]

That's interesting. The closest we could do is namespace as JSX identifiers can't have special symbols other than $. Like bool:data-loading. TS will be annoying. However prop: might already accomplish the same in most cases.

[TrueMrMaverick]

I'm still facing an issue with draggable.
It appears that I need to specifically set draggable like:

<div draggable={true} />

Is it intended or in the future, it can be improved to a bit more common:

<div draggable />

[ryansolid]

@TrueMrMaverick
This is intended behavior because it is how the browser works. Draggable is weird attribute. See the warning: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/draggable. Unfortunately it isn't the only one like this.

It used to work that way but it causes problems for other "boolean non-booleans" (as I'm calling them now). The decision was to follow what the browser does rather than force it one way and having no way to force it the other way. Following browser conventions while less consistent avoid a lot of headaches later when you are now responsible for any edge cases.

[TrueMrMaverick]

@ryansolid thanks for such a quick response!
I'm only beginning to work with Solid and based on your answer I have the following question.
Does it mean that I should set attributes and props explicitly in Solid?
Maybe I missed something in the documentation...

[ryansolid]

I'm saying we follow browser standards on things.. like if draggable isn't a boolean property.. ex Chrome, Firefox, and Safari doesn't support:

<div draggable />

We aren't going to support it. Generally speaking on native elements we follow attribute conventions of the browser. So you can just refer to MDN docs to know what or how they work.

React and some of the other frameworks sort of tried to normalize this stuff, but years later we're now trying to undo that work because it leads to equally inconsistent (and often less documented) behaviors.