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

Selection of checkboxes should based on values instead of order in DOM #35

Closed
yusaf opened this issue Aug 12, 2023 · 7 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@yusaf
Copy link

yusaf commented Aug 12, 2023

Describe the bug
When filling the form with data using FormDataJson.fromJson .... for checkboxes the checkbox is only selected if the data provided is the exact order that checkboxes appear in the DOM

To Reproduce
In the following example https://jsfiddle.net/yusafme/vuj3kth5/3/ the checkboxes are checked as you would expect

<form>
  Email<input name="communication[preference][]" type="checkbox" value="email">
  SMS<input name="communication[preference][]" type="checkbox" value="sms">
  Letter<input name="communication[preference][]" type="checkbox" value="letter">
</form>
<script>
let form = document.querySelector("form")
FormDataJson.fromJson(form, {communication:{
	preference:["email", "sms"]
}});
</script>

However in this example https://jsfiddle.net/yusafme/vuj3kth5/4/ only the first checkbox is checked, but you would want both the email checkbox and letter checkbox to be checked

<form>
  Email<input name="communication[preference][]" type="checkbox" value="email">
  SMS<input name="communication[preference][]" type="checkbox" value="sms">
  Letter<input name="communication[preference][]" type="checkbox" value="letter">
</form>
<script>
let form = document.querySelector("form")
FormDataJson.fromJson(form, {communication:{
	preference:["email", "letter"]
}});
</script>
@yusaf yusaf added the bug Something isn't working label Aug 12, 2023
@yusaf yusaf changed the title Selection of checkboxes based on values instead of order Selection of checkboxes should based on values instead of order in DOM Aug 12, 2023
@brainfoolong
Copy link
Owner

Thanks for your report. I think this is almost impossible to fix for how the internals currently work, without breaking any other working edge cases.

Undefined input names (with an implicit key from []) are tricky. To make this working, the internal recursion must be totally reworked, as it's currently unclear what name matches the exact values. It would require a separate register with all the checkbox values in each name level, then checking that. It gets even more complicated when somebody mixes named and implicit keys.

I don't tend to touch this right now.

If you think the other way, uniquely named input names with a proper object instead of array values, example communication[preference][sms] and {communication:{ preference:[{"sms": "sms"}} it will work without problems.

@yusaf
Copy link
Author

yusaf commented Aug 12, 2023

BTW thank you soo much for this project, really very grateful, I'm using it in my own project which hopefully I'll share with you when complete.

I tried going through the internals, but honestly I got lost very quickly !!!

I have done a few workarounds within my project already, but was really struggling with this issue

Although I may have thought of another workaround just now!!!, something along the lines

form.querySelectorAll('[name="communication[preference][]"][value="letter"]').checked = true

Would it be possible within the internals if the key is [] to iterate over the array of values and check the checkboxes like above?

@brainfoolong
Copy link
Owner

brainfoolong commented Aug 12, 2023

Unfortunately no. The inputs are traversed in a tree manner, depending on the options given. There is no deeper selector use at all. Basically its just traversing the name-tree and traversing the given object-tree and comparing the same levels and values.

Keeping track of which tree level you are is the hard part and not easy to change in this project without breaking anything.

And as you see in your example, the trees are not equal, so its not finding it.

It's sadly just a case that don't work right now. I understand your thing, but don't have time and muse to touch it now just because i know how hard it will be to fix and not breaking anything else :)

@yusaf
Copy link
Author

yusaf commented Aug 12, 2023

Completely understand, thanks for your time and quick responses

I've come up with a hacky workaround for now https://jsfiddle.net/yusafme/vuj3kth5/18/ !!!

let form = document.querySelector('form');

function fromJson(data) {
  FormDataJson.fromJson(form, data);
  checkBoxes(data, '');
}

function checkBoxes(data, nameAttr = '') {
  const isStringArray = Array.isArray(data) && data.every((str) => typeof str === 'string');
  if (isStringArray) {
    if (!form.querySelectorAll(`[name="${nameAttr}[]"]`).length) {
      return;
    }
    for (let i = 0, iLen = data.length; i < iLen; i++) {
      const checkbox = form.querySelector(`[name="${nameAttr}[]"][value="${data[i]}"]`);
      if (checkbox) {
        checkbox.checked = true;
      }
    }
    return;
  }

  for (let key in data) {
    const newNameAttr = nameAttr === '' ? key : nameAttr + `[${key}]`;
    if (data[key] && typeof data[key] === 'object') {
      checkBoxes(data[key], newNameAttr);
    }
  }
}

fromJson({
  communication: {
    preference: ['email', 'letter']
  }
});

@brainfoolong
Copy link
Owner

I was curious about a fix... And tried something. Maybe give it a shot from source of current main?

59bbdac

@yusaf
Copy link
Author

yusaf commented Aug 13, 2023

Fantastic, working like a charm!

Thanks soo much you've been a great help :)

@brainfoolong
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants