-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Remove autoplay for video in course about page #2517
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
Conversation
The iframe was generated with autoplay=1, which, sometimes, would cause the video to play in the background.
Also, |
Actually, there’s no need to manually edit the |
@cahrens should we get a review from the studio team? |
I'm unsure what the desired change in behavior is. Is it to stop autoplay of the course about video just when editing the course in Studio, or is it to stop autoplay in the LMS? If it is LMS, doesn't the fact that autoplay=1 is appended in toggle_login_modal mean that this change is not relevant? |
Course about page, in the LMS. The JavaScript I linked does not append The This pretty much fixes a race condition that would occur when the JavaScript would take some time to load/run and the video would start playing in the background. |
@jzoldak I think an LMS review would be sufficient. They better understand the behavior they want to achieve. And a test would be on the LMS side. |
@jzoldak I'm the wrong person to ask about the LMS team these days. @dianakhuang? |
@nedbat I was actually tagging you from an opensource perspective. I'm not sure this is the way edx.org wants it to work, although it might be the way that people in the openedx community want it to? Or maybe I don't understand the change. |
I'm happy with the change in behavior. @dianakhuang can you take a quick look at the code (it's one line)? |
Looks good to me. 👍 |
…play Remove autoplay for video in course about page
Thanks @tusbar ! |
* feat: [AXM-24] Update structure for course enrollments API (#2515) * feat: [AXM-24] Update structure for course enrollments API * style: [AXM-24] Improve code style * fix: [AXM-24] Fix student's latest enrollment filter * feat: [AXM-47] Add course_status field to primary object (#2517) * feat: [AXM-40] add courses progress to enrollment endpoint (#2519) * fix: workaround for staticcollection introduced in e40a01c * feat: [AXM-40] add courses progress to enrollment endpoint * refactor: [AXM-40] add caching to improve performance * refactor: [AXM-40] add progress only for primary course * refactor: [AXM-40] refactor enrollment caching optimization --------- Co-authored-by: Glib Glugovskiy <[email protected]> * feat: [AXM-53] add assertions for primary course (#2522) * feat: [AXM-53] add assertions for primary course * test: [AXM-53] fix tests * style: [AXM-53] change future_assignment default value to None * refactor: [AXM-53] add some optimization for assignments collecting * feat: [AXM-200] Implement user's enrolments status API * style: [AXM-200] Improve code style * refactor: [AXM-200] Divide get method into smaller methods --------- Co-authored-by: NiedielnitsevIvan <[email protected]> Co-authored-by: Glib Glugovskiy <[email protected]>
The iframe was generated with
autoplay=1
, which, sometimes, would cause the video to play in the background.This is a quick fix, it won’t fix the existing courses as the
<iframe>
is stored in the database. To do so, you’ll have to edit the"about/video.html"
file manually.