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

[WIP] Added options to configure YouTube embedding #938

Closed
wants to merge 1 commit into from
Closed

[WIP] Added options to configure YouTube embedding #938

wants to merge 1 commit into from

Conversation

aschempp
Copy link
Member

@aschempp aschempp commented Jul 7, 2017

Probably needs a database migration too to set the default options on update.

@leofeyer
Copy link
Member

leofeyer commented Jul 7, 2017

Wouldn't it be much easier if you reversed the logic?

  • Hide suggested videos when the video finishes
  • Hide player controls
  • Hide video title and player actions

That would be backwards compatible and does not require any migration.

@leofeyer leofeyer added this to the 4.5.0 milestone Jul 7, 2017
@aschempp
Copy link
Member Author

aschempp commented Jul 7, 2017 via email

@fritzmg
Copy link
Contributor

fritzmg commented Jul 7, 2017 •
edited
Loading

In my extension (fritzmg/contao-youtube-iframe) I also designed the checkboxes this way so that if you check none of them, the default values of YouTube are active (i.e. no paramter will be passed to YouTube). Currently I support:

  • Show closed captions by default (cc_load_policy=1)
  • White progress bar (color=white)
  • Disallow fullscreen (fs=0)
  • Disable annotations by default (iv_load_policy=3)
  • Modest branding (modestbranding=1)
  • Disable related videos (rel=0)
  • Disable information (showinfo=0)
  • Start Time
  • End Time
  • Force page language (overrides the automatic language of the YouTube player with the page's language)

@leofeyer
Copy link
Member

leofeyer commented Jul 7, 2017

If @fritzmg has already made an extension, there is no reason to merge this PR, is there?

@fritzmg
Copy link
Contributor

fritzmg commented Jul 7, 2017

I think it would make sense to provide such functionality in the core.

@leofeyer
Copy link
Member

Ok, then @aschempp please update the PR and reverse the logic as discussed above.

@leofeyer leofeyer changed the title Added options to configure YouTube embedding [WIP] Added options to configure YouTube embedding Jul 11, 2017
@aschempp
Copy link
Member Author

I wonder if we should merge @fritzmg's features instead, apparently there are way more options than my PR provides?

@leofeyer leofeyer force-pushed the develop branch 5 times, most recently from ed0a7e5 to 3dadd44 Compare September 28, 2017 10:39
@leofeyer leofeyer force-pushed the develop branch 11 times, most recently from 0a13971 to 4c8802b Compare November 2, 2017 22:34
@leofeyer leofeyer force-pushed the develop branch 3 times, most recently from 495cb11 to 1358688 Compare November 5, 2017 16:21
@leofeyer leofeyer force-pushed the develop branch 3 times, most recently from da44f69 to 90840d3 Compare November 20, 2017 13:28
@leofeyer leofeyer assigned leofeyer and unassigned aschempp Nov 27, 2017
leofeyer added a commit that referenced this pull request Nov 27, 2017
@leofeyer
Add all the player options to the YouTube element (see #938).
c7b4f38
@leofeyer
Copy link
Member

Implemented in c7b4f38.

@leofeyer leofeyer closed this Nov 27, 2017
leofeyer added a commit that referenced this pull request Nov 27, 2017
@leofeyer
Add all the player options to the YouTube element (see #938).
877454b
@aschempp aschempp deleted the feature/youtube-options branch November 29, 2017 10:11
@baumannsven baumannsven mentioned this pull request May 30, 2019
Open
6 tasks
leofeyer added a commit that referenced this pull request Nov 27, 2019
@leofeyer
Correctly show custom templates in "override all" mode (see #1007)
ccd29e1 
Description
-----------

Fixes #938

Commits
-------

a15f7fd2 Correctly show custom templates in "override all" mode (see #938)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@leofeyer leofeyer

Labels
feature
Projects
None yet
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
3 participants
@aschempp @leofeyer @fritzmg