added support for boolean attributes #48 #49
Conversation
attrs/attrs.go
Outdated
|
|
||
| // list of supported boolean attributes | ||
| // see https://html.spec.whatwg.org/multipage/indices.html#attributes-3 for reference | ||
| var BooleanAttrs = map[string]struct{}{ |
There was a problem hiding this comment.
- I'm not sure this needs to be publicly to people and since it's only used in
elem.gocan we move it there for now like thevoidElements? - Also, can we add the full list and add anything missing from the
attrsconstants to there. We can make separate ticket for that as well and link it in this as well or do it as a followup.
There was a problem hiding this comment.
I'm not sure this needs to be publicly to people and since it's only used in elem.go can we move it there for now like the voidElements?
I think you are probably right, though I feel it's more proper to keep this boolean attrs list close to attrs themselves. But for sake of keeping things simple I agree that is completely fine to have it in elem.go.
Also, can we add the full list and add anything missing from the attrs constants to there. We can make separate ticket for that as well and link it in this as well or do it as a followup.
I think it's better to add an attribute to the booleanAttrs map when we want't to support it in the code.
elem.go
Outdated
| @@ -91,6 +93,26 @@ func (e *Element) RenderTo(builder *strings.Builder) { | |||
| builder.WriteString(`>`) | |||
| } | |||
|
|
|||
| // return string representation of given attribute with its value | |||
| func (e *Element) renderAttr(attrName string) string { | |||
| var builder strings.Builder | |||
There was a problem hiding this comment.
This creates a new string for every attribute and slows down performance. Can we pass in the builder like appendAttrToBuilder(builder *strings.Builder, attrName string) and append the string to the existing builder instead of creating a new one?
There was a problem hiding this comment.
Thanks for pointing this out, I fixed this to use a builder.
Added support to boolean attributes.
HTML has 24 such attributes, but this package supports only 8 so far.