Edit docs for dom objects #6552
Conversation
|
|
Great suggestions @MsQCompSci – just added examples for I just double-checked and it turns out I edited p5.Element.changed() and p5.Element.input() as part of #6534. I accidentally lumped them in with the top-level DOM functions because they're in a different part of the file. |
|
@davepagurek @limzykenneth @Qianqianye if these changes look good to you, please go ahead and merge them. |
src/dom/dom.js
Outdated
| * // Create a p5.MediaElement using createAudio(). | ||
| * beat = createAudio('assets/beat.mp3'); | ||
| * // Play the beat in a loop. | ||
| * beat.loop(); |
There was a problem hiding this comment.
I think this one might not work when navigating directly to the page since it's trying to autoplay sound. Maybe this could be a muted video so you can see it play immediately? Or maybe double clicking starts the loop instead of stops it?
There was a problem hiding this comment.
I haven't tried in awhile but last I remember double click can be a bit flaky on touch screens (they will zoom instead) so might want to consider just using single click overall.
src/dom/dom.js
Outdated
| * // Create a p5.MediaElement using createAudio(). | ||
| * beat = createAudio('assets/beat.mp3'); | ||
| * // Play the beat in a loop. | ||
| * beat.loop(); |
src/dom/dom.js
Outdated
| * // Create a p5.MediaElement using createAudio(). | ||
| * dragon = createAudio('assets/lucky_dragons.mp3'); | ||
| * // Play the audio in a loop. | ||
| * dragon.loop(); |
src/dom/dom.js
Outdated
| * textAlign(CENTER); | ||
| * text('start at beginning', width / 2, height / 2); | ||
| * textWrap(CHAR); | ||
| * text('Double-click to goto 2 seconds', 10, 10); |
There was a problem hiding this comment.
Might need to adjust the coordinates for the text:
src/dom/dom.js
Outdated
| * | ||
| * // Create a p5.MediaElement using createAudio(); | ||
| * let beat = createAudio('assets/beat.mp3'); | ||
| * beat.autoplay(true); |
There was a problem hiding this comment.
We might need it to be muted for this to work reliably. Maybe doing video instead of audio for a lot of these examples would let us keep the autoplay behaviour while muted?
|
Great suggestions @davepagurek @limzykenneth. I tried to work around most of these issues using |
src/dom/dom.js
Outdated
| * dragon.showControls(); | ||
| * | ||
| * // Jump to a random starting time. | ||
| * let d = dragon.duration(); |
There was a problem hiding this comment.
oh looks like the tests are failing on this one. It seems like duration() is NaN on first load. You might have to do this code block in the loaded callback for createAudio?
There was a problem hiding this comment.
Good catch. Starting at a random time isn't essential, so now it just jumps to 2 seconds. Feels more focused.
Addresses #6511
Changes:
I edited the inline docs for objects in
src/dom/dom.jsincluding:p5.MediaElementp5.File@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