Changing Select Option

[stalgiag]

First attempt. Doesn't seem like an ideal design as it requires that you to know both the name and value of the option you want to change.

Open to other suggestions!

Closes #1554

[lmccart]

@mlarghydracept I think something like this pseudcode is close-ish:

when I call: mySelect.option(name, value)

if (option exists with name) {
  if (value === false) { // this is a way to remove options
    remove option
  }
  else {
    if (option.name currently == option.value) { // we assume the user wants to update both
      option.name = option.value = value;
    } else {
      option.value = value;
      // option.name stays the same
    }
  }
} else {
  add option as normal
}

what do you think?

[stalgiag]

Thanks for the help! I implemented basically exactly what you wrote. The only small problem is that it is impossible for a user to have two options with the same name as trying to add a new one with the same name will just overwrite the value for the old one. This seems kind of unavoidable without adding an unwieldy boolean argument into the mix. For now this feels pretty user friendly!

[ggorlen]

The only small problem is that it is impossible for a user to have two options with the same name as trying to add a new one with the same name will just overwrite the value for the old one. This seems kind of unavoidable without adding an unwieldy boolean argument into the mix. For now this feels pretty user friendly!

This led to a confusing situation for a student I mentored that resulted in them spending a couple of days trying to understand. They were creating a select element from a large column pulled in from a CSV file that happened to contain duplicates. The existing p5 .option() logic resulted in undefined showing up many times in their dropdown on 0.8.0. After we upgraded to 1.4.2 while working to isolate the problem to p5, the undefineds disappeared from the option text contents but we noticed some elements were still duplicated and some values were inexplicably being set to undefined.

To make matters worse, some of the columns had unescaped HTML special characters like &. P5 uses

      const opt = document.createElement('option');
      opt.innerHTML = name;

to make the element. .innerHTML causes "foo & bar" to become "foo & bar", so the subsequent if (this.elt[i].innerHTML === name) { fails to detect the option as already existing. Could we use textContent here, or keep track of the elements in an internal data structure?

Maybe I missed it, but I don't see mention that options must be unique in the documentation. I'll open a PR for improving the documentation and open an issue for potentially changing this behavior, but I wanted to leave a note here in case anyone else runs into this.

I'm not sure if it could be considered, but I'd prefer if the behavior didn't allow changing options via the select object, but rather provided a hook on each option to change its own value and text. This would allow duplicate options and generally be less magical and error-prone.