Closed obecker closed 3 years ago
I like the idea, and you've listed good reasons to do this. Will this cause any breaking changes for users, or is this returning it to the state it was in at v1.4.0? I'm still learning some of this history of this tag so I'd just like to know if it fits for v 1.x or v2.
After reviewing the commit history, yes we should do this before the next release. Returning those tags to being generated fixes a lot of troubles for us.
@obecker did you plan on creating a PR for this, or should I work on it?
@sembler Here you are!
Fun fact: the manual body
tag missed a lot of onsomeevent attributes. Now with the generated code, this has been fixed as well.
I would propose to remove the specialized
HtmlTag
class (and alsoHeadTag
andBodyTag
) and just let them be generated like otherContainerTag
classes.ContainerTag html = html(body(h1("Hello, world!")));
doesn't compile anymore (HtmlTag
is not aContainerTag
)html().withLang("en").with(body(h1("Hello, world!")));
doesn't compile anymore, the attributes need to be moved to the end, resulting inhtml(body(h1("Hello, world!"))).withLang("en")
which is for real world scenarios impracticalrender
method implementation), which is IMHO somewhat error-prone (for example the first version simply forgot to render the attributes - see https://github.com/tipsy/j2html/issues/138#issuecomment-822171574 )body
tag and create HTML like thishtml(h1("Hello, world!"))
(see https://html.spec.whatwg.org/#syntax-tag-omission )html
element, but not for other elements with a comparable strict content model, for exampleul
,ol
,dl
, ortr
?Summarized: the above disadvantages outweight the (small and questionable) advantage of enforcing a correct content model for the
html
element , so we should replace the manual version of this class with the corresponding generated version.