name attribute?Additional Questions:
Does the component accept additional HTML attributes? For example, adding an aria attribute to a button.
As far as I could see we're not taking this into account ... yet.
As discussed in other "review" issue tickets, it might be better to come up with some common patterns concerning these extra attributes before adding anything we might end up reverting.
Is there anything about the public API that you would change?
Not at the moment.
Is the naming of the available props clear?
As far as I'm concerned that's a "yes", because it's just of matter of reading the billboard.js documentation and examples.
I've also had other people ask me about these charts and their usage (with the previous metal+soy version) and I usually suggested to look at the billboard.js documentation too.
I don't know how to do this in the most efficient way but it would be nice to specify that this is "just" a thin wrapper on top of billboard.js because it sometimes seems that wasn't the expectation. @bryceosterhaus @matuzalemsteles what do you guys think?
Yeah you are right, we could do a better job at pointing to the billboard docs. I haven't look at charts in awhile, but I think we might benefit from going through it one more time and trying to line of the API to be as close to billboard as possible and any unnecessary logic on our end.
@bryceosterhaus yes that sounds like a good idea too - I haven't seen too much unnecessary stuff too far.
@julien I went ahead and added an additional prop for providing attributes to the charts container element. Let me know if this seems appropriate https://github.com/liferay/clay/pull/2514. I didn't use ...otherProps since I wanted to leave that open for other billboard config and API.
@bryceosterhaus yes that looks appropriate to me. Thanks.