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

Added the ability to set options for child components directly on the parent options #1599

Closed
wants to merge 1 commit into from

Conversation

heff
Copy link
Member

@heff heff commented Oct 20, 2014

For example if you wanted to disable the mute toggle:

videojs(id, { controlBar: { muteToggle: false } });

Whereas before you had to do:

videojs(id, { children: { controlBar: { children: { muteToggle: false } } } });

@mmcc, ping

… parent options

For example if you wanted to disable the mute toggle:

    videojs(id, { controlBar: { muteToggle: false } });

Whereas before you had to do:

    videojs(id, { children: { controlBar: { children: { muteToggle: false } } } });
@gkatsev
Copy link
Member

gkatsev commented Oct 20, 2014

Finally!

@mmcc
Copy link
Member

mmcc commented Oct 20, 2014

Wow this is only a few lines more than when we were talking about the other day...looks great to me.

@heff
Copy link
Member Author

heff commented Oct 20, 2014

Ha, whatever Gary, "finally" :-P

Yeah it wasn't too crazy of a change.

I should have noted that this allows us to define children with an array, while still having the option to easily disable specific children later.

options = {
  children: ['myComponent', 'myOtherComponent'],
  myComponent: false
}

We couldn't do that before with the array method, but the array is safer for specific child ordering. I'm not sure how significant of a change it would be, but we should start moving over to using arrays for all components.

@heff heff closed this in f27bdd2 Oct 28, 2014
@heff
Copy link
Member Author

heff commented Oct 28, 2014

Merged this in, though it'd probably be good to look for any areas we can improve docs on this, if there are any

@heff heff added the confirmed label Oct 28, 2014
@mmcc
Copy link
Member

mmcc commented Oct 28, 2014

I mention this in that doc branch, so I’ll make this change before resubmitting.


Matthew McClure

On Tue, Oct 28, 2014 at 11:46 AM, Steve Heffernan
notifications@github.com wrote:

Merged this in, though it'd probably be good to look for any areas we can improve docs on this, if there are any

Reply to this email directly or view it on GitHub:
#1599 (comment)

jgubman added a commit to jgubman/video.js that referenced this pull request Oct 29, 2014
* 'stable' of https://github.com/videojs/video.js: (22 commits)
  Added line to changelog for 4.10.1
  Release 4.10.1
  Removed alert...
  Release 4.10.0
  @heff turned on the custom html controls for touch devices. closes videojs#1617
  updated options to include sans-children, and included self-hosted font blurb
  added CORS information to tracks guide. Fixes videojs#837
  doc updates + readme quick start
  updated options guide to include components
  @heff Added the ability to set options for child components directly in the parent options. closes videojs#1599
  @DevGavin added a Simplified Chinese translation. closes videojs#1593
  @mmcc fixed an issue with the VolumeButton assuming it was vertical by default. closes videojs#1592
  @heff enhanced the event listener API to allow for auto-cleanup of listeners on other componenets and elements. closes videojs#1588
  @mmcc fixed an issue where errors on source tags could get missed. closes videojs#1575
  Added doc for remaining time and removed onWaitEnd doc
  maybe actually check for keyLocation
  Update languages.md
  @heff updated the poster to use CSS styles to display; fixed the poster not showing if not originally set. closes videojs#1568
  Release 4.9.1
  Bumped to videojs-swf v4.5.1 to fix a data sanitization issue. closes videojs#1587
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants