htmx
htmx copied to clipboard
loading states: Respect path filter in processing undo callbacks
Description
The loading-states extension queues up callbacks on htmx:beforeRequest
. On htmx:beforeOnLoad
it executes any undo callbacks that mayProcessUndoCallback
allows. mayProcessUndoCallback
only checks for the existence of the target.
Suppose we have multiple requests in flight. We use data-loading-path
to only have matching requests modify the loading states of target elements. While attaching loading states can be filtered, removing loading states always succeeds as the undo queue is drained on htmx:beforeOnLoad
.
This means that target elements are marked as loaded even though their requests are still in flight.
This change passes the request path to mayProcessUndoCallback
to allow for delayed execution of undo callbacks when a path filter is available.
Corresponding issue: https://github.com/bigskysoftware/htmx/issues/2296
Testing
I tested this manually with a test page that would trigger multiple requests, served by a backend that introduces random delays to the responses. I used the loading states extension and only set up data-loading-aria-busy
on target elements. With this change those elements whose requests were still in flight would remain in aria-busy
state, while those whose requests were completed would have the attribute removed.
Without the change any completed request would cause the removal of aria-busy
from all elements, even if their requests were still in flight.
Checklist
- [X] I have read the contribution guidelines
- [X] I have targeted this PR against the correct branch (
master
for website changes,dev
for source changes) - [X] This is either a bugfix, a documentation update, or a new feature that has been explicitly approved via an issue
- [X] I ran the test suite locally (
npm run test
) and verified that it succeeded
702 passing (4s)
1 pending
I had this bug and this PR fixed it. In my opinion, it should be merged.
Thanks @rekado!
Can I do anything to increase the likelihood of getting this merged?
Hey @rekado, PRs are a bit stalled at the moment as we're focused on htmx 2, we'll get to PRs (and issues) again once htmx 2 is out.
Please note that with htmx 2, extensions will move to another repo, thus I'd encourage you to port your PR to this repo when you have the chance!
If you'd like to get this change into htmx 1, please retarget to the v1 branch as the dev
branch now reflects htmx 2 which doesn't include extensions in its src
anymore
Hey, are you still interested in that change @rekado ? If so, please retarget to v1
as explained above, and/or make another PR to the extensions repo, thanks!