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

Support formaction and formmethod on <button> #1777

Merged
merged 1 commit into from Sep 14, 2023

Conversation

jacob-ebey
Copy link
Contributor

@jacob-ebey jacob-ebey commented Sep 9, 2023

Bug: The these two forms should POST to the same URL but do not.

<form>
  <button formaction="/action" formmethod="POST">Submit</button>
</form>
<form hx-boost="true">
  <button formaction="/action" formmethod="POST">Submit Boosted</button>
</form>

Fix: take into account formaction and formmethod of the submitter.

I'm not familiar with the HTMX codebase but issueAjaxRequest seemed like a reasonable place to handle this. event.submitter is relatively new as well, so you might want a more backwards compatible implementation, but this shows the gist of the issue and the fix.

Instead of using event.submitter, I have opted to use the tracked button click on the internal data. This should work on all browser targets supported by HTMX.

@jacob-ebey
Copy link
Contributor Author

Addresses #623

@alexpetros
Copy link
Collaborator

Closing this on account of not following the contribution guidelines, re: IE 11 support.

I do appreciate you calling that out though, as I wouldn't have thought to look for it otherwise.

@alexpetros alexpetros closed this Sep 9, 2023
@jacob-ebey
Copy link
Contributor Author

Closing this on account of not following the contribution guidelines, re: IE 11 support.

I do appreciate you calling that out though, as I wouldn't have thought to look for it otherwise.

You plan on addressing this? If not, please re open this and I'll update it for ie11

@alexpetros
Copy link
Collaborator

Sure! If you want to update for IE11 compatibility I'm happy to re-open it.

@alexpetros alexpetros reopened this Sep 10, 2023
@jacob-ebey
Copy link
Contributor Author

I'm super jetlaged at the moment but will try to get to it before the weekend is up.

@jacob-ebey
Copy link
Contributor Author

@alexpetros Updated to use existing internals to support IE11.

@alexpetros
Copy link
Collaborator

Yup, this looks great, thank you! Seems like a very straightforward thing to support.

@alexpetros alexpetros added enhancement New feature or request ready for review Issues that are ready to be considered for merging labels Sep 10, 2023
@xhaggi
Copy link
Contributor

xhaggi commented Sep 12, 2023

On May 24, I created the following PR #1451 that already contained this fix in ce9ccff.

Too bad things have to be addressed twice as PRs sit around for months.

@alexpetros
Copy link
Collaborator

alexpetros commented Sep 13, 2023

This PR is a little easier to merge than the linked change, unfortunately. The linked change would also support attributes changing after the process step, which is definitely good, but also a larger surface area for regressions, which is why we haven't gotten to it yet.

@1cg 1cg merged commit 89f13e6 into bigskysoftware:dev Sep 14, 2023
1 check passed
@adoublef
Copy link

adoublef commented Sep 20, 2023

following as I just discovered I was having issues figuring how to use the formaction with hx-boost for a simple counter example (using v1.9.5)

@alexpetros
Copy link
Collaborator

alexpetros commented Sep 20, 2023

You should be good to go as soon as the 1.9.6 release is cut (very soon)!

@jacob-ebey
Copy link
Contributor Author

jacob-ebey commented Sep 21, 2023

Cheers 🥂

Person cheering

@jacob-ebey jacob-ebey deleted the formmethod_formaction branch September 24, 2023 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 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.

None yet

5 participants