hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.65k stars 421 forks source link

form with multiple submit buttons, clicked button is lost #272

Closed jorismak closed 2 years ago

jorismak commented 3 years ago

When you add a 'name' attribute on a

sukei commented 3 years ago

Had the issue, Its not because of the multiple submit buttons, but because of the button itself. Changing this to an <input type="submit"> works better. BTW, the button works in the browser scenario, I think it is an issue and deserve to be fixed.

jorismak commented 3 years ago

Symfony renders

sukei commented 3 years ago

I'm using Symfony too, Thats why i came with a TurboSubmitType with a custom rendering template to fix it. It's not ideal because you can't use HTML elements within. I use that kinds of hack to improve things:

<label class="flex items-center">
    <input class="bg-transparent" type="submit" name="form[next]" value="Next">

    <svg xmlns="http://www.w3.org/2000/svg" class="ml-2 h-5 w-5" fill="none" viewBox="0 0 24 24" stroke="currentColor">
      <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 5l7 7-7 7" />
    </svg>
</label>
ghost commented 3 years ago

Turbo does handle named submitters, but not if they have a null value (see here). That seems to be the problem. Does Symfony gives you a button with no value?

I’m not sure the reason why null values are skipped currently, but it seems surprising to me. A submit <button> without a value seems pretty solid to me.

ghost commented 3 years ago

Ah, after a bit more digging, it seems the null check is because we don’t want the submitter in the query string if you submit a GET form (see here).

sukei commented 3 years ago

I doesn't work better using POST. BTW I wasn't aware that button can hold a value, Will try this next time.

jorismak commented 3 years ago

But even if it works with a value, that means we're placing a value on something just for Turbo to pick it up. Sounds like a workaround.

A <button type="submit" name="button_reference">Send!</button> seems perfectly normal to me.

ghost commented 3 years ago

Oh, turns out this is a bug fix: https://github.com/hotwired/turbo/pull/184

sukei commented 3 years ago

that means we're placing a value on something just for Turbo to pick it up

Totally agree

danielpuglisi commented 3 years ago

Experiencing the same issue in Chromium browsers. Safari works.

seanpdoyle commented 2 years ago

Is this resolved by either https://github.com/hotwired/turbo/pull/184 or https://github.com/hotwired/turbo/pull/405?

jorismak commented 2 years ago

Will have to retest with the latest release to be sure, but just from quickly browsing the issue threads, they don't appear related. But maybe they fixed it by accident?

jorismak commented 2 years ago

Made a quick test with 7.1.0, and no, it doesn't appear to be fixed.

source form:

<form name="turbo_form" method="post">
   <div><label for="turbo_form_textInput">A random text field</label><input type="text" id="turbo_form_textInput" name="turbo_form[textInput]" /></div>
   <div><button type="submit" id="turbo_form_specialButton" name="turbo_form[specialButton]">Special button</button></div>
   <div><button type="submit" id="turbo_form_submit" name="turbo_form[submit]">Submit button</button></div>
</form>

When disabling turbo on the form by adding a data-turbo="false" attribute, clicking one of the buttons submits something like this:

turbo_form%5BtextInput%5D=test&turbo_form%5BspecialButton%5D=

so, decoded:

turbo_form[textInput]: test
turbo_form[specialButton]: 

or

turbo_form%5BtextInput%5D=test&turbo_form%5Bsubmit%5D=

(the other 'submit' button)

At least in Chrome / Edge here, when not clicking a button, but entering text in the text-input and pressing 'enter' actually sends the same payload as if you clicked on the 'specialButton'. Probably because it's first in the DOM and of type=submit. But even then, it gets send.

When enabling turbo, the payload is like this:

turbo_form%5BtextInput%5D=test

or decoded:

turbo_form[textInput]: test

...so either of the buttons is completely missing.

It appears (if I remember the discussion correctly) that because there was no 'value' for the field, turbo decides to not send the field at all. While all (most?) browsers have different behavior.

And this breaks certain functionality in Symfony (PHP) framework (or forms using this through some other ways) where you can add multiple buttons and check on form-submit which one is the one that the user clicked.

Specially for wizard-style forms where you have a 'prev' and 'next' button, this is used a lot I guess.

seanpdoyle commented 2 years ago

Ah, I think I understand. Thank you for sharing the extra context.

Is the underlying issue that the SubmitEvent.submitter has a [name] attribute, but does not declare a [value] attribute, so this block is omitting it from the submission:

https://github.com/hotwired/turbo/blob/1563ebbfa3f4c8530b334e194da9db630163041b/src/core/drive/form_submission.ts#L226-L236

jorismak commented 2 years ago

Seems (very) likely.

(I just tested with 7.2.0-beta.1 to be sure, but still the same).

jorismak commented 2 years ago

Yes, adding a value="foobar" to the specialButton makes it work (and still not working for the other submit button).

So that might be a workaround, but is still different to browser behavior.

I don't know Turbo internals, but you could leave that loop in there to leave out empty form fields, but add the 'submitter' after that.

For what it's worth, if I leave the text-input field empty and hit submit, it still gets submitted with an empty value (because it's not null, but it's an empty string I'm guessing).

This happens both with turbo and without turbo. So that leaves me wondering, why the value != null check anyway at all?

seanpdoyle commented 2 years ago

I've opened https://github.com/hotwired/turbo/pull/653 to attempt to resolve this issue.