Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Makes Examples Mobile-friendly #217

Open
wants to merge 3 commits into
base: release
Choose a base branch
from
Open

Conversation

justsml
Copy link

@justsml justsml commented Oct 22, 2019

Dear ml5 community, πŸ‘‹

I'm making a Pull Request(PR). Please see the details below.

β†’ Step 1: Which branch are you submitting to? 🌲

Release (for bug fixes)

β†’ Step 2: Describe your Pull Request πŸ“

Add missing viewport meta tag
Minor fix to location of <script> tags in examples.

Copy link
Contributor

@joeyklee joeyklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @justsml - Thanks so much for this! This is great. I will merge this in when ready.

note: @shiffman we definitely should address this across our examples.

@justsml justsml marked this pull request as ready for review October 22, 2019 02:05
@justsml
Copy link
Author

justsml commented Oct 22, 2019

Thanks @joeyklee 🀘

Let me know if I need to make any changes.

Copy link
Contributor

@joeyklee joeyklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @justsml - I made a note about where the sketch.js file should live below. Would it be possible to undo that revision?

<title>BodyPix with Webcam</title>
<script src="https://unpkg.com/[email protected]/dist/ml5.min.js" type="text/javascript"></script>

<style></style>
<script src="sketch.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm mistaken, but we should always try to leave the sketch.js at the end of the body tag to make sure that the DOM elements are loaded before calling the script.

This example will work fine this way, but for consistency, we should just always call script.js at the end of the index.html.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reply!
That sounds about right, I'll have time to hack on it later today 🀘

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch! Let me know whenever you're ready and I can review. If you're able to resolve any of the merge conflicts that would be great, otherwise I can also take care :)

Many thanks!

@bomanimc
Copy link
Member

Hey @justsml! We're archiving this repository and moving its contents into the core ml5-library repository (more context on issue #809 and pull request #831). We appreciate your contributions and would love it if you'd consider reopening this pull request on the ml5-library repo! Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants