Render dictionaries in the positional argument of a tag as tag attributes #13
Conversation
… tag attributes
htmltools/core.py
Outdated
| for k, v in __m.items(): | ||
| val = self._normalize_attr_value(v) | ||
| if val is None: | ||
| continue | ||
| attrs[self._normalize_attr_name(key)] = val_ | ||
| nm = self._normalize_attr_name(k) | ||
|
|
||
| sep = " " | ||
| if isinstance(val, HTML): | ||
| sep = HTML(sep) | ||
|
|
||
| final_val = val | ||
| attr_val = attrs.get(nm) | ||
| if attr_val: | ||
| final_val = attr_val + sep + final_val | ||
|
|
||
| attrs[nm] = final_val | ||
|
|
||
| for k, v in attrs.items(): | ||
| self_v = self.get(k) | ||
| if self_v: | ||
| sep = " " | ||
| if isinstance(v, HTML): | ||
| sep = HTML(sep) | ||
| attrs[k] = self_v + sep + v |
There was a problem hiding this comment.
I think the string concatenation should go in the constructor (or maybe the Tag function?), instead of in the update method. For example, this makes sense:
div({"class": "a"}, class_="b")
#> <div class="a b"></div>But this would be surprising, and make it difficult (or impossible?) to replace attributes with a new value:
x = div(class_="a")
x.attrs.update(class_="b")
x
#> <div class="a b"></div>There was a problem hiding this comment.
It'd still be possible via __setitem__, but I suppose it'd be confusing in that it differs from usual update() behavior
|
As a general note, we want to support stuff like this: def foo(*args, **kwargs):
return div(*args, kwargs, {"class": "foo1"}, class_="foo2" )
def bar(*args, **kwargs):
return foo(*args, kwargs, {"class": "bar1"}, class_="bar2" )
bar(class_="myclass")
#> <div class="myclass bar1 bar2 foo1 foo2"></div>But we also want to keep the original |
htmltools/core.py
Outdated
| # Preserve the HTML() when combining two HTML() attributes | ||
| sep = HTML(" ") |
There was a problem hiding this comment.
I think that if we're combining an HTML-marked and non-HTML-marked attribute, the result should be an HTML-marked attribute (and the non-HTML attribute should be escaped immediately).
For example, with this code:
div({"foo": HTML("a<b")}, foo="c<d")It would be reasonable to expect the result to be:
<div foo="a<b c<d"></div>But as currently written in this PR, it would be:
<div foo="a<b c<d"></div>However, now that I think about it, it may make sense to do this in a separate PR and change how adding HTML() objects works in general (not just here).
There was a problem hiding this comment.
Also, if we do want this behavior, we should have tests for it.
There was a problem hiding this comment.
I agree that behavior would be sensible, but it seems like it'd add a fair amount of complexity to get similar behavior when concatenating HTML/non-HTML in general, which is the more common case (BTW, we don't support HTML() attributes in R):
>>> div(HTML("&") + " &")
<div>& &</div>So, given that we're not sure that's something we want to support in general, I think it makes sense to keep things this way for consistency. I filed an issue about it #15
Closes #10