MoOx / pjax

Easily enable fast Ajax navigation on any website (using pushState + xhr)
MIT License
1.46k stars 125 forks source link

Bug with submit form - radio and checkbox #124

Closed BiserStoilov closed 6 years ago

BiserStoilov commented 6 years ago

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 6 years ago

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 6 years ago

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

BiserStoilov commented 6 years ago

@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 6 years ago

@BiserStoilov Looks good. Tested it :+1:

borkor commented 6 years ago

@BiserStoilov Can you make a pull request?

robinnorth commented 6 years ago

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 6 years ago

Fixed in #129