Skip to content

Conversation

@djyotta
Copy link
Contributor

@djyotta djyotta commented Jun 25, 2024

Closes #458

Copy link
Collaborator

@lovasoa lovasoa left a comment

Choose a reason for hiding this comment

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

Good. Can we document this behavior somewhere? Maybe in one of the examples of the form component?

@djyotta
Copy link
Contributor Author

djyotta commented Jun 25, 2024

@lovasoa will do. Still testing it. I had some trouble with applying it to the textarea input directly. Seems adding it to the submit button works.

I want to make sure this works with also a file input present as they would both trigger adding the attribute to the submit button...

@lovasoa lovasoa marked this pull request as draft June 25, 2024 12:00
@lovasoa
Copy link
Collaborator

lovasoa commented Jun 25, 2024

Okay, good! Switching this pr to draft mode, then.
Feel free to switch it back whenever you want.

@djyotta
Copy link
Contributor Author

djyotta commented Jun 25, 2024

@lovasoa after reading the following:

I decided to expose the enctype as top-level attribute as formenctype is intended for submit and image type inputs only.

Further, I would suggest we remove the formenctype being added automatically when file type input is present. Even though it's nice for newbies, there are the following short-comings:

  • it overrides the form enctype - could be confusing when dev explicitly sets this
  • it won't be automatically applied to other submit buttons (if any)

For now I updated the docs recommending to set it explicitly for file upload rather than rely on current behaviour. I also added an example of setting different formenctype on submit buttons to labour the point.

We can decide whether to alter existing behaviour independently of this PR

@djyotta djyotta marked this pull request as ready for review June 25, 2024 22:57
@lovasoa
Copy link
Collaborator

lovasoa commented Jun 26, 2024

I understand what you mean. But the whole promise of SQLPage is to make it easy to build UIs on top of databases without any other knowledge than just sql. I really think file upload should work by default, and not require even knowing what formenctype means.

@djyotta
Copy link
Contributor Author

djyotta commented Jun 27, 2024

For reference - here is proposed changes that I think will make the handling of form encoding to work intuitively whilst still allowing flexability for developer to set specific encoding

djyotta#1

Copy link
Collaborator

@lovasoa lovasoa left a comment

Choose a reason for hiding this comment

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

Ok, let's merge it like this at the moment, I'll update it later. Thanks for the contribution!

@lovasoa lovasoa merged commit 032109d into sqlpage:main Jun 27, 2024
@djyotta
Copy link
Contributor Author

djyotta commented Jun 27, 2024

No problem. Thanks for SQLpage making UI easy.

Sorry I couldn't catch the vision on this one. It's hard to see the forrest through the trees sometimes.

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.

Support multipart/form-data as encoding type on form component

2 participants