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

Serialize form using UrlSearchParams #26

Merged
merged 13 commits into from
Aug 30, 2022
Merged

Serialize form using UrlSearchParams #26

merged 13 commits into from
Aug 30, 2022

Conversation

daun
Copy link
Member

@daun daun commented Aug 28, 2022

  • Switch to native URLSearchParams API to serialize form data before submission
  • Fixes Doesn't submit textareas #25
  • Simplifies code and improves handling of edge cases
  • Changes browser compatibility, so probably worth a major version

@daun daun marked this pull request as ready for review August 28, 2022 20:28
@daun daun requested review from hirasso and gmrchk and removed request for hirasso August 28, 2022 20:28
@hirasso
Copy link
Member

hirasso commented Aug 30, 2022

While testing this PR I stumbled upon a possible bug: The form's get parameter isn't being replaced in the URL but instead appended. This leads to this if submitting a form multiple times:

?searched=somtething&searched=another&searched=yet+another

https://test.rassohilber.com/swup-forms-plugin/test/

If you submit the form a few times you can see how the URL grows... and grows... :)

This bug actually already existed before the edits in this branch. Should I take care of fixing this and just push it to this PR as well? Or too messy?

@daun
Copy link
Member Author

daun commented Aug 30, 2022

@hirasso Right, nice catch!

I've never come across this one as I always set the action attribute, but yes, we should handle it the same as the browser would. The solution is probably to construct the action from window.location.path instead of window.location.href for GET requests, right? I can push a fix later.

@daun
Copy link
Member Author

daun commented Aug 30, 2022

Looks like that's exactly what browsers do for GET requests: strip everything after the question mark.

@daun
Copy link
Member Author

daun commented Aug 30, 2022

@hirasso I've fixed the mentioned bug and did some additional cleanup. Ready for review :)

src/index.js Show resolved Hide resolved
@hirasso hirasso self-requested a review August 30, 2022 12:23
hirasso
hirasso previously approved these changes Aug 30, 2022
@hirasso hirasso self-requested a review August 30, 2022 12:38
@daun daun deleted the url-search-params branch August 30, 2022 12:53
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.

Doesn't submit textareas
2 participants