Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore formmethod when value is "dialog" #1867

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

SamDudley
Copy link
Contributor

@SamDudley SamDudley commented Oct 6, 2023

In the following example htmx will issue a DIALOG request instead of a POST request.

<dialog>
    <form hx-post="/test">
        <button formmethod="dialog" name="foo" value="bar">Submit</button>
    </form>
</dialog>

This PR changes the behaviour so that htmx will issue a POST request.

Closes #1866

@SamDudley SamDudley changed the title Ignore formmethod when value is "dialog" Fixes #1866 Ignore formmethod when value is "dialog" Oct 6, 2023
@alexpetros alexpetros added the bug Something isn't working label Oct 6, 2023
@alexpetros
Copy link
Collaborator

alexpetros commented Oct 6, 2023

Heads up that I don't think you tagged the right issue in the description (I updated it).

Otherwise, good catch, thanks for the PR.

@alexpetros alexpetros added the ready for review Issues that are ready to be considered for merging label Oct 6, 2023
@1cg 1cg merged commit 355a4fa into bigskysoftware:dev Oct 7, 2023
1 check passed
@pokonski
Copy link
Contributor

pokonski commented Jul 22, 2024

Hey, @SamDudley . According to MDN and what I observed, such forms should never be submitted to the backend at all:

When a <form> within a <dialog> is submitted via the dialog method, the dialog box closes, the states of the form controls are saved but not submitted,

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog

Also their example code does mention this again:

image

So the method should not be changed, but submit completely skipped

@SamDudley
Copy link
Contributor Author

Hey @pokonski, I've left a comment on #2753 discussing this issue. In short, I agree that my change is inconsistent with the MDN docs and support it being changed.

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.

4 participants