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

Audio button #3223

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/css/components/_audio.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.video-js .vjs-audio-button {
@extend .vjs-icon-audio;
}
1 change: 1 addition & 0 deletions src/css/video-js.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
@import "components/chapters";
@import "components/descriptions";
@import "components/subtitles";
@import "components/audio";
@import "components/adaptive";
@import "components/captions-settings";
@import "components/modal-dialog";
64 changes: 64 additions & 0 deletions src/js/control-bar/audio-track-controls/audio-track-button.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* @file audio-track-button.js
*/
import TrackButton from '../track-button.js';
import Component from '../../component.js';
import * as Fn from '../../utils/fn.js';
import AudioTrackMenuItem from './audio-track-menu-item.js';

/**
* The base class for buttons that toggle specific text track types (e.g. subtitles)
*
* @param {Player|Object} player
* @param {Object=} options
* @extends TrackButton
* @class AudioTrackButton
*/
class AudioTrackButton extends TrackButton {
constructor(player, options = {}) {
options.tracks = player.audioTracks && player.audioTracks();
Copy link
Member

Choose a reason for hiding this comment

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

is this used anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

yes, it is, in the TrackButton, missed it.


super(player, options);

this.el_.setAttribute('aria-label', 'Audio Menu');
}

/**
* Allow sub components to stack CSS class names
*
* @return {String} The constructed class name
* @method buildCSSClass
*/
buildCSSClass() {
return `vjs-audio-button ${super.buildCSSClass()}`;
}

/**
* Create a menu item for each audio track
*
* @return {Array} Array of menu items
* @method createItems
*/
createItems(items = []) {
let tracks = this.player_.audioTracks && this.player_.audioTracks();
Copy link
Member

@gkatsev gkatsev Apr 29, 2016

Choose a reason for hiding this comment

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

why check that it exists? It should always be present.
[edit] "it" meaning this.player_.audioTracks

Copy link
Member

Choose a reason for hiding this comment

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

If you made a mock player to use with custom controls, this would fail, so, I guess we should keep it around for now, maybe remove it in a major bump.


if (!tracks) {
return items;
}

for (let i = 0; i < tracks.length; i++) {
let track = tracks[i];

items.push(new AudioTrackMenuItem(this.player_, {
// MenuItem is selectable
'selectable': true,
'track': track
Copy link
Member

Choose a reason for hiding this comment

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

This object literal could be simplified to:

{
  track,
  selectable: true
}

Copy link
Member

Choose a reason for hiding this comment

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

To elaborate, I feel like we should start writing code that will pass the video.js linter even if the project at large won't pass it.

}));
}

return items;
}
}

Component.registerComponent('AudioTrackButton', AudioTrackButton);
export default AudioTrackButton;
71 changes: 71 additions & 0 deletions src/js/control-bar/audio-track-controls/audio-track-menu-item.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* @file audio-track-menu-item.js
*/
import MenuItem from '../../menu/menu-item.js';
import Component from '../../component.js';
import * as Fn from '../../utils/fn.js';

/**
* The audio track menu item
*
* @param {Player|Object} player
* @param {Object=} options
* @extends MenuItem
* @class AudioTrackMenuItem
*/
class AudioTrackMenuItem extends MenuItem {
constructor(player, options) {
let track = options.track;
let tracks = player.audioTracks();

// Modify options for parent MenuItem class's init.
options.label = track.label || track.language || 'Unknown';
options.selected = track.enabled;
Copy link
Member

Choose a reason for hiding this comment

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

In AudioTrackButton it's passing selected not enabled so I wonder if this line is correct...

Copy link
Member

Choose a reason for hiding this comment

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

Or if the selected: true above is pointless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not following, AudioTrackButton passes selectable: true to populate the menu with clickable menu items. It also passes track which has an enabled property which is really the currently active track. If it is active we want to indicate that it is selected by setting AudioTrackMenuItem's selected option.

Copy link
Member

Choose a reason for hiding this comment

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

It's because I am apparently illiterate. I mis-read selectable as selected. Ignore me. 😄


super(player, options);

this.track = track;

if (tracks) {
let changeHandler = Fn.bind(this, this.handleTracksChange);

tracks.addEventListener('change', changeHandler);
this.on('dispose', () => {
tracks.removeEventListener('change', changeHandler);
});
}
}

/**
* Handle click on audio track
*
* @method handleClick
*/
handleClick(event) {
let tracks = this.player_.audioTracks();

super.handleClick(event);

if (!tracks) return;

for (let i = 0; i < tracks.length; i++) {
let track = tracks[i];

if (track === this.track) {
track.enabled = true;
}
}
}

/**
* Handle audio track change
*
* @method handleTracksChange
*/
handleTracksChange(event) {
this.selected(this.track.enabled);
}
}

Component.registerComponent('AudioTrackMenuItem', AudioTrackMenuItem);
export default AudioTrackMenuItem;
2 changes: 2 additions & 0 deletions src/js/control-bar/control-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import ChaptersButton from './text-track-controls/chapters-button.js';
import DescriptionsButton from './text-track-controls/descriptions-button.js';
import SubtitlesButton from './text-track-controls/subtitles-button.js';
import CaptionsButton from './text-track-controls/captions-button.js';
import AudioTrackButton from './audio-track-controls/audio-track-button.js';
import PlaybackRateMenuButton from './playback-rate-menu/playback-rate-menu-button.js';
import CustomControlSpacer from './spacer-controls/custom-control-spacer.js';

Expand Down Expand Up @@ -63,6 +64,7 @@ ControlBar.prototype.options_ = {
'descriptionsButton',
'subtitlesButton',
'captionsButton',
'audioTrackButton',
'fullscreenToggle'
]
};
Expand Down
34 changes: 11 additions & 23 deletions src/js/control-bar/text-track-controls/text-track-button.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* @file text-track-button.js
*/
import MenuButton from '../../menu/menu-button.js';
import TrackButton from '../track-button.js';
import Component from '../../component.js';
import * as Fn from '../../utils/fn.js';
import TextTrackMenuItem from './text-track-menu-item.js';
Expand All @@ -15,32 +15,20 @@ import OffTextTrackMenuItem from './off-text-track-menu-item.js';
* @extends MenuButton
* @class TextTrackButton
*/
class TextTrackButton extends MenuButton {
class TextTrackButton extends TrackButton {

constructor(player, options){
super(player, options);

let tracks = this.player_.textTracks();

if (this.items.length <= 1) {
this.hide();
}
constructor(player, options = {}){
options.tracks = player.textTracks();

if (!tracks) {
return;
}

let updateHandler = Fn.bind(this, this.update);
tracks.addEventListener('removetrack', updateHandler);
tracks.addEventListener('addtrack', updateHandler);

this.player_.on('dispose', function() {
tracks.removeEventListener('removetrack', updateHandler);
tracks.removeEventListener('addtrack', updateHandler);
});
super(player, options);
}

// Create a menu item for each text track
/**
* Create a menu item for each text track
*
* @return {Array} Array of menu items
* @method createItems
*/
createItems(items=[]) {
// Add an OFF menu item to turn all tracks off
items.push(new OffTextTrackMenuItem(this.player_, { 'kind': this.kind_ }));
Expand Down
44 changes: 44 additions & 0 deletions src/js/control-bar/track-button.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* @file track-button.js
*/
import MenuButton from '../menu/menu-button.js';
import Component from '../component.js';
import * as Fn from '../utils/fn.js';

/**
* The base class for buttons that toggle specific text track types (e.g. subtitles)
*
* @param {Player|Object} player
* @param {Object=} options
* @extends MenuButton
* @class TrackButton
*/
class TrackButton extends MenuButton {

constructor(player, options){
let tracks = options.tracks;

super(player, options);

if (this.items.length <= 1) {
this.hide();
}

if (!tracks) {
return;
}

let updateHandler = Fn.bind(this, this.update);
tracks.addEventListener('removetrack', updateHandler);
tracks.addEventListener('addtrack', updateHandler);

this.player_.on('dispose', function() {
tracks.removeEventListener('removetrack', updateHandler);
tracks.removeEventListener('addtrack', updateHandler);
});
}

}

Component.registerComponent('TrackButton', TrackButton);
export default TrackButton;