htmx icon indicating copy to clipboard operation
htmx copied to clipboard

loading states: Respect path filter in processing undo callbacks

Open rekado opened this issue 1 year ago • 4 comments

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

rekado avatar Feb 09 '24 15:02 rekado

I had this bug and this PR fixed it. In my opinion, it should be merged.

Thanks @rekado!

davidjr82 avatar Apr 10 '24 04:04 davidjr82

Can I do anything to increase the likelihood of getting this merged?

rekado avatar May 13 '24 10:05 rekado

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

Telroshan avatar May 14 '24 07:05 Telroshan

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!

Telroshan avatar Jul 22 '24 17:07 Telroshan