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

disable the audio tracks that were not clicked on. #3538

Closed

Conversation

snyderizer
Copy link
Contributor

Description

When an audio track menu item is clicked, the other audio tracks should be disabled. Potential fix for #3510.

Specific Changes proposed

In the AudioTrackMenuItem click handler, disable the tracks that were not clicked on, in addition to enabling the track that was clicked on.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

I did not see any existing unit tests for this class, did manual testing only.

(FWIW, contrib feature submit just printed "undefined" after I entered the description and didn't seem to create a pull request so I did this through Github. Apologies if this is not formatted as you desire.)

I did not see any existing unit tests for this class, did manual testing only.
@brandonocasey
Copy link
Contributor

Thanks for the PR! The format looks fine from what I can tell. I think that contrib may be broken at the moment so that is why you are experiencing issues with it. We are working on a replacement.

I think this looks good to me and through some of my own tests I can confirm that it works. LGTM

@misteroneill
Copy link
Member

Yeah, this LGTM.

And the code is cleaner, IMHO.

@gkatsev gkatsev added confirmed patch This PR can be added to a patch release. labels Aug 23, 2016
@gkatsev
Copy link
Member

gkatsev commented Aug 24, 2016

Merged to stable.

@gkatsev gkatsev closed this Aug 24, 2016
helenjer pushed a commit to helenjer/video.js that referenced this pull request Sep 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants