bigskysoftware / htmx

</> htmx - high power tools for HTML
https://htmx.org
Other
38.19k stars 1.3k forks source link

Empty class elements left after swap #1701

Open dmcg opened 1 year ago

dmcg commented 1 year ago

Inspecting the page after a swap, htmx leaves an empty class=“” attribute on the target element. It’s benign, but means that the resulting page is not the same as the a page for the same server state rendered from scratch, which makes testing harder.

You can see the problem in https://youtu.be/qYuSr__OiHk at 23:15

Telroshan commented 1 year ago

Hey, that's because htmx adds temporary classes such as htmx-request to your elements while the requests are in flight, to let you use some styling when needed, such as a hx-indicator to display say a loading spinner until the request is done.

Since those classes are added, it's then actually simply because of how the JS API works that the empty class attribute remains, it's not related to htmx itself.

You can observe that behaviour in this JSFiddle

<div id="test">Inspect me in F12</div>

<button type="button" onclick="test.classList.add('test')">Add class</button>

<button type="button" onclick="test.classList.remove('test')">Remove class (classList)</button>
<button type="button" onclick="test.className=''">Remove class (className)</button>

Using both classList and className, you'll see that after removing the class, the class attribute doesn't go away, and remains here, empty

dmcg commented 1 year ago

I can see how it happens, and if it can’t be fixed then so be it, but it seems a shame that testing is made more complicated than it could be?

duanemoody commented 4 months ago

Hey, that's because htmx adds temporary classes such as htmx-request to your elements while the requests are in flight, to let you use some styling when needed, such as a hx-indicator to display say a loading spinner until the request is done.

Looking at the source code, it's actually because doSettle is for some reason still manually adding/removing those utility classes to the elements through bare-metal elt.classList.add/elt.classList.remove statements, instead of leveraging its own addClassToElement/removeClassFromElement methods, which do bother doing checks on elt.classList.length === 0 then removing the attribute as needed.

This pull request replaces all direct node.classList methods in the codebase with their corresponding wrapper functions ensuring whatever housecleaning is necessary gets done.