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 documentation to #2391 #2425

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented Aug 3, 2015

Rebased #2391 with documentation

gkatsev and others added 19 commits August 3, 2015 13:57
TextTrackList has a couple of event listeners that users rely on. By
emulated the TextTrackList we can hopefully eliminate some of the
friction that can occur during tech changes around the text track event
handlers.
When switching techs, we want to clear out the old text tracks and then
restore them using the currently appropriate tracks.
Add event listeners in the constructor. Remove event listeners in
dispose.
'removetrack' listener needs to be removed asynchronously to make sure
that we get all the events that are to be triggered.
Add comments to the text-track-list-converter functions. trackToJson was only used internally, so mark it with an underscore to indicate it's not necessarily a stable API.
@dmlap
Copy link
Member Author

dmlap commented Aug 3, 2015

See the #2391 for reviews

@gkatsev gkatsev closed this in 2dfd315 Aug 3, 2015
@dmlap dmlap deleted the review-gkatsev-always-emulated-text-track-list branch August 3, 2015 19:19
});

test('should have removed tracks on dispose', function(assert) {
let done = assert.async();
Copy link
Member

Choose a reason for hiding this comment

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

Getting "TypeError: assert.async is not a function". Master is broken for me.

@heff
Copy link
Member

heff commented Aug 3, 2015

Got the tests fixed by forcing qunit to update. npm wasn't doing it just with an npm install for some reason.

I'm seeing a lot more "ERROR:" messages logged now. Not positive it's this one specifically but we need to get a handle on those.

screen shot 2015-08-03 at 3 35 26 pm

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.

3 participants