Skip to content

#458 - allow user to set formenctype #459

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

Merged
merged 7 commits into from
Jun 27, 2024

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
9 checks passed
@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