Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render dictionaries in the positional argument of a tag as tag attributes #13

Merged
merged 5 commits into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 48 additions & 11 deletions htmltools/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ class MetadataNode:

TagT = TypeVar("TagT", bound="Tag")

# Types that can be passed in as attributes to tag functions.
TagAttrArg = Union[str, float, bool, None]

# Types of objects that can be a child of a tag.
TagChild = Union["Tagifiable", "Tag", MetadataNode, str]

Expand All @@ -79,13 +82,11 @@ class MetadataNode:
"TagList",
float,
None,
Dict[str, TagAttrArg], # i.e., tag attrbutes (e.g., {"id": "foo"})
List["TagChildArg"],
Tuple["TagChildArg", ...],
]

# Types that can be passed in as attributes to tag functions.
TagAttrArg = Union[str, int, float, bool, None]


# Objects with tagify() methods are considered Tagifiable. Note that an object returns a
# TagList, the children of the TagList must also be tagified.
Expand Down Expand Up @@ -245,9 +246,11 @@ def _repr_html_(self) -> str:
# TagAttrs class
# =============================================================================
class TagAttrs(Dict[str, str]):
def __init__(self, **kwargs: TagAttrArg) -> None:
def __init__(
self, attrs: Mapping[str, TagAttrArg] = {}, **kwargs: TagAttrArg
) -> None:
super().__init__()
self.update(**kwargs)
self.update(attrs, **kwargs)

def __setitem__(self, name: str, value: TagAttrArg) -> None:
val = self._normalize_attr_value(value)
Expand All @@ -265,11 +268,32 @@ def update( # type: ignore

def _update(self, __m: Mapping[str, TagAttrArg]) -> None:
attrs: Dict[str, str] = {}
for key, val in __m.items():
val_ = self._normalize_attr_value(val)
if val_ is None:

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
Copy link
Collaborator

@wch wch Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd still be possible via __setitem__, but I suppose it'd be confusing in that it differs from usual update() behavior


super().update(**attrs)

@staticmethod
Expand Down Expand Up @@ -339,10 +363,23 @@ def __init__(
**kwargs: TagAttrArg,
) -> None:
self.name: str = _name
self.attrs: TagAttrs = TagAttrs(**kwargs)
self.attrs: TagAttrs = TagAttrs()

# As a workaround for Python not allowing for numerous keyword
# arguments of the same name, we treat any dictionaries in the
# positional arguments as attributes.
child_args: List[TagChildArg] = []
for arg in _flatten(args):
if isinstance(arg, dict):
self.attrs.update(arg)
else:
child_args.append(arg)

self.attrs.update(kwargs)

self.children: TagList = TagList()

self.children.extend(args)
self.children.extend(child_args)
if children:
self.children.extend(children)

Expand Down
14 changes: 12 additions & 2 deletions tests/test_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ def test_tag_attrs_update():
assert x.attrs == {"a": "1", "b": "2", "c": "C"}


def test_tag_multiple_repeated_attrs():
x = div({"class": "foo", "class_": "bar"}, class_="baz")
y = div({"class": "foo"}, {"class_": "bar"}, class_="baz")
z = div({"class": "foo"}, {"class": "bar"}, class_="baz")
assert x.attrs == {"class": "foo bar baz"}
assert y.attrs == {"class": "foo bar baz"}
assert z.attrs == {"class": "foo bar baz"}
x.attrs.update({"class": "bap", "class_": "bas"}, class_="bat")
assert x.attrs == {"class": "foo bar baz bap bas bat"}


def test_tag_shallow_copy():
dep = HTMLDependency(
"a", "1.1", source={"package": None, "subdir": "foo"}, script={"src": "a1.js"}
Expand Down Expand Up @@ -363,9 +374,8 @@ def test_attr_vals():


def test_tag_normalize_attr():
# Note that x_ maps to x, and it gets replaced by the latter.
x = div(class_="class_", x__="x__", x_="x_", x="x")
assert x.attrs == {"class": "class_", "x-": "x__", "x": "x"}
assert x.attrs == {"class": "class_", "x-": "x__", "x": "x_ x"}

x = div(foo_bar="baz")
assert x.attrs == {"foo-bar": "baz"}
Expand Down