redwood icon indicating copy to clipboard operation
redwood copied to clipboard

[Bug?]: <Form> onSubmit bubbles

Open smargovsky opened this issue 1 year ago • 4 comments

What's not working?

Using nested forms will bubble submit events from the inner form to the outer form. HTML spec disallows nested forms, but we can get around this in React using Portals ( like a modal ). However, React bubbles events from Portals to the parent components.

Normally we can get around this bubbling by stopping the submit event propagation. See this discussion

However, <Form> handles the submit event and by the time the consumer tries to call event.stopPropagation() the event has bubbled.

Either adding event.stopPropagation() or implementing a property for stopping propagation in <Form> will allow the workaround from the discussion linked above.

function FormInner<TFieldValues extends FieldValues>(
...
   <form
      ref={ref}
      {...rest}
      onSubmit={(event) => {
        if (stopPropagation) {
             event.stopPropagation()
        }
        formMethods.handleSubmit((data, event) =>
          onSubmit?.(data, event)
        )}
      }}
    >

How do we reproduce the bug?

Attempt to implement the solution from this discussion using Redwood <Form>.

What's your environment? (If it applies)

System:
    OS: macOS 13.5.1
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.0 - /private/var/folders/jj/rx_db5t972qbr687xkkrgljr0000gn/T/xfs-a5d7351a/node
    Yarn: 3.2.3 - /private/var/folders/jj/rx_db5t972qbr687xkkrgljr0000gn/T/xfs-a5d7351a/yarn
  Databases:
    SQLite: 3.39.5 - /usr/bin/sqlite3
  Browsers:
    Chrome: 116.0.5845.179
    Safari: 16.6
  npmPackages:
    @redwoodjs/auth-custom-setup: 5.4.2 => 5.4.2
    @redwoodjs/core: 5.1.2 => 5.1.2

Are you interested in working on this?

  • [ ] I'm interested in working on this

smargovsky avatar Sep 08 '23 16:09 smargovsky

Hey @smargovsky - thanks for writing this up.

Trying to understand the issue better - is it not possibly to have the event.stopPropogation call in your handleSubmit?

Either way it feels like you would have to either add a prop or add this extra line, when using nested forms.

dac09 avatar Sep 19 '23 02:09 dac09

I have this issue too, but for me if I define it with an interface for example like: onSubmit: SubmitHandler<FormValues> I don't get access to the stopPropagation() method.

ladderschool avatar Nov 09 '23 21:11 ladderschool

Hey @ladderschool and @smargovsky - I can see the issue on RHF - and clearly they seem to have workarounds too.

Would you be able to provide a reproduction for this? Here's a guide https://github.com/redwoodjs/redwood/blob/main/CONTRIBUTING.md#creating-a-reproduction-to-include-with-issues

It'll help cut through the noise, to see what we actually need to solve. Note that you can always directly use Forms from react-hook-form too, if you wish to.

@jtoar assigning to you :)

dac09 avatar Nov 29 '23 06:11 dac09

@smargovsky , the example you linked to from RHF doesn't have to be achieved via form-in-a-form pattern.

a <dialog> (or a custom modal component in the example's naming convention) doesn't have to be in a specific spot in the DOM since it won't be visible, and thus doesn't have to be inside a <form>.

i created an example repo (albeit a bit unrealistic example) that demonstrates how you could achieve a similar pattern by placing the <dialog> (and its child <form>) in a different place of the DOM and thus not having to worry about violating the HTML spec. and not needing any portals.

i'd point out that i made the example with redwood comps, but the problem isn't really a redwood problem per se (nor really even a RHF problem). it's more about getting the semantic DOM "right" when it comes to <dialog> and following the HTML spec.

@ladderschool, i'd be curious to hear more about your use case to know what's causing you to go with frowned upon form-in-a-form pattern. i would think that most use cases wouldn't need form-in-a-form, and thus not need a new way for the Redwood <Form> comp to change it's event handling. but more info is useful.

colbywhite avatar Dec 14 '23 20:12 colbywhite