Doctype preamble for elem.Html
Closed this issue · 4 comments
When using elem.Html, everything gets neatly wrapped in an <html></html> element as expected.
But it's expected/required by browsers to have a <!DOCTYPE html> preamble. It'll work without it, but you immediately get a warning in the console as without the preamble you're rendering in the legacy/quirks mode which is usually not desirable. Since a document can only have a single html element, it seems like elem-go could safely emit it.
So two questions:
- Have I missed some built-in way of doing this?
- I'm happy to submit a PR to add this, but it seems the way to do it would be to special case the Html element in
RenderTo()which feels a bit iffy. Is there a better way?
@daenney Great catch! That would be great if you could add it. Maybe, we could do something like:
func Html(attrs attrs.Props, children ...Node) *Element {
return NewElement("html", attrs, children...).withCustomOptions(CustomOptions{OpeningTag: "<!DOCTYPE html>"})
}I've been avoiding chaining since the pattern isn't used elsewhere in the library, but in this case since it's not an exposed API and it would be used so infrequently, I'm not opposed to it. What do you think? I guess we could also have a new function like NewElementWithCustomRender but that may duplicate more code than we'd like.
@daenney What I wrote earlier was incorrect since the doctype preamble actually comes before the <html>. Apologies for the oversight. Because <!DOCTYPE html> is so common, I think it's probably fine to just hard code it in the Render method:
func (e *Element) Render() string {
var builder strings.Builder
// Automatically add DOCTYPE for 'html' elements
if e.Tag == "html" {
builder.WriteString("<!DOCTYPE html>")
}
e.RenderTo(&builder)
return builder.String()
}If anyone needs different preambles down the road we could have a custom option in Render maybe. I think the simple approach is fine here until additional customization is requested. Do you still want to take this?
Ah fair enough. Sorry for not responding to the earlier comment, it completely slipped my mind. I'll take a look at whipping up a PR for this next week.