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

wants to merge 1 commit into from

Conversation

igobranco
Copy link
Collaborator

Instead of loading the external video player,
it only loads the video player if the user clicks on the big ▶ icon.

It only works on the published page. On the edit mode it still show the old behavior.
This has performance benefits so the external video player JS libraries, like Youtube, only loads when the user wants to view the video.

The image that is uses is the video poster or the course cover.

image

@igobranco igobranco self-assigned this Dec 8, 2023
@igobranco
✨(video_player) lazy load embed video player
9ab3da6 
Instead of loading the external video player,
it only loads the video player if the user clicks on the
big ▶ icon.
@igobranco igobranco force-pushed the igobranco/lazy-load-video-player branch from fd1091b to 9ab3da6 Compare December 8, 2023 23:23
@jbpenrath jbpenrath self-requested a review December 11, 2023 15:11
jbpenrath
jbpenrath reviewed Dec 11, 2023
View reviewed changes
Copy link
Member

@jbpenrath jbpenrath left a comment •
edited

Choose a reason for hiding this comment

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

Currently, the user has to click twice to start the video that is weird but unfortunately autoplay has several restriction to be enable (add allow="autoplay" to the iframe and the video has to be muted by default...)

CleanShot.2023-12-11.at.23.47.18.mp4

src/richie/apps/core/templates/djangocms_video/default/video_player.html
Comment on lines +19 to +92
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>
</span>
</a>
</body>
</html>
"
{% endif %}
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/richie/apps/core/templates/djangocms_video/default/video_player.html
Comment on lines +48 to +86
<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>
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>

tests/apps/core/test_videoplayer.py
Comment on lines +19 to +20
The `course_teaser` placeholder should return an `iframe` with an embedded `srcdoc`
with a content of a link to the original external video service.
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.

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

@jbpenrath jbpenrath jbpenrath left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@igobranco igobranco

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@igobranco @jbpenrath