htmx
htmx copied to clipboard
preloaded places `htmx-request` class on `body`
Thanks for this fantastic piece of software!
I found a minor issue. When triggering preloading (by hovering) of some element, some other elements on my page suddenly got a new style. I found this was because preload places the htmx-request
class on body
. Probably this is the culprit, as it doesn't set anything. (Or the code that interprets undefined as body of course)
https://github.com/bigskysoftware/htmx/blob/d766c14aa6aead788519fdf037f8c6387106c460/src/ext/preload.js#L51-L53
IMO it shouldn't place the class at all, but if it must, then it should be on the element that triggered it
@benpate or is this intended behavior?
Hola @Hades32 -
This isn't exactly intended behavior, so much as the default way that htmx handles requests. In the code you're highlighting, we just call the standard htmx.ajax() function and allow it to do its natural thing. Therefore you should be able to control where this goes using the hx-indicator attribute -- but again, that's the default behavior and I haven't tested this specifically.
Does this help? If not, we can try to make some changes to the extension so that it "does the right thing" -- whatever that it :)
Thanks for your response @benpate. I'm not sure if changing the indicator target would work, but IMHO this would still be wrong. E.g. I play an animation on a button when the request is fired and I want it to preload on hover. BUT of course I don't want the button animation to play when I hover. And with the current behavior (which seems to default target the body element) all my buttons animate when I hover one of them.
So, not sure how to do this implementation-wise, but in my opinion preloading should not set the htmx-request tag at all, as it's semantically incorrect. At least not by default, or at the very least not when using the mouseover trigger
Yes, you're right. It's not "correct" as in the right thing to do. It is something that we've inherited from the default behavior, and will be pretty tough to fix right now without duplicating the htmx.ajax()
function.
I am hoping for some underlying structural changes in htmx 2.0 that could make this more realistic to fix. @1cg - this is my tickler/request/reminder for splitting up the ajax and swap features of htmx. How realistic is this possibility in 2.0?
@benpate I also would be interested in splitting the ajax and swap. I extended the preload extension to not only prime the cache but to save and replay the request (https://www.reddit.com/r/htmx/comments/147xjo4/preload_extension_only_primes_the_browsers_cache/)
In the current form, I still had to make a request to ensure htmx did the normal things, but it would be super to avoid that.
Hey @checketts - I answered you on Reddit. You're 100% correct. Thanks for keeping this issue visible. I'm looking forward to updates in htmx that will make preload (and a bunch of other stuff) a million times better.
I believe I found the root cause of the class being placed on the body (see PR #1493)
Since no source
element is provided in the htmx.ajax
call it default to the document body.
Great. But I'd still argue that preloading shouldn't place the request class anywhere, as it is just an implementation detail that shouldn't leak to user-visible state
I agree. Here I was able to find a solution:
During the preload we can provide our own hx-indicator
. This works but has the problem of logging an error that '.preload-indicator returns no matches' if it isn't present in your application.
var previousIndicator = node.getAttribute("hx-indicator");
node.setAttribute("hx-indicator", ".preload-indicator")
htmx.ajax("GET", hxGet, {
source: node,
handler:function(elt, info) {
done(info.xhr.responseText);
}});
if(previousIndicator) {
node.setAttribute("hx-indicator", previousIndicator)
} else {
node.removeAttribute("hx-indicator")
}
@benpate @1cg I would like to propose that hx-indicator
be able to support false
or none
as options to disable the behavior. Would a PR for that in htmx core be acceptable?
Also would adding support for a preload-indicator
be a good idea for the extension? I would be happy to send both changes.
hx-indicator should support none
since that's a valid css selector that will return nothing (unless someone has a custom element called none
, in which case they get what they deserve)
@1cg None 'works' but causes a console error (see https://github.com/bigskysoftware/htmx/blob/b4a61c543b283eb2315a47708006783efb78f563/src/htmx.js#L678)
The selector "false" on hx-indicator returned no matches!
Looking at the code I see that hx-indicator=""
works, and avoids the error. So no core change is needed.
@Hades32 I submitted a pr that addresses this behavior. See #1511
Awesome! Thank you for keeping working on this!