Draft: add attribute control for components + docs

[maxblee]

This adds attribute-level control to the Svg, ScaledSvg, Canvas, Html, and Webgl layout layers. Addresses #71 .

[mhkeller]

Thanks again for this. I looked up a performance benchmark and it seems that Object.keys is slightly faster than Object.entries. I switched it but do you have a test scenario for this? If not, we should add this functionality to one example for each layout so we can use the website as a test.

[mhkeller]

I moved this into its own function so it wasn't repeated and I also added some type checking and a warning message. One question though is what if you want to dynamically unset these properties? Should this function make sure that the properties are always synced with the value of attrs? That seems most sveltey and similar to using spread props.

[mhkeller]

One idea is to prefix these attributes with something and every time this function runs, it removes prefixed attributes that are not present in attrs Scratch that. The property names will like be significant and can't be modified. The more I think about it though, I think it should replicate the unsetting behavior of using spreads props. So maybe it keeps track on a per-element basis of which props were added? We should look at how svelte itself handles this...

[maxblee]

Sorry for taking so long to get back on this. I definitely think it makes sense to be able to dynamically unset the properties. I imagine you could use an internal Set to keep track of which attributes were initially set, then compute the difference to figure out which elements, if any, were unset. But there might be a more elegant way to handle it as well.

I've checked locally to make sure everything built properly but agree that we should add a test scenario, particularly one that handles unsetting/resetting attributes.

Also, a quick note on the hasAttribute call that I made in the initial pull request: The reason I put that in there was to handle boolean attributes like required or hidden. According to the DOM spec, getAttribute should return an empty string on non-existing attributes, which would also be what you'd expect to see from an existing boolean attribute. However, it looks like modern browsers return null instead of adhering to the DOM spec, so maybe not an actual concern...

[mhkeller]

I don't totally understand. How does the hasAttribute check fix / handle / help this scenario? And is there a way to solve this scenario in a way that allows for changing an existing property?

[maxblee]

Sorry I wasn't clear. Say you want to pass this object as attrs in the setElementAttributes function:

const attrs = { hidden: "" }

If you did this, according to the DOM spec, element.getAttribute('hidden') !== attrs['hidden'] would return false since getAttribute and attrs['hidden'] would both return empty strings. However, element.hasAttribute('hidden') would return false. So by replacing the check with element.getAttribute(attr) !== property || !element.hasAttribute(attr), element.setAttribute(attr, property); would run where it otherwise wouldn't (according to the DOM spec). Again, modern browsers seem to all return null on non-existing properties, so this is probably a moot point.

If I'm not mistaken, this should already allow for changing an existing property, since element.getAttribute(attr) !== property will return false if someone changes an attribute from its initial value. The tricky thing is going to be handling attributes that are removed. I think you could handle that case by holding an internal Set that keeps track of the attributes that have been set on the element, computing the difference between that set and Object.keys(attrs), and either running element.removeAttribute(attr) or resetting the attribute to its original value (if someone changed one of layercake's computed attributes). But there might be/probably is a more elegant + performant way to handle that, and agreed that it makes sense to see what Svelte does with spread props and $$restProps.

[mhkeller]

I'm going to close this in favor of #133 since that will add the ability for aria-labels without the possible performance issues cited here. Let me know if there's a good reason for continuing with this approach, though, or if there are other commonly used props that we should pass to layout components.