Add option to return an HTMLElement from the icon helper

[LuomaJuha]

Sometimes it is easier to manipulate the icon when it is an HTMLElement like when assigining it into a value, so here is a small adjustment for the icon function.

[demiankatz]

Thanks, @LuomaJuha! I'd like to get @crhallberg's opinion on this, since I believe he wrote most of the original code that's impacted. He's going to be on vacation for the next week or so, but hopefully he can look at it when he returns if he can't get to it before he leaves tomorrow. :-)

[crhallberg]

My comments are nitpicks. This looks very good and I like the direction!

[crhallberg]

You could use a Set to remove duplicate values as you compile them.

const newAttrsSet = new Set([...newAttrs, ...oldAttrs.split(" ")]);
//...
_element.setAttribute(key, Array.from(newAttrsSet).join(" "));

[crhallberg]

Are there other attributes that require space separated strings other than class? If we're going to be working on an Element directly, I'd like Element to do as much work as possible. This actually makes me think we should be overriding the attributes instead of appending them (except for class). I know this is based on my code but I've since learned how complicated this can get.

[LuomaJuha]

Hmm, I would think that class could be the only one, only if there is some custom datasets but otherwise shouldn't be, ill try and adjust

[LuomaJuha]

@crhallberg made some small changes where if the key is not class then just sets it instantly.

[demiankatz]

Thanks, @LuomaJuha! @crhallberg, do you mind taking another look at this when you have a chance?

[demiankatz]

I suspect we'll be able to merge this soon, but since things are getting locked down for 9.1, it will probably have to wait for release 10.0. I'm setting a milestone to be sure we don't lose track of it.

[crhallberg]

Thanks for making those changes! I was thinking about using classList for this, but I think that would lead to more confusing code. Thank you!

[demiankatz]

Thanks, @LuomaJuha and @crhallberg!