hyperx
hyperx copied to clipboard
fix some attributes not being set properly under vdom
fixes #28
I tried to keep with the coding convention - hope this is OK.
also, I wasn't able to run the coverage script on windows without manually expanding the glob pattern :( ... there's a few failures when running that, but it's nothing that has been introduced here.
+ if (/data-/.test(key)) data(cur[1], key, parts[i][1])
Should that be /^data-/
?
I'm not sure that other implementations aside from virtual-dom use an attributes
property. If enough of them do, this seems ok but otherwise it would be best to put this behavior behind a flag or to have a separate module somewhere to wrap data attributes:
var h = require('virtual-dom/h')
var hx = require('hyperx')(function (tagName, props, children) {
if (!props.attributes) props.attributes = {}
Object.keys(props).forEach(function (key) {
if (/^data-/.test(key)) props.attributes[key] = props[key]
})
return h(tagName, attr, children)
})
Should that be
/^data-/
?
Yes... yes it should.
This approach in general is a good call. I found that when the change applied to all engines, hypertext promptly blew up. (seems it doesn't like attributes
being a object there; it tries to call forEach on it)
Anyway, I've added the flag as {vdom: true}
and updated the fix to apply to all the data-, aria- attributes, and when a style is provided as a string instead of an object. All of these should go under props.attributes
Any update on this?
I don't think hyperx should add logic specific to a particular virtual dom library. I think substack's custom factory suggestion is a better approach.
fwiw, that's basically what the PR does, but it puts it behind a flag so the library can handle it for users in a consistent manner.
If consumers do need to implement this in their own app, a note should probably be added to the readme about lack of aria-
, data-
and style
attributes when using virtual-dom.... add a sample implementation, and maybe a link to the vnode docs in virtual-dom (https://github.com/Matt-Esch/virtual-dom/blob/v2.1.1/docs/vnode.md#propertiesstyle-vs-propertiesattributesstyle)
For posterity, an implementation that supports data-
, aria-
and style
attributes:
var h = require('virtual-dom/h')
var hx = require('hyperx')(function (tagName, props, children) {
if (!props.attributes) props.attributes = {}
Object.keys(props).forEach(function (key) {
if (/^(data|aria)-/.test(key) || (key === 'style' && typeof props[key] === 'string')) {
props.attributes[key] = props[key]
}
})
return h(tagName, attr, children)
})
I think a note like that in the virtual-dom example section would be great, yes :)