Checkbox values in a form #139

New Issue

2018-03-14T09:35:01-05:00

zeraphie commented

2018-03-14 09:35:01 -05:00

(Migrated from github.com)

Hey again,

I've just been checking through this with a form and I've noticed some strangeness with how forms are being handled (namely, checkboxes in forms)

The default behaviour for a browser is that on submitting a form, it will only include a checkbox if it is checked, and that the value of the checkbox is what is sent. I.e. you can attach the value attribute as what gets submitted, and it changes the value that gets submitted (default is "on")

However, after debugging it some more, I've realised that the parser for the form may be looking to see if the value attribute may be present and submitting that value, instead of checking first if it's checked.

To compare the results, I added a simple log below the paramObject parsing and discovered the difference (using FormData to get the data from the form)

  var paramObject = []
  for (var elementKey in el.elements) {
    var element = el.elements[elementKey]
    // jscs:disable disallowImplicitTypeConversion
    if (!!element.name && element.attributes !== undefined && element.tagName.toLowerCase() !== "button") {
      // jscs:enable disallowImplicitTypeConversion
      if ((element.attributes.type !== "checkbox" && element.attributes.type !== "radio") || element.checked) {
        paramObject.push({name: encodeURIComponent(element.name), value: encodeURIComponent(element.value)})
      }
    }
  }

  console.log([...new FormData(el).entries()], paramObject);

As a result of this, I'd suggest changing the parser to using FormData, something like the following

  // Get the entries of the FormData object, and reformat them into an array
  // of objects with a name and value
  let paramObject = [...new FormData(el).entries()].reduce((total, item) => {
    total.push({
      name: item[0],
      value: item[1]
    });
    
    return total;
  }, []);

For the moment I've hotpatched this as it's crucial for the application I've added in

Hey again, I've just been checking through this with a form and I've noticed some strangeness with how forms are being handled (namely, checkboxes in forms) The default behaviour for a browser is that on submitting a form, it will only include a checkbox if it is checked, and that the value of the checkbox is what is sent. I.e. you can attach the value attribute as what gets submitted, and it changes the value that gets submitted (default is "on") However, after debugging it some more, I've realised that the parser for the form may be looking to see if the value attribute may be present and submitting that value, instead of checking first if it's checked. To compare the results, I added a simple log below the paramObject parsing and discovered the difference (using FormData to get the data from the form) ```javascript var paramObject = [] for (var elementKey in el.elements) { var element = el.elements[elementKey] // jscs:disable disallowImplicitTypeConversion if (!!element.name && element.attributes !== undefined && element.tagName.toLowerCase() !== "button") { // jscs:enable disallowImplicitTypeConversion if ((element.attributes.type !== "checkbox" && element.attributes.type !== "radio") || element.checked) { paramObject.push({name: encodeURIComponent(element.name), value: encodeURIComponent(element.value)}) } } } console.log([...new FormData(el).entries()], paramObject); ``` As a result of this, I'd suggest changing the parser to using FormData, something like the following ```javascript // Get the entries of the FormData object, and reformat them into an array // of objects with a name and value let paramObject = [...new FormData(el).entries()].reduce((total, item) => { total.push({ name: item[0], value: item[1] }); return total; }, []); ``` For the moment I've hotpatched this as it's crucial for the application I've added in

BehindTheMath commented

2018-03-14 13:11:44 -05:00

(Migrated from github.com)

Unfortunately, FormData.entries() is a newer feature, so not all browsers support it.

Unfortunately, `FormData.entries()` is a newer feature, so not all browsers support it.

robinnorth commented

2018-03-14 15:08:54 -05:00

(Migrated from github.com)

Hmm, any chance you can put together a test case using JSFiddle or similar? There's a form example under example/forms.html you can use as a base.

The current method for building the request params object should only be including checked inputs though, as if ((element.attributes.type !== "checkbox" && element.attributes.type !== "radio") || element.checked) { results in all checkboxes and radio buttons except those that are checked being skipped.

Hmm, any chance you can put together a test case using JSFiddle or similar? There's a form example under `example/forms.html` you can use as a base. The current method for building the request params object should only be including checked inputs though, as `if ((element.attributes.type !== "checkbox" && element.attributes.type !== "radio") || element.checked) {` results in all checkboxes and radio buttons except those that are checked being skipped.

robinnorth commented

2018-03-14 15:13:45 -05:00

(Migrated from github.com)

However, the PR that fixed various issues around form submission (#129) landed after 0.2.5 was released, so unless you're using the latest code from master, you won't have this in your copy of the library -- please can you confirm which version of Pjax you're using?

robinnorth commented

2018-03-15 04:43:42 -05:00

(Migrated from github.com)

Yeah, just checked and the code sample you posted shows that you're using 0.2.5 (the same code block from master looks like this) -- checkout the library from master and I think you'll find everything should work as expected.

Yeah, just checked and the code sample you posted shows that you're using 0.2.5 (the same code block from master looks like [this](https://github.com/MoOx/pjax/blob/05fa833169712bcdc9e0db723022495dbc5a15c1/lib/proto/attach-form.js#L53-L57)) -- checkout the library from master and I think you'll find everything should work as expected.

zeraphie commented

2018-03-15 09:33:49 -05:00

(Migrated from github.com)

I'm using yarn, and the latest release is 0.2.5 on there

BehindTheMath commented

2018-03-15 09:35:59 -05:00

(Migrated from github.com)

Try installing from the Github repo instead, so you get the latest commits.

With npm, that would be npm install moox/pjax. Yarn probably has a similar command.

Try installing from the Github repo instead, so you get the latest commits. With npm, that would be `npm install moox/pjax`. Yarn probably has a similar command.

zeraphie commented

2018-03-15 12:35:14 -05:00

(Migrated from github.com)

That seems to have fixed it now, it was a bit of a pain to get though, when is 0.2.6 coming out? I usually keep all of my assets as the required version which is why I ask

BehindTheMath commented

2018-03-15 12:36:10 -05:00

(Migrated from github.com)

When all the bugs are fixed. 😀

When all the bugs are fixed. :grinning:

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#139