Fix reset modifier timing and form lookup - #3981
Closed
1cg wants to merge 1 commit into
Closed
Conversation
Contributor
Author
|
Not super happy with this, seems hard to get trigger info to the right spot. Maybe drop in favor of an hx-reset extension? |
Collaborator
|
yeah we could simplify it a little maybe with and if we did just this change then an extension can pickup custom trigger specs like reset and use them properly instead of guessing if the triggering event was the one that triggered the request we need to reset. |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3977.
Problem
The
resettrigger modifier ran in the trigger listener, before htmx collected the request body. Three defects:__handleTriggerEventcollected the body, so a<form hx-post hx-trigger="submit reset">sentmsg=instead of the value the user typed.hx-confirmstill reset the form. The reset ran before the confirm gate.elt.closest('form')ignoredform="id". An element that points at a form from outside it did not reset that form.Changes
src/htmx.js:innerno longer resets. It passes the firingspecto the handler.__createHtmxEventHandlerrecordsctx.resetfrom that spec.__issueRequest, after the confirm gate. It runs only when the request is committed, and after the body is collected.elt.form || elt.closest('form').Tests: the two tests from #3977 asserted a synchronous reset, which no longer holds. Rewrote them and added four for the sent body, per-spec attribution, cancelled confirm, and
form="id".Docs corrected on both reference pages. The old text promised a reset "before the request is sent", which was the bug.
1735 passed, 0 failed, 100 percent coverage.
Open question
Not super happy with this, seems hard to get trigger info to the right spot. Maybe drop in favor of an
hx-resetextension?The spec is known in the trigger listener, but the reset must happen much later. The options I looked at:
onTrigger(elt, spec, handler)is internal API used byhx-ws,hx-sse, andhx-multipart, and all of them ignore the second arg.detailis a read-only number onUIEvent, so there is nowhere to put it on aclick. Also racy, becausefrom:fans one event to several specs and the read happens after anawait.Marked as a draft while we decide.