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

Add playsInline attribute on the video element #7928

Merged
merged 6 commits into from
Jan 21, 2022

Conversation

Falke-Design
Copy link
Member

@Falke-Design Falke-Design commented Jan 19, 2022

The main PR was closed because we renamed the master branch to main.
For more infos look into: #7364

Fixes #7357

@Malvoz
Copy link
Member

Malvoz commented Jan 20, 2022

re #7364 (comment):

I'd rather not merge until it has been tested (and I don't have a iOS device either)

I think this should be good to merge. playsinline would simply allow videos to play without automatically going into fullscreen mode, in browsers where they otherwise would.

Without playsinline:
https://css-tricks.com/what-does-playsinline-mean-in-web-video/#:~:text=what%20it%20looks%20like%20without%20playsinline

With playsinline:
https://css-tricks.com/what-does-playsinline-mean-in-web-video/#:~:text=what%20it%20looks%20like%20with%20playsinline

@Falke-Design
Copy link
Member Author

Falke-Design commented Jan 20, 2022

We should setup a demo, so that we can easy share a link to check if it works. (I will share the dist js as a link in the next days).
We can set it up in leafletjs.com/edit.html and then I can upload it to my server, because on phones the plunk doesn't work.

@@ -78,6 +78,7 @@ export var VideoOverlay = ImageOverlay.extend({
vid.autoplay = !!this.options.autoplay;
vid.loop = !!this.options.loop;
vid.muted = !!this.options.muted;
vid.playsInline = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make this an option like all the other ones, there might be use-cases where someone would actually want to have this play in full-screen.

@Falke-Design
Copy link
Member Author

Here a link to test on a mobile device: http://falke-design.bplaced.net/leaflet/video/videooverlay.html

@Falke-Design Falke-Design requested a review from jonkoops January 20, 2022 19:43
Copy link
Collaborator

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

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

LGTM

@jonkoops
Copy link
Collaborator

@Falke-Design could you rebase this on main? I'd like to see if Bundlemon reports the file size increase properly.

@Falke-Design
Copy link
Member Author

@jonkoops does it work?

@jonkoops
Copy link
Collaborator

Strangely enough it reports a smaller size, which seems impossible. I'll wait for some more PRs so we can see is this is a persistent problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Video overlay: playsinline attribute required for phones on iOS
4 participants