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

Prevent submitting outer form when enter is pressed in Url/Unsplash plugin #4278

Closed
wants to merge 1 commit into from

Conversation

arturi
Copy link
Contributor

@arturi arturi commented Jan 12, 2023

Related: transloadit/uppy.io#51

Problem: when Uppy Dashboard is wrapped in an outer <form>, pressing enter on inputs inside Uppy submits that outer form, instead of confirming the input inside Uppy. This PR seems to resolve it in my tests.

Is there some use case where actual form and submit event would be better, does handling enter key not cut it?

Copy link
Contributor

@mifi mifi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM!

Is there some use case where actual form and submit event would be better, does handling enter key not cut it?

At least on iOS, we need to wrap the input in a form in order to get the blue submit button on the bottom right of the on screen keyboard. Without a form it will just show a grey "Return" key. So that's one argument for wrapping in a form.

https://stackoverflow.com/questions/4864167/show-search-button-in-iphone-ipad-safari-keyboard/11254336#11254336

Another argument for using a form might be to improve accessibility - because a form has semantics, and everyone knows forms can be submitted with a standard submit action, while an input with an onKeyDown: if keyCode === 13, is not very semantic.

@Murderlon
Copy link
Member

I'm also interested in trying out putting a form in Uppy. It might be error-prone to always think about adding preventDefault in places that need it.

@arturi
Copy link
Contributor Author

arturi commented Jan 13, 2023

I would be all for using <form> for semantics, the arguments against it are: you can’t wrap a form in a form in HTML, so if we use one in Uppy UI, it’s a requirement for devs to not place Uppy Dashboard in their own <form>.

Why you would want that?

  1. Progressive enhancement. You have form with file input, then Uppy comes and replaces it with fancy UI. JS not loaded? Form still there. Although here we do just that https://uppy.io/examples/xhrupload/ and destroy the form before loading Uppy, so that should work.
  2. Actual form with fields — name, surname, date of birth, etc, then comes “attach a photo”, then more fields. Could be a button that calls Dashboard modal, but if it's inline, many would want to place Dashboard inside the form then, and we have a conflict.

Example (I mean the drag-drop area this time):

@arturi
Copy link
Contributor Author

arturi commented Jan 13, 2023

Oh found this by @aduh95 that we use in the meta field editor (which I forgot about): #3113

And also got me thinking, what about custom inputs that we support in the meta editor? They will not have our stopPropagation/preventDefault and will submit the outer form?

@Murderlon
Copy link
Member

And also got me thinking, what about custom inputs that we support in the meta editor? They will not have our stopPropagation/preventDefault and will submit the outer form?

I think in this case we control the "submit" button, right? People are only allowed to add fields.

@arturi
Copy link
Contributor Author

arturi commented Jan 16, 2023

I think in this case we control the "submit" button, right? People are only allowed to add fields.

Yes, but the current issue and PR is with the field, if you press enter in the field (input), without event.preventDefault(), the outer form submits?

@mifi
Copy link
Contributor

mifi commented Jan 16, 2023

let's go with form attribute then - best of both worlds

@arturi
Copy link
Contributor Author

arturi commented Jan 16, 2023

Yep, originally @aduh95's idea, already implemented in

I will try to replicate this everywhere we have inputs with submits.

@arturi arturi changed the title Prevent submitting outer form from Url/Undplash input enter Prevent submitting outer form when enter is pressed in Url/Unsplash plugin Jan 16, 2023
@arturi
Copy link
Contributor Author

arturi commented Jan 23, 2023

Superseded by #4283

@arturi arturi closed this Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants