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

Enable opting-out of form submissions #3

Merged
merged 2 commits into from
Dec 31, 2020
Merged

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Dec 14, 2020

Similar to Disabling Turbo Drive on Specific Links,
guard intercepting form subissions with a similar check for
[data-turbo-drive="false"] on the submitter, <form> element, or an
ancestor.

Similarly, do not intercept when the submitter or form element
specifies a method that isn't an HTTP verb (via
formmethod="dialog" attribute or method="dialog"
attribute, respectively).

In practice, HTTP submitting <form> elements will only ever declare
method="get" or method="post". The one exceptional case is
method="dialog"
, which we check for as a special case.

Testing

Since we're covering behavior for the <dialog> element and running
the suite in both Chrome and Firefox, the test suite needs to polyfill
support for the <dialog> element.

src/core/session.ts Outdated Show resolved Hide resolved
src/core/session.ts Outdated Show resolved Hide resolved
src/core/session.ts Outdated Show resolved Hide resolved
@domchristie
Copy link
Contributor

Would this be a good place to incorporate the changes from https://github.com/turbolinks/turbolinks/pull/495/files?

I've not manually tested Turbo Drive with non-fetch form submissions, but from the look of Session#start, the initial replace will still break the "Are you sure…?" refresh behaviour after rendering a form submission:

this.replace(Location.currentLocation)

After a traditional POST, calling history.replaceState will replace the POST entry with a GET, and therefore refreshing won't trigger the confirm dialog. The fix was to store the initial restoration identifier as a property on the History instance rather than replacing the browser's history state.

I'd be happy to port these changes over if there's interest?

@ChrisZou
Copy link

My I ask when would this be merged? 😂😂 Quite a few issues are depending on this to be fixed. It's preventing me to use hotwire to do so many things.

@seanpdoyle seanpdoyle force-pushed the form-submitter-skip branch 4 times, most recently from b33bc20 to f381ac5 Compare December 30, 2020 15:26
src/core/session.ts Outdated Show resolved Hide resolved
src/core/session.ts Outdated Show resolved Hide resolved
Similar to [Disabling Turbo Drive on Specific Links][turbo-drive-false],
guard intercepting form subissions with a similar check for
`[data-turbo-drive="false"]` on the submitter, `<form>` element, or an
ancestor.

Similarly, *do not intercept* when the submitter or form element
specifies a method that isn't an HTTP verb (via
[formmethod="dialog"][formmethod] attribute or [method="dialog"][method]
attribute, respectively).

In practice, HTTP submitting `<form>` elements will only ever declare
`method="get"` or `method="post"`. The one [exceptional case is
`method="dialog"`][method-dialog], which we check for as a special case.

Testing
---

Since we're covering behavior for the `<dialog>` element _and_ running
the suite in both Chrome and Firefox, the test suite needs to polyfill
support for the `<dialog>` element.

[turbo-drive-false]: https://turbo.hotwire.dev/handbook/drive#disabling-turbo-drive-on-specific-links
[formmethod]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attr-formmethod
[method]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form#attr-method
[method-dialog]: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-submission-2
It’s enough to test that Turbo doesn’t handle form submissions when method=dialog or formmethod=dialog.
@jon-sully
Copy link

jon-sully commented Mar 18, 2021

Just adding in case other folks come around, I believe contrary to the initial description of this PR, the necessary attribute on the <form> needs to be data-turbo="false", not data-turbo-drive="false"

But this is well-documented on https://turbo.hotwire.dev/handbook/drive#disabling-turbo-drive-on-specific-links-or-forms

In Rails parlance,

form_with model: @xyz, url: etc, data: { turbo: false }

Which is great because data: { 'turbo-drive' => false } doesn't look nearly as slick 😜

seanpdoyle added a commit to seanpdoyle/turbo that referenced this pull request Sep 10, 2021
Follow-up to hotwired#3

---

When a `<form data-turbo-frame="...">` element is submitted, it's
handled by the `Frames/FormInterceptor` class instead of the
`FormSubmitObserver` class, which was originally fixed to cover this
behavior in [hotwired#3][].

This commit reproduces the fix so that it's consistent across `<form>`
elements, regardless of their targetting.

[hotwired#3]: hotwired#3
dhh pushed a commit that referenced this pull request Nov 12, 2021
Follow-up to #3

---

When a `<form data-turbo-frame="...">` element is submitted, it's
handled by the `Frames/FormInterceptor` class instead of the
`FormSubmitObserver` class, which was originally fixed to cover this
behavior in [#3][].

This commit reproduces the fix so that it's consistent across `<form>`
elements, regardless of their targetting.

[#3]: #3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants