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.

Sign up for GitHub

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

Jump to bottom

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

Closed
1 of 17 tasks
lindapaiste opened this issue Mar 9, 2024 · 1 comment · Fixed by #6838
Closed
1 of 17 tasks

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

lindapaiste opened this issue Mar 9, 2024 · 1 comment · Fixed by #6838
Labels
Area:DOM Bug

Comments

@lindapaiste
Copy link
Contributor

lindapaiste commented Mar 9, 2024 •
edited
Loading

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

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 lindapaiste added the Bug label Mar 9, 2024
@lindapaiste
Copy link
Contributor Author

lindapaiste commented Mar 9, 2024 •
edited
Loading

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.

p5.js/src/dom/dom.js

Lines 228 to 234 in fd5dc4c

} else if (
children.length > 0 &&
children.every(function (c) {
return c.tagName === 'INPUT' || c.tagName === 'LABEL';
})
) {
return this.createRadio(new p5.Element(elt, this));

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>.

p5.js/src/dom/dom.js

Lines 1465 to 1468 in fd5dc4c

if (
arg0 instanceof p5.Element &&
(arg0.elt instanceof HTMLDivElement || arg0.elt instanceof HTMLSpanElement)
) {

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

p5.js/src/dom/dom.js

Lines 1483 to 1485 in fd5dc4c

radioElement = document.createElement('div');
self = addElement(radioElement, this);
this.elt = radioElement;

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).

@lindapaiste lindapaiste mentioned this issue Mar 9, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area:DOM Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check the element type before calling createRadio from select() lindapaiste/p5.js
1 participant
@lindapaiste