I have an SVG image that is embedded in an HTML page inside an Alpine component. As part of the SVG there's a text element that I would like to bind to some Alpine data. Unfortunately, x-text does not seem to work for SVG elements.
I believe this is because x-texttries to set .innerText on the element, but SVG elements, or at least <text> don't have an .innerText attribute.
Something like this could work:
- el.innerText = output
+ if (el.hasAttribute('innerText') {
+ el.innerText = output;
+ } else if (el.hasAttribute('textContent') {
+ el.textContent = output;
+ }
It's not the most elegant solution, but unless somebody has a better approach, I can put together a PR for this.
That seems like a reasonable PR.
Make sure to include a test.
I don't know how exactly JSDOM works but I think it doesn't implement innerText correctly so you might have issues there
@philippbosch If you don't fancy doing a PR for this, I'll take a look on Sunday.
Oh, I‘ll happily put together a PR. But thanks!
Would we have issues if we always set textContent?
Would we have issues if we always set textContent?
I don't think so tbh, it should be fine. There are some differences in the behaviour of the two properties but I can't see any major problems.
That was my first thought as well. What made me shy away from that approach was that the other x-text test were failing. But that might be a limitiation in jsdom which maybe doesn't update innerText when textContent is changed.
But yeah, I can definitely look into using textContent always. I found this article which covers the differences between the two. I guess I'll start with writing some more tests first.
No worries, it was just to satisfy my curiosity. :)
No, you're totally right, @SimoTod!
Turns out that jsdom does not implement innerText at all, so the existing tests are not very helpful. We are simply testing if we can set a variable on an object …
I'm gonna change the implementation of x-text and also the tests to use textContent instead. I'm fairly sure it will be fine, like @ryangjchandler said, because the differences between innerText and textContent are mostly about _reading_ and not about _writing_ the value.
So, this PR is getting a lot more complex when we swap innerText for textContent. But I still think it‘s the right thing to do.
jsdom has a pretty robust implementation of textContent but none at all of innerText. That means all tests that currently test something on innerText are not very realistic. Also, textContent – in jsdom and browsers – is always a string. So tests like this one need to be changed to expect "0" instead of 0: https://github.com/alpinejs/alpine/blob/2db9cfbc589a1fc4537bf85dca5beb7730d189c9/test/if.spec.js#L92
I‘m going through all the tests and will update the PR tomorrow or on Monday.
Scary changes ☹️