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

Properly handle buttons with formmethod=dialog #2753

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

geoffrey-eisenbarth
Copy link

Description

Buttons with formmethod specified should only be handled by HTMX if the value is an HTTP verb. This partially addresses Issue #2554 (along with PR #2752) and (imho) fixes the behavior introduced in PR #1867. If a button with formmethod="dialog" is a clicked, HTMX should ignore the request and let the browser handle closing the dialog (see this comment).

Testing

I removed two ajax tests added by PR #2752 (that were also duplicated in the shadowdom tests) and replaced them with a test that should be checking that the server had no response. The tests passed but I'm not 100% the test were correctly written.

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

@Telroshan
Copy link
Collaborator

Telroshan commented Jul 22, 2024

Hey, I'm gonna shamelessly copy-paste the message I left on the other PR, that actually doesn't apply to it but applies here!

Hey, while I perfectly understand the concern of following the specs, the issue is always about retro-compatibility ; as you've noticed yourself on PR #1867 and related issue #1866, since it's been merged, at least the OP and likely other persons have been running this setup, which would suddenly not work anymore as they would expect it to make the request with the fallback method specified.

That's exactly the kind of stuff that led us to make htmx 2 ; making breaking changes, changing some defaults to either fit the specs, or set new defaults that felt more logical (security ones for ex such as selfRequestsOnly).

And now that htmx 2 is officially released, it would take an htmx 3, which I'll be honest about, is very, very unlikely to happen anytime soon (if at all).
So, for this kind of change, I'm afraid you'd have to go for an opt-in approach, i.e. leave the default behavior as is and provide a configuration option to make dialog skip submit altogether.

I know it can be frustrating, I've been there, I hope you'll understand!

@Telroshan Telroshan added under discussion Issues that are being considered but require further alignment bug Something isn't working labels Jul 22, 2024
@geoffrey-eisenbarth
Copy link
Author

Perhaps for the purposes of discussion, @SamDudley could chime in here and help us understand the intent of his PR? Given their example below,

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

Is this supposed to 1) submit the form (via htmx) and then 2) close the dialog (via user agent)? Or something else?

@geoffrey-eisenbarth
Copy link
Author

I completely understand not wanting to break backwards compatibility, and would be willing to add a config value if that's deemed an acceptable path forward.

Thanks!

@SamDudley
Copy link
Contributor

Hey, it's been awhile since I worked on the project that prompted my PR, but I'll try to recall my intentions.

In the project I was working on, I wanted the form to be submitted (via htmx) and then for the dialog to be closed. I seem to recall that closing the dialog required some additional code, so it's possible the example would only submit the form (via htmx) and not close the dialog (testing required).

Having read this comment, and looking through the MDN docs myself, I agree that the behaviour I introduced is inconsistent with the MDN docs (and I assume the HTML spec).

It's interesting to think about what approach people would expect htmx to take. Should the hx-post (like in my example) take precedence and a form submission be made? or should the formmethod behaviour be retained and the request ignored?

Either way, personally I have no issues with this PR changing the behaviour to be consistent with the HTML spec, however in the interest of maintaining backwards compatibility, making the behaviour opt-in seems like the most sensible approach.

@geoffrey-eisenbarth
Copy link
Author

Just spitballing here, but it seems to me like the "correct" way to submit a form (via hx-post) and then close the dialog would be to do something like

<dialog>
    <form hx-post="/test" hx-on:htmx:after-swap="event.target.closest('dialog').close()">
        <button name="foo" value="bar">Submit</button>
    </form>
</dialog>

This way people who use HTMX as progressive enhancement only can still have nice no-js behavior:

<dialog hx-boost="true">
  <form method="post">
    [...]
    <button type="submit" name="foo" value="bar">Submit</button>
    <button formmethod="dialog">Close</button>
  </form>
</dialog>

@scitech
Copy link

scitech commented Oct 23, 2024

We ran into this today and assumed the opposite behavior would happen:

<dialog>
<form hx-post="/something/somewhere>
  <!-- we thought this would close without issuing a request: -->
  <button formmethod="dialog">Cancel</button>
  <!-- we only expected this to issue the request: -->
  <button>Submit</button>
</form>
</dialog>

htmx's current behavior makes sense to me on reflection, because formmethod is overriding the native method and not the hx-post attribute, after all. However, boosted forms complicate it somewhat.

We were able to resolve this for our use case by adding an ID and an explicit trigger pointing to the submit button, like:

<dialog>
<form hx-post="/something/somewhere hx-trigger="from: #submit-button">
  <button formmethod="dialog">Cancel</button>
  <button id="submit-button">Submit</button>
</form>
</dialog>

What I was thinking would be nice is a way to tell htmx that a button should be ignored:

<dialog>
<form hx-post="/something/somewhere>
  <button formmethod="dialog" hx-ignore-events-originating-here>Cancel</button>
  <button>Submit</button>
</form>
</dialog>

That way, we wouldn't need to complicate the ID namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working under discussion Issues that are being considered but require further alignment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants