added DataAttr and DataAttrs function #78
Conversation
attrs/utils_test.go
Outdated
| @@ -0,0 +1,20 @@ | |||
| package attrs_test | |||
There was a problem hiding this comment.
This can just be package attrs then you can remove the attrs. from usage in this file.
attrs/utils.go
Outdated
| } | ||
|
|
||
| // Return Props of data attributes build from given name-value pairs | ||
| func DataAttrs(pairs map[string]string) Props { |
There was a problem hiding this comment.
Great idea on also adding a way to add multiple considering these may be less unique that other attributes! What do you think about introducing a reusable type named KeyValueMap or something similar, to avoid constantly writing map[string]string? It could streamline things for users. However, I'm a bit cautious about adding complexity that might make the library harder for developers to grasp. What do you think? Is the trade-off worth it?
attrs/utils.go
Outdated
|
|
||
| import "strings" | ||
|
|
||
| // Return name for a data attribute |
There was a problem hiding this comment.
Go convention is to start comments with the function name. It's helps godoc generate documentation. Can we change the comments to // DataAttr returns the name for a data attribute. please?
attrs/utils.go
Outdated
| return builder.String() | ||
| } | ||
|
|
||
| // Return Props of data attributes build from given name-value pairs |
There was a problem hiding this comment.
Same note on comment here. Thanks!
attrs/utils.go
Outdated
| } | ||
|
|
||
| // DataAttrs returns Props of data attributes build from given name-value pairs | ||
| func DataAttrs(pairs map[string]string) Props { |
There was a problem hiding this comment.
We might need to only have the DataAttr and not DataAttrs function. Without some sort of props merge I'm not sure this will work and may be confusing to the developer. Let's just simplify with the single DataAttr method for now. Sorry I didn't catch this earlier. Thanks.
Issue: #73
Example usage: