Skip to content

Conversation

MichaelWest22
Copy link
Collaborator

@MichaelWest22 MichaelWest22 commented Jul 30, 2025

Description

Problem:
Since v2.0.5 (#3336), links inside htmx-enabled elements were incorrectly having their navigation blocked. This broke expected behavior where links should work normally even when placed inside clickable htmx containers.

Before (broken):

<div hx-get="/data">
  <a href="/page">Link</a> <!-- Navigation was blocked -->
</div>

Clicking the link would prevent navigation and only trigger the htmx request.

After (fixed):
Links inside htmx containers now navigate normally, preserving the pre-2.0.5 behavior users expect.

Technical change:
Re wrote shouldCancel to make it easier to understand and logical and split submit from click event handling based on suggestion from @1cg. Found that the core of the issue is the from: trigger handling added in #3336 and there is a more reliable simpler way to handle this by just passing eltToListenOn instead of elt to shouldCancel we no longer need to do evt.target lookup inside the function and can instead retain sensible logic in shouldCancel but for users using from: it now handles these situations without changing behaviour for normal triggers. Most of the time eltToListenOn is just elt anyway and when its not we need to use this custom elt as the element to locate the wrapping link or button that needs to be canceled.

Note that shouldCancel before was handling click events on forms and preventing them before and this change now only cancels submit events on forms which makes more sense as forms do not trigger on click events on links and form submit buttons do. This means some of the submitCancel tests needed to be revised and a regression test needed updating.

Backward compatibility:
Restores the previous behavior where links work as expected

Notes:
In the ideal situation links like this could be handled by htmx allowing better progressive enhancement options and this change would not be required but this could be a change in expected behavior. We could also consider rolling back #3336 as an alternative solution.

Corresponding issue:
#3395

Testing

Added shouldCancel test and expanded from: regression tests.

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@MichaelWest22 MichaelWest22 added bug Something isn't working ready for review Issues that are ready to be considered for merging labels Jul 30, 2025
@1cg 1cg merged commit cee310e into bigskysoftware:dev Sep 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready for review Issues that are ready to be considered for merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants