Add classList support to SVGElement.
#391
Conversation
bicknellr
changed the title
bicknellr
changed the title
classList support to SVGElement.
| suite('SVGElement classList', function() { | ||
| const createSVGElement = () => document.createElementNS('http://www.w3.org/2000/svg', 'g'); | ||
|
|
||
| test('length', () => { |
There was a problem hiding this comment.
The big remaining test we want here is to ensure that setting and removing the class attribute is correctly reflected in classList.
There was a problem hiding this comment.
SVGElement overwrites the usual Element#className with an SVGAnimatedString and Chrome doesn't seem to return the correct result for .classList.contains(x) after setting .className.baseVal = x. (I filed a bug for this here.) Getting .classList.value after setting .className.baseVal seems to work, but I think avoiding .contains() kind of defeats the purpose.
Given that this is a browser-compat bug for something we don't want to repair / polyfill ourselves (classList itself or the SVGAnimatedString#baseVal setter), I'm not really sure where to go from here other than to just not use both className and classList on SVG elements at the same time and test using only classList.
There was a problem hiding this comment.
(following up from external chat) For some reason I read your comment as being about setting className but it was actually about setting the class attribute via setAttribute. I'll write a test for that specific case.
bicknellr
force-pushed
the
svg-element-class-list
branch
from
919916c to
737cd2d
Compare
This adds support for
classListtoSVGElement. Thanks to @justinfagnani for pointing out this trick, which makes this super simple. Originally, I included polyfills for newer pieces ofDOMTokenList, but it sounds like we might not need those for the next version of Lit, so I've split them out into a separate PR (#392).