search

flekschas / svelte-simple-modal

A simple, small, and content-agnostic modal for Svelte v3 and v4
https://svelte.dev/repl/b95ce66b0ef34064a34afc5c0249f313
MIT License
422 stars 30 forks source link

Closing a modal embedded in a form submits the form #83

Closed jnysteen closed 2 years ago

jnysteen commented 2 years ago

First of all, thanks for a great library!

I have identified an issue: If a modal is contained within a form, the form is submitted when the close button on the modal is clicked.

Here is a Svelte REPL that shows the issue - notice how "submitted" is logged to the console, when the close button is clicked. https://svelte.dev/repl/dd87925ed7a64f08936257414fd63841?version=3.48.0

This behaviour can cause unintended form submissions.

I'll create a PR with a fix

I will not create a PR as I currently can't properly test if this change will fix it - but if I inspect the rendered HTML, add this change, and then click the close button, the form is not submitted. The change can be found here: https://github.com/flekschas/svelte-simple-modal/compare/master...jnysteen:master

flekschas commented 2 years ago

Thanks for reporting this issue. May I ask why the <form /> element is wrapping <Modal />? When the <form /> element is inside Popup.svelte the problem does not appear. I am asking because I am have a hard time understanding the current use case here.

flekschas commented 2 years ago

Apart from that, your fix seems to make sense to me, so I am happy to add it.

Demo with fix: https://svelte.dev/repl/7e3efb8138314975ad246a3a575a450b?version=3.48.0

Do you want to submit a PR with this fix? Otherwise I can also add it real quick.

flekschas commented 2 years ago

@jnysteen I took the liberty to turn your branch into a PR: https://github.com/flekschas/svelte-simple-modal/pull/84

flekschas commented 2 years ago

Fixed via #84

flekschas commented 2 years ago

Just released v1.4.1 which contains the fix. Thanks again for reporting and fixing the issue!