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.

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

Disable autoplay on native videos #10516

Merged
merged 1 commit into from Oct 1, 2020
Merged

Conversation

reobin
Copy link
Contributor

@reobin reobin commented Oct 1, 2020

What type of PR is this? (check all applicable)

  • Refactor
  • Feature? or is it
  • Bug Fix
  • Optimization
  • Documentation Update

Description

I'm not sure it 100% closes #6067 because the issue refers to a user setting for video autoplay.

The general consensus, for the moment, was to simply disable autoplay.

The autostart value was removed since it defaults to false, and the play() on ready was also removed.

Related Tickets & Documents

#6067

QA Instructions, Screenshots, Recordings

  • Load up any article with a native video
  • Verify that it doesn't autoplay

Added tests?

  • yes
  • no, because they aren't needed
  • no, because I need help

Added to documentation?

  • docs.forem.com
  • readme
  • no documentation needed

@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 1, 2020
Copy link
Contributor

@fdocr fdocr left a comment

Choose a reason for hiding this comment

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

LGTM

Also here's how it looks to me locally in case other reviewers have trouble setting up videos locally: https://share.getcloudapp.com/6quPk0XA

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Oct 1, 2020
Copy link
Contributor

@vaidehijoshi vaidehijoshi left a comment

Choose a reason for hiding this comment

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

Thanks for this, @reobin, this looks good! Let's try it out :)

@fdocr fdocr merged commit 87590e7 into forem:master Oct 1, 2020
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting for disabling autoplay
3 participants