Bug with submit form - radio and checkbox #124

Closed
opened 2018-02-17 05:12:45 -05:00 by BiserStoilov · 7 comments
BiserStoilov commented 2018-02-17 05:12:45 -05:00 (Migrated from github.com)

Hi,

i found small bug when pjax submit form with radio and checkbox.

This code in attach-form.js

if ((element.attributes.type !== "checkbox" && element.attributes.type !== "radio") || element.checked) { paramObject.push({name: encodeURIComponent(element.name), value: encodeURIComponent(element.value)}) }

maybe must replace with this

if ((element.attributes.type.nodeValue !== "checkbox" && element.attributes.type.nodeValue !== "radio") || element.checked) { paramObject.push({ name: encodeURIComponent(element.name), value: encodeURIComponent(element.value) }) }

Best regards

Hi, i found small bug when pjax submit form with radio and checkbox. This code in attach-form.js ` if ((element.attributes.type !== "checkbox" && element.attributes.type !== "radio") || element.checked) { paramObject.push({name: encodeURIComponent(element.name), value: encodeURIComponent(element.value)}) } ` maybe must replace with this ` if ((element.attributes.type.nodeValue !== "checkbox" && element.attributes.type.nodeValue !== "radio") || element.checked) { paramObject.push({ name: encodeURIComponent(element.name), value: encodeURIComponent(element.value) }) } ` Best regards
robinnorth commented 2018-02-17 06:17:37 -05:00 (Migrated from github.com)

Thanks @BiserStoilov. Attr.nodeValue is deprecated in favour of Attr.value, but otherwise I think you are right.

Feel free to submit a PR (ideally with tests) to implement this, if you like, otherwise this will get looked at at some point for a future release.

Thanks @BiserStoilov. `Attr.nodeValue` is deprecated in favour of `Attr.value`, but otherwise I think you are right. Feel free to submit a PR (ideally with tests) to implement this, if you like, otherwise this will get looked at at some point for a future release.
borkor commented 2018-02-17 06:58:25 -05:00 (Migrated from github.com)

Unfortunately this solution will not work with Select. Just tested it.

Unfortunately this solution will not work with Select. Just tested it.
BiserStoilov commented 2018-02-17 08:12:15 -05:00 (Migrated from github.com)

@borkor this work for me with form with any element:

var elementType; if (element.tagName.toLowerCase() === "input") { if (element.type === "text") { elementType = "input"; } if (element.type === "checkbox") { elementType = "checkbox"; } if (element.type === "radio") { elementType = "radio"; } } if (element.tagName.toLowerCase() === "select") { elementType = "select"; } if ((elementType !== "checkbox" && elementType !== "radio") || element.checked) { paramObject.push({ name: encodeURIComponent(element.name), value: encodeURIComponent(element.value) }) }

Please, test...

Best regards

@borkor this work for me with form with any element: `var elementType; if (element.tagName.toLowerCase() === "input") { if (element.type === "text") { elementType = "input"; } if (element.type === "checkbox") { elementType = "checkbox"; } if (element.type === "radio") { elementType = "radio"; } } if (element.tagName.toLowerCase() === "select") { elementType = "select"; } if ((elementType !== "checkbox" && elementType !== "radio") || element.checked) { paramObject.push({ name: encodeURIComponent(element.name), value: encodeURIComponent(element.value) }) }` Please, test... Best regards
borkor commented 2018-02-17 10:49:11 -05:00 (Migrated from github.com)

@BiserStoilov Looks good. Tested it 👍

@BiserStoilov Looks good. Tested it :+1:
borkor commented 2018-02-17 10:56:01 -05:00 (Migrated from github.com)

@BiserStoilov Can you make a pull request?

@BiserStoilov Can you make a pull request?
robinnorth commented 2018-02-21 13:38:30 -05:00 (Migrated from github.com)

Thanks for your PR, @BiserStoilov. In fixing #126, however, I have also resolved this issue in my PR. Sorry about that!

Perhaps you could check out the branch and test to see if this resolves your original issue?

Thanks for your PR, @BiserStoilov. In fixing #126, however, I have also resolved this issue in my PR. Sorry about that! Perhaps you could check out the branch and test to see if this resolves your original issue?
robinnorth commented 2018-03-02 15:25:59 -05:00 (Migrated from github.com)

Fixed in #129

Fixed in #129
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#124