Add the option to use FormData to encode the form elements #153

Merged
BehindTheMath merged 4 commits from feature/form-data into master 2018-04-29 14:05:23 -05:00
BehindTheMath commented 2018-04-26 08:38:55 -05:00 (Migrated from github.com)

If the form's enctype attribute is set to multipart/form-data, use FormData to encode the form's elements.

Fixes #132.

If the form's `enctype` attribute is set to `multipart/form-data`, use `FormData` to encode the form's elements. Fixes #132.
robinnorth (Migrated from github.com) reviewed 2018-04-27 03:53:13 -05:00
@@ -34,0 +45,4 @@
function parseFormElements(el) {
var requestParams = []
robinnorth (Migrated from github.com) commented 2018-04-27 03:50:06 -05:00

You can remove requestParams: [] from line 19 now, as this makes that initialisation redundant.

You can remove `requestParams: []` from line 19 now, as this makes that initialisation redundant.
robinnorth (Migrated from github.com) commented 2018-04-27 03:52:46 -05:00

You should either add var formData = requestOptions. formData || null on line 9 to match the corresponding requestParams variable we use in this module, and then replace all other instances of requestOptions.formData with formData or remove var requestParams = requestOptions.requestParams || null from line 8 and replace all instances of requestParams with requestOptions.requestParams.

You should either add `var formData = requestOptions. formData || null` on line 9 to match the corresponding `requestParams` variable we use in this module, and then replace all other instances of `requestOptions.formData` with `formData` or remove `var requestParams = requestOptions.requestParams || null` from line 8 and replace all instances of `requestParams` with `requestOptions.requestParams`.
robinnorth commented 2018-04-27 03:55:18 -05:00 (Migrated from github.com)

I think you'll also need to update your TypeScript definitions as part of this, is that correct?

I think you'll also need to update your TypeScript definitions as part of this, is that correct?
BehindTheMath commented 2018-04-27 08:45:28 -05:00 (Migrated from github.com)

I think you'll also need to update your TypeScript definitions as part of this, is that correct?

Good point.

>I think you'll also need to update your TypeScript definitions as part of this, is that correct? Good point.
BehindTheMath (Migrated from github.com) reviewed 2018-04-27 08:46:32 -05:00
@@ -34,0 +45,4 @@
function parseFormElements(el) {
var requestParams = []
BehindTheMath (Migrated from github.com) commented 2018-04-27 08:46:32 -05:00

Fixed.

Fixed.
robinnorth (Migrated from github.com) approved these changes 2018-04-27 11:46:18 -05:00
robinnorth (Migrated from github.com) left a comment

Thanks for actioning my feedback, LGTM 👍

Thanks for actioning my feedback, LGTM 👍
BehindTheMath commented 2018-04-27 11:47:48 -05:00 (Migrated from github.com)

It's great to have a second pair of eyes on it. There's always something that I miss. So thanks for that.

It's great to have a second pair of eyes on it. There's always something that I miss. So thanks for that.
robinnorth commented 2018-04-30 04:27:42 -05:00 (Migrated from github.com)

Nice one on getting this feature in there, I think it's probably time we pushed out a new version now!

Nice one on getting this feature in there, I think it's probably time we pushed out a new version now!
BehindTheMath commented 2018-04-30 08:24:55 -05:00 (Migrated from github.com)

I'm going to start working on that.

I'm going to start working on that.
robinnorth commented 2018-04-30 08:41:28 -05:00 (Migrated from github.com)

Great, let me know if there's anything I can do to help.

Great, let me know if there's anything I can do to help.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#153