processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.13k stars 3.23k forks source link

p5.dom `select('form')` does not select `<form>` element #6836

Closed lindapaiste closed 1 month ago

lindapaiste commented 4 months ago

Most appropriate sub-area of p5.js?

p5.js version

1.9.1

Web browser and version

No response

Operating system

Windows 10

Steps to reproduce this

Steps:

  1. Include a <form> element in the HTML.
  2. Access the element in p5 using form = select('form').
  3. Log the underlying HTML element of the p5.Element with console.log(form.elt).
  4. It's an empty div <div></div>??

This previously worked correctly using v0.9.0 of the standalone p5.dom package. I wondered if it was an intentional change that you can no longer select by tag name, but the documentation says that "The string can be an ID, class, tag name, or a combination" and includes an example with select('canvas').

Snippet:

https://editor.p5js.org/lindapaiste2/sketches/QX6xg14AC

function setup() {
  createCanvas(400, 400);

  const formNativeEl = document.getElementsByTagName('form')[0];
  formNativeEl.addEventListener('change', () => console.log('native form onChange fired')); // this fires

  const formP5El = select('form');
  formP5El.changed(() => console.log('p5 form onChange fired')); // never fires

  console.log('is same el', formP5El.elt === formNativeEl); // false

  console.log('native el', formNativeEl); // <form>...</form>

  console.log('p5 el', formP5El, formP5El.elt); // _class {elt: HTMLDivElement, ...}, <div></div>
}

function draw() {
}
<!DOCTYPE html>
<html lang="en">
  <head>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.9.1/p5.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.9.1/addons/p5.sound.min.js"></script>
    <link rel="stylesheet" type="text/css" href="style.css" />
    <meta charset="utf-8" />
  </head>
  <body>
    <main>
      <form>
        <label>
          <input type="checkbox" id="apples" />
          Apples
        </label>
        <label>
          <input type="checkbox" id="bananas" />
          Bananas
        </label>
      </form>
    </main>
    <script src="sketch.js"></script>
  </body>
</html>
lindapaiste commented 4 months ago

I looked into the source code and the reason that this is happening is due to some special handling in the p5.prototype._wrapElement function, where if every child of the element is an <input> it will call createRadio.

https://github.com/processing/p5.js/blob/fd5dc4c4ac440e02287e9d03e231477f4cf8c6fd/src/dom/dom.js#L228-L234

It's supposed to use the p5.Element as the container for the radio, but that only works if the element is a <span> or <div> and does not work here because the element is a <form>.

https://github.com/processing/p5.js/blob/fd5dc4c4ac440e02287e9d03e231477f4cf8c6fd/src/dom/dom.js#L1465-L1468

So the createRadio function creates a new div element and sets it as the .elt.

https://github.com/processing/p5.js/blob/fd5dc4c4ac440e02287e9d03e231477f4cf8c6fd/src/dom/dom.js#L1483-L1485

This means that calling select('form') when all of the form children are input will return a p5.Element with reference to this new empty <div> and no reference to the actual <form> element . Trying to use this p5.Element will not work as intended, because it is not the right element.


The simplest fix here is that the portion of the code which conditionally calls createRadio should also check if the element is able to be a radio parent element. That is, && (elt instanceof HTMLDivElement || elt instanceof HTMLSpanElement).