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.

Sign up for GitHub

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

Jump to bottom

✨(video_player) lazy load embed video player #2171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

✨(video_player) lazy load embed video player #2171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).

### Added

- Lazy load embed video player
- Add a footer on enrollment's item in the learner dashboard. It give the
possibility to purchase linked product or download linked certificate.
- Add download contracts pages on the teacher dashboard.
Expand Down
83 changes: 82 additions & 1 deletion src/richie/apps/core/templates/djangocms_video/default/video_player.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,95 @@
{% load i18n cms_tags %}
{% load i18n cms_tags extra_tags thumbnail static %}
{% comment %}
This is a copy of original template from plugin just to clean <iframe> from
obsolete attribute "frameborder" and invalid "allowfullscreen" attribute value.

For performance reasons instead of loading the video iframe directly,
it changes the default template with an iframe "srcdoc" that adds the
video poster or the course cover with a big play icon '▶'.
Only after the user clicks on the play icon '▶', the browser loads the external
video player iframe. It tries to autoplay the external video player.
{% endcomment %}

{% if instance.embed_link %}
{# show iframe if embed_link is provided #}
<div class="aspect-ratio">
<iframe
title="{% if instance.label %}{{ instance.label }}{% else %}{% trans "Video" %}{% endif %}"
{% if not request.toolbar.edit_mode_active %}
srcdoc="<html>
<head>
<link rel='stylesheet' type='text/css' href='{% static 'richie/css/main.css' %}'>
<style>*{padding:0;margin:0;overflow:hidden}html,body{height:100%}img{filter: brightness(.85)}img,span{position:absolute;width:100%;top:0;bottom:0;margin:auto;}span{text-align:center;font:48px/1.5 sans-serif;fill:white;display:flex;justify-content:center;align-items:center;}span svg{transition:.5s;}img:hover, span:hover svg{fill-opacity:1;filter: drop-shadow(3px 3px 30px rgb(0 0 0 / 0.65));}span svg{filter: drop-shadow(3px 3px 12px rgb(0 0 0 / 0.25));}</style>
</head>
<body>
<a href='{{ instance.embed_link_with_parameters }}{% if '?' not in instance.embed_link_with_parameters %}?{% endif %}&autoplay=1' title='{% trans 'View the presentation video' %}'>
{% if instance.poster %}
<img
src='{{ instance.poster.url }}'
{# alt forced to empty string for a11y because the image does not carry more information than the course title #}
alt='' />
{% else %}
{% placeholder_as_plugins "course_cover" as cover_plugins %}
{% blockplugin cover_plugins.0 %}
<img
src='{% thumbnail instance.picture 300x170 replace_alpha='#FFFFFF' crop upscale subject_location=instance.picture.subject_location %}'
srcset='
{% thumbnail instance.picture 300x170 replace_alpha='#FFFFFF' crop upscale subject_location=instance.picture.subject_location %} 300w
{% if instance.picture.width >= 600 %},{% thumbnail instance.picture 600x340 replace_alpha='#FFFFFF' crop upscale subject_location=instance.picture.subject_location %} 600w{% endif %}
{% if instance.picture.width >= 900 %},{% thumbnail instance.picture 900x510 replace_alpha='#FFFFFF' crop upscale subject_location=instance.picture.subject_location %} 900w{% endif %}
'
sizes='(max-width:62em) 100vw, 660px'
alt='{% if instance.picture.default_alt_text %}{{ instance.picture.default_alt_text }}{% else %}{% trans 'course cover image' %}{% endif %}'
/>
{% endblockplugin %}
{% endif %}

<span class='video-player-play-icon'>
<svg
width='85px'
height='85px'
version='1.1'
id='svg5'
xmlns='http://www.w3.org/2000/svg'
xmlns:svg='http://www.w3.org/2000/svg'>
<defs
id='defs2' />
<g
id='layer1'
transform='scale(3.5294116)'>
<g
id='g1018'
transform='matrix(1.1837053,0,0,1.1837053,-74.289058,-125.78024)'>
<g
id='g1600'
transform='translate(-1.6497001,-1.6496809)'>
<path
id='path860'
style='fill:currentColor;stroke-width:0.135807'
d='m 83.362493,117.43582 a 9.4266709,9.4266709 0 0 1 -9.42667,9.42668 9.4266709,9.4266709 0 0 1 -9.426671,-9.42668 9.4266709,9.4266709 0 0 1 9.426671,-9.42667 9.4266709,9.4266709 0 0 1 9.42667,9.42667 z' />
<g
id='g828'
transform='matrix(0.04654184,0,0,0.04654172,64.409458,107.90944)'
style='fill:#ffffff'>
<g
id='g826'
style='fill:#ffffff'>
<path
d='M 204.11,0 C 91.388,0 0,91.388 0,204.111 c 0,112.725 91.388,204.11 204.11,204.11 112.729,0 204.11,-91.385 204.11,-204.11 C 408.221,91.388 316.839,0 204.11,0 Z m 82.437,229.971 -126.368,72.471 c -17.003,9.75 -30.781,1.763 -30.781,-17.834 V 140.012 c 0,-19.602 13.777,-27.575 30.781,-17.827 l 126.368,72.466 c 17.004,9.752 17.004,25.566 0,35.32 z'
id='path824'
style='fill:#ffffff' />
</g>
</g>
</g>
</g>
</g>
</svg>
Comment on lines +48 to +86
Copy link
Member

Choose a reason for hiding this comment

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

Let's reduce this svg through https://jakearchibald.github.io/svgomg/

Suggested change
<svg
width='85px'
height='85px'
version='1.1'
id='svg5'
xmlns='http://www.w3.org/2000/svg'
xmlns:svg='http://www.w3.org/2000/svg'>
<defs
id='defs2' />
<g
id='layer1'
transform='scale(3.5294116)'>
<g
id='g1018'
transform='matrix(1.1837053,0,0,1.1837053,-74.289058,-125.78024)'>
<g
id='g1600'
transform='translate(-1.6497001,-1.6496809)'>
<path
id='path860'
style='fill:currentColor;stroke-width:0.135807'
d='m 83.362493,117.43582 a 9.4266709,9.4266709 0 0 1 -9.42667,9.42668 9.4266709,9.4266709 0 0 1 -9.426671,-9.42668 9.4266709,9.4266709 0 0 1 9.426671,-9.42667 9.4266709,9.4266709 0 0 1 9.42667,9.42667 z' />
<g
id='g828'
transform='matrix(0.04654184,0,0,0.04654172,64.409458,107.90944)'
style='fill:#ffffff'>
<g
id='g826'
style='fill:#ffffff'>
<path
d='M 204.11,0 C 91.388,0 0,91.388 0,204.111 c 0,112.725 91.388,204.11 204.11,204.11 112.729,0 204.11,-91.385 204.11,-204.11 C 408.221,91.388 316.839,0 204.11,0 Z m 82.437,229.971 -126.368,72.471 c -17.003,9.75 -30.781,1.763 -30.781,-17.834 V 140.012 c 0,-19.602 13.777,-27.575 30.781,-17.827 l 126.368,72.466 c 17.004,9.752 17.004,25.566 0,35.32 z'
id='path824'
style='fill:#ffffff' />
</g>
</g>
</g>
</g>
</g>
</svg>
<svg xmlns="http://www.w3.org/2000/svg" width="85" height="85"><path d="M83.362 117.436a9.427 9.427 0 0 1-9.426 9.426 9.427 9.427 0 0 1-9.427-9.426 9.427 9.427 0 0 1 9.427-9.427 9.427 9.427 0 0 1 9.426 9.427z" style="fill:currentColor;stroke-width:.135807" transform="matrix(4.17778 0 0 4.17778 -269.089 -450.822)"/><path d="M204.11 0C91.388 0 0 91.388 0 204.111c0 112.725 91.388 204.11 204.11 204.11 112.729 0 204.11-91.385 204.11-204.11C408.221 91.388 316.839 0 204.11 0Zm82.437 229.971-126.368 72.471c-17.003 9.75-30.781 1.763-30.781-17.834V140.012c0-19.602 13.777-27.575 30.781-17.827l126.368 72.466c17.004 9.752 17.004 25.566 0 35.32z" style="fill:#fff" transform="scale(.19444)"/></svg>

</span>
</a>
</body>
</html>
"
{% endif %}
Comment on lines +19 to +92
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to create dedicated html template for this srcdoc then include it as a string here ?

src="{{ instance.embed_link_with_parameters }}"
{{ instance.attributes_str }}
allowfullscreen
Expand Down
36 changes: 36 additions & 0 deletions tests/apps/core/test_videoplayer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"""
Test the custom video player with a performance improvement.
"""
import random

import lxml.html # nosec
from cms.test_utils.testcases import CMSTestCase

from richie.apps.courses.factories import VIDEO_SAMPLE_LINKS, CourseFactory


class CoursesTemplatesCourseDetailRenderingCMSTestCase(CMSTestCase):
"""
Test the custom video player with a performance improvement.
"""

def test_templates_course_detail_teaser_video_cover_empty(self):
"""
The `course_teaser` placeholder should return an `iframe` with an embedded `srcdoc`
with a content of a link to the original external video service.
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `course_teaser` placeholder should return an `iframe` with an embedded `srcdoc`
with a content of a link to the original external video service.
When the `course_teaser` placeholder is filled with a VideoPlayerPlugin,
it should return an `iframe` with an embedded `srcdoc` with
a content of a link to the original external video service.

"""
video_sample = random.choice(VIDEO_SAMPLE_LINKS) # nosec
course = CourseFactory(fill_teaser=video_sample, should_publish=True)

response = self.client.get(course.extended_object.get_absolute_url())
self.assertEqual(response.status_code, 200)
html = lxml.html.fromstring(response.content)
iframe = html.cssselect(".subheader__teaser .aspect-ratio iframe")[0]
self.assertIn("html,body", iframe.get("srcdoc"))
self.assertIn("allowfullscreen", iframe.keys())
self.assertEqual(iframe.get("title"), video_sample.label)
self.assertEqual(iframe.get("src"), video_sample.url)

iframe_srcdoc_html = lxml.html.fromstring(iframe.get("srcdoc"))
iframe_anchor = iframe_srcdoc_html.cssselect("a")[0]
self.assertEqual(iframe_anchor.get("href"), video_sample.url + "?&autoplay=1")