Revise docs for dom functions #6534
Conversation
|
Edits look great! |
src/dom/dom.js
Outdated
| * @param {String|p5.Element|HTMLElement} [container] CSS selector string, <a href="#/p5.Element">p5.Element</a> | ||
| * , or HTML element to search within | ||
| * @return {p5.Element[]} Array of <a href="#/p5.Element">p5.Element</a>s containing nodes found | ||
| * @param {String} selectors CSS selector string of element to search for.. |
There was a problem hiding this comment.
Small typo I think:
| * @param {String} selectors CSS selector string of element to search for.. | |
| * @param {String} selectors CSS selector string of element to search for. |
src/dom/dom.js
Outdated
| @@ -339,18 +486,28 @@ p5.prototype.createDiv = function (html = '') { | |||
| }; | |||
|
|
|||
| /** | |||
| * Creates a `<p></p>` element in the DOM with given inner HTML. Used | |||
| * for paragraph length text. | |||
| * Creates a `<p></p>` element. It's commonly use for | |||
There was a problem hiding this comment.
Small typo:
| * Creates a `<p></p>` element. It's commonly use for | |
| * Creates a `<p></p>` element. It's commonly used for |
src/dom/dom.js
Outdated
| @@ -359,16 +516,42 @@ p5.prototype.createP = function (html = '') { | |||
| }; | |||
|
|
|||
| /** | |||
| * Creates a `<span></span>` element in the DOM with given inner HTML. | |||
| * Creates a `<span></span>` element. It's commonly used as a | |||
| * container for other elements. | |||
There was a problem hiding this comment.
We also say this for <div>, maybe we can add something small about it being generally used as a container for inline things? Not sure what the best wording for that is. Things that might wrap along with text?
src/dom/dom.js
Outdated
| * background(200); | ||
| * | ||
| * // Create an h3 element within the span. | ||
| * let span = createSpan('<h3>p5*js</h3>'); |
There was a problem hiding this comment.
This works but I also wonder if there's a way to illustrate the difference with a div here. Maybe if you have a div that already has some text, you could add a span with red text into the middle of its contents?
src/dom/dom.js
Outdated
| @@ -377,29 +560,54 @@ p5.prototype.createSpan = function (html = '') { | |||
| }; | |||
|
|
|||
| /** | |||
| * Creates an `<img>` element in the DOM with given src and | |||
| * alternate text. | |||
| * Creates an `<img>` element. | |||
There was a problem hiding this comment.
Maybe worth differentiating this from p5.Image: this exists outside of the canvas while p5.Image is only visible when you draw it on the canvas
src/dom/dom.js
Outdated
| * the button. | ||
| * | ||
| * The second parameter, `value`, is optional. It's a string that sets the | ||
| * button's value. See <a href="#/p5.Element/value">p5.Element.value()</a> for |
There was a problem hiding this comment.
Think it's worth linking to MDN here https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#value for an explanation of why a button might need a value and how it gets used? It's definitely an edge case, I didn't even realize this was a thing until a few minutes ago, but I just had to go google it to see what it might mean 😅
| * function handleVideo() { | ||
| * video.size(100, 100); | ||
| * video.loop(); | ||
| * video.volume(0); |
There was a problem hiding this comment.
If I go directly to https://computiful.dev/sod-dom-functions/reference/#/p5/createVideo to simulate coming from a google search (or search for createVideo, go to the docs, then reload) the video doesn't immediately play, I suspect this is because we're muting after looping:
Maybe we should try muting first? If that does indeed fix it, it's probably worth mentioning explicitly in the docs that that ordering matters because of how the browser deals with autoplaying media.
src/dom/dom.js
Outdated
| * // Load the audio. | ||
| * beat = createAudio('assets/beat.mp3', () => { | ||
| * // Play the audio in a loop once it's loaded. | ||
| * beat.loop(); |
There was a problem hiding this comment.
This one also doesn't work for me if I go directly to the URL for this doc. Maybe we can start it on click, or only load the audio on click? Probably worth mentioning for both audio and video that this might happen if there hasn't been a user interaction
|
Thanks @davepagurek! I fixed the typos, reworked the |
Addresses #6511
Changes:
I edited the inline docs for the top-level functions in
src/dom/dom.js. This includes everything in the DOM group except the following:p5.Elementsee Edit docs for p5.Element #6512p5.MediaElementp5.FileI'll make a separate pull request for the last four items.
@dkessner, @MsQCompSci, @davepagurek, @limzykenneth, @Qianqianye, @SarveshLimaye, @SoundaryaKoutharapu, @ramya202000, @BamaCharanChhandogi, @Obi-Engine10, @MarceloGoncalves, @hiddenenigma I'd love to incorporate any feedback you may have! Here's a staging version of the edits in this pull request.
PR Checklist
npm run lintpasses