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.

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

Add embed example. #863

Merged
merged 3 commits into from May 4, 2018

Conversation

Projects
3 participants
@sheeeng
Contributor

sheeeng commented May 1, 2018

Fix #739.

@sheeeng

This comment has been minimized.

Contributor

sheeeng commented May 1, 2018

I did not manage to disable autoplay video inside the <embed> element.
Tried autoplay="false" and autostart="false", but to no avail.

Any suggestions to disable autoplay of video inside the <embed> element?

@sheeeng sheeeng changed the title from Draft embed example. to Add embed example. May 1, 2018

@wbamberg

This comment has been minimized.

Member

wbamberg commented May 2, 2018

@sheeeng , thanks again for your contribution! It looks great.

Any suggestions to disable autoplay of video inside the element?

I don't know how to do this either. Maybe @schalkneethling does?

I think it would be better to take a copy of the video under /media/examples, rather than stream it from another site. I think perhaps the one here http://techslides.com/sample-webm-ogg-and-mp4-video-files-for-html5 would be a good choice, for a few reasons:

  • it's quite small (the webm is the smallest so perhaps we should choose that)
  • it's provided as a sample to use
  • it's short and not annoying, so I think it would be acceptable even if we can't prevent autoplay

What do you think?

.output {
background: #eee;
position: relative;
}

This comment has been minimized.

@wbamberg

wbamberg May 2, 2018

Member

Please make sure files end with a new line.

@sheeeng

This comment has been minimized.

Contributor

sheeeng commented May 2, 2018

Pre-commit hooks failed. Any idea?

husky > npm run -s precommit (node v8.4.0)

 PASS  __tests__/console-utils.test.js

 RUNS  __tests__/puppeteer-js-editor.test.js

Test Suites: 1 passed, 1 of 2 total
Tests:       12 passed, 12 total
 FAIL  __tests__/puppeteer-js-editor.test.js (5.371s)
  ● JS Editor › renders expected output after clicking Run

    Timeout - Async callback was not invoked within the 5000ms timeout specified by jest.setTimeout.

      at node_modules/jest-jasmine2/build/queue_runner.js:68:21

Test Suites: 1 failed, 1 passed, 2 total
Tests:       1 failed, 12 passed, 13 total
Snapshots:   0 total
Time:        6.14s
Ran all test suites.

husky > pre-commit hook failed (add --no-verify to bypass)
@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented May 2, 2018

@sheeeng make sure there is no other instance of the dev server running. Also make sure you have installed all the latest things by running npm install

@sheeeng

This comment has been minimized.

Contributor

sheeeng commented May 2, 2018

@schalkneethling, thanks! It works now after npm install && npm run build && npm start and then browse http://localhost:4444/pages/tabbed/embed.html.

type="video/webm"
src="/media/examples/small.webm"
width="320"
height="240">

This comment has been minimized.

@schalkneethling

schalkneethling May 2, 2018

Collaborator

@sheeeng Here is how to prevent it from auto playing ~ https://stackoverflow.com/a/30104133/355766

This comment has been minimized.

@sheeeng

sheeeng May 2, 2018

Contributor
--- a/live-examples/html-examples/embedded-content/embed.html
+++ b/live-examples/html-examples/embedded-content/embed.html
@@ -1,6 +1,8 @@
 <embed
     type="video/webm"
     src="/media/examples/small.webm"
+    autoplay="false"
+    autostart="false"
     width="320"
     height="240">

Tried both "false" and "0", but the video still playing automatically on Firefox 59.0.2 browser.

This comment has been minimized.

@schalkneethling

schalkneethling May 2, 2018

Collaborator

@sheeeng And in Chrome?

This comment has been minimized.

@sheeeng

sheeeng May 2, 2018

Contributor

Video plays automatically on Chrome 66.0.3359.139 with "false" and "0" too.

This comment has been minimized.

@wbamberg

wbamberg May 2, 2018

Member

Yes, I found that post when reviewing this PR, but none of those worked for me either. I think given a short, non-annoying video (which this is) then autoplay is acceptable. But I'd like to get @schalkneethling opinion too, before merging this.

This comment has been minimized.

@sheeeng

sheeeng May 3, 2018

Contributor

I have uploaded another video example with WebM and MP4 format at #885. Perhaps, we can decide which video example to use in both places?

@schalkneethling schalkneethling added this to To do in Q2 - Sprint 2 via automation May 2, 2018

@schalkneethling schalkneethling added this to the Quarter 2 ~ Sprint 2 milestone May 2, 2018

@sheeeng

This comment has been minimized.

Contributor

sheeeng commented May 3, 2018

Sorry for the mess. I will clean them.
I did git pull git@github.com:mdn/interactive-examples.git master --rebase on my working branch.

sheeeng added some commits May 3, 2018

@sheeeng

This comment has been minimized.

Contributor

sheeeng commented May 3, 2018

Use audioless video for the example by default.
Folks can remove the trailing _audioless on the filename to try the video with audio.

@wbamberg

Thanks again for your contribution @sheeeng ! This looks good to me.

Note that the footer isn't positioned correctly, this is due to #881, and that will need to be resolved anyway for all the similar examples, so it should not stop us merging this one.

@wbamberg wbamberg merged commit eb47ba5 into mdn:master May 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

Q2 - Sprint 2 automation moved this from To do to Done May 4, 2018

wbamberg added a commit to wbamberg/interactive-examples that referenced this pull request May 5, 2018

Merge remote-tracking branch 'upstream/master'
* upstream/master:
  Add interactive example for <section> (mdn#888)
  Add embed example. (mdn#863)
  Add <span> example (mdn#882)
  chore: add @tbazin as contributor (mdn#889)
  Improve naming of unbound function example (mdn#886)
  Add html input number (mdn#866)
  Add html input range (mdn#878)
  chore(deps): update dependency jest-puppeteer to v3 (mdn#879)
  Add input-image example (mdn#864)
  Add html input radio (mdn#868)
  Add iframe example. (mdn#875)
  fix: change output for tabbed editor to shadow dom (mdn#873)
  Remove unused CSS
  Add input-month example

@sheeeng sheeeng deleted the sheeeng:add-embed-example branch May 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment