Skip to content

Commit

Permalink
fix(youtube-player): unable to bind to events after initialization (#…
Browse files Browse the repository at this point in the history
…18996)

The `youtube-player` component is set up so that it won't bind any events if there aren't any listeners at the time the player is created. This makes it impossible to bind an event by getting a reference to the player using `ViewChild` and subscribing to it. These changes fix the issue using a lazy observable which binds any events once the user subscribes and transfers them between players.
  • Loading branch information
crisbeto authored and jelbourn committed Apr 24, 2020
1 parent d26ba73 commit 0695e82
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 61 deletions.
1 change: 1 addition & 0 deletions src/youtube-player/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ ng_test_library(
deps = [
":youtube-player",
"@npm//@angular/platform-browser",
"@npm//rxjs",
],
)

Expand Down
29 changes: 19 additions & 10 deletions src/youtube-player/fake-youtube-player.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,38 @@ export function createFakeYtNamespace(): FakeYtNamespace {
]);

let playerConfig: YT.PlayerOptions | undefined;
const boundListeners = new Map<keyof YT.Events, Set<(event: any) => void>>();
const playerCtorSpy = jasmine.createSpy('Player Constructor');

playerCtorSpy.and.callFake((_el: Element, config: YT.PlayerOptions) => {
playerConfig = config;
return playerSpy;
});

const eventHandlerFactory = (name: keyof YT.Events) => {
playerSpy.addEventListener.and.callFake((name: keyof YT.Events, listener: (e: any) => any) => {
if (!boundListeners.has(name)) {
boundListeners.set(name, new Set());
}
boundListeners.get(name)!.add(listener);
});

playerSpy.removeEventListener.and.callFake((name: keyof YT.Events, listener: (e: any) => any) => {
if (boundListeners.has(name)) {
boundListeners.get(name)!.delete(listener);
}
});

function eventHandlerFactory(name: keyof YT.Events) {
return (arg: Object = {}) => {
if (!playerConfig) {
throw new Error(`Player not initialized before ${name} called`);
}

if (playerConfig && playerConfig.events && playerConfig.events[name]) {
playerConfig.events[name]!(arg as any);
}

for (const [event, callback] of playerSpy.addEventListener.calls.allArgs()) {
if (event === name) {
callback(arg as YT.PlayerEvent);
}
if (boundListeners.has(name)) {
boundListeners.get(name)!.forEach(callback => callback(arg));
}
};
};
}

const events: Required<YT.Events> = {
onReady: eventHandlerFactory('onReady'),
Expand Down
56 changes: 54 additions & 2 deletions src/youtube-player/youtube-player.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {Component, ViewChild} from '@angular/core';
import {YouTubePlayerModule} from './youtube-module';
import {YouTubePlayer, DEFAULT_PLAYER_WIDTH, DEFAULT_PLAYER_HEIGHT} from './youtube-player';
import {createFakeYtNamespace} from './fake-youtube-player';
import {Subscription} from 'rxjs';

const VIDEO_ID = 'a12345';

Expand All @@ -22,7 +23,7 @@ describe('YoutubePlayer', () => {

TestBed.configureTestingModule({
imports: [YouTubePlayerModule],
declarations: [TestApp, StaticStartEndSecondsApp],
declarations: [TestApp, StaticStartEndSecondsApp, NoEventsApp],
});

TestBed.compileComponents();
Expand Down Expand Up @@ -411,6 +412,48 @@ describe('YoutubePlayer', () => {
jasmine.objectContaining({startSeconds: 42, endSeconds: 1337}));
});

it('should be able to subscribe to events after initialization', () => {
const noEventsApp = TestBed.createComponent(NoEventsApp);
noEventsApp.detectChanges();
events.onReady({target: playerSpy});
noEventsApp.detectChanges();

const player = noEventsApp.componentInstance.player;
const subscriptions: Subscription[] = [];
const readySpy = jasmine.createSpy('ready spy');
const stateChangeSpy = jasmine.createSpy('stateChange spy');
const playbackQualityChangeSpy = jasmine.createSpy('playbackQualityChange spy');
const playbackRateChangeSpy = jasmine.createSpy('playbackRateChange spy');
const errorSpy = jasmine.createSpy('error spy');
const apiChangeSpy = jasmine.createSpy('apiChange spy');

subscriptions.push(player.ready.subscribe(readySpy));
events.onReady({target: playerSpy});
expect(readySpy).toHaveBeenCalledWith({target: playerSpy});

subscriptions.push(player.stateChange.subscribe(stateChangeSpy));
events.onStateChange({target: playerSpy, data: 5});
expect(stateChangeSpy).toHaveBeenCalledWith({target: playerSpy, data: 5});

subscriptions.push(player.playbackQualityChange.subscribe(playbackQualityChangeSpy));
events.onPlaybackQualityChange({target: playerSpy, data: 'large'});
expect(playbackQualityChangeSpy).toHaveBeenCalledWith({target: playerSpy, data: 'large'});

subscriptions.push(player.playbackRateChange.subscribe(playbackRateChangeSpy));
events.onPlaybackRateChange({target: playerSpy, data: 2});
expect(playbackRateChangeSpy).toHaveBeenCalledWith({target: playerSpy, data: 2});

subscriptions.push(player.error.subscribe(errorSpy));
events.onError({target: playerSpy, data: 5});
expect(errorSpy).toHaveBeenCalledWith({target: playerSpy, data: 5});

subscriptions.push(player.apiChange.subscribe(apiChangeSpy));
events.onApiChange({target: playerSpy});
expect(apiChangeSpy).toHaveBeenCalledWith({target: playerSpy});

subscriptions.forEach(subscription => subscription.unsubscribe());
});

});

/** Test component that contains a YouTubePlayer. */
Expand Down Expand Up @@ -448,9 +491,18 @@ class TestApp {

@Component({
template: `
<youtube-player [videoId]="videoId" [startSeconds]="42"[endSeconds]="1337"></youtube-player>
<youtube-player [videoId]="videoId" [startSeconds]="42" [endSeconds]="1337"></youtube-player>
`
})
class StaticStartEndSecondsApp {
videoId = VIDEO_ID;
}


@Component({
template: `<youtube-player [videoId]="videoId"></youtube-player>`
})
class NoEventsApp {
@ViewChild(YouTubePlayer) player: YouTubePlayer;
videoId = VIDEO_ID;
}
110 changes: 67 additions & 43 deletions src/youtube-player/youtube-player.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
ChangeDetectionStrategy,
Component,
ElementRef,
EventEmitter,
Input,
NgZone,
OnDestroy,
Expand All @@ -40,6 +39,7 @@ import {
Subject,
of,
BehaviorSubject,
fromEventPattern,
} from 'rxjs';

import {
Expand All @@ -55,6 +55,7 @@ import {
take,
takeUntil,
withLatestFrom,
switchMap,
} from 'rxjs/operators';

declare global {
Expand Down Expand Up @@ -102,6 +103,15 @@ interface PendingPlayerState {
template: '<div #youtubeContainer></div>',
})
export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
/** Whether we're currently rendering inside a browser. */
private _isBrowser: boolean;
private _youtubeContainer = new Subject<HTMLElement>();
private _destroyed = new Subject<void>();
private _player: Player | undefined;
private _existingApiReadyCallback: (() => void) | undefined;
private _pendingPlayerState: PendingPlayerState | undefined;
private _playerChanges = new BehaviorSubject<Player | undefined>(undefined);

/** YouTube Video ID to view */
@Input()
get videoId(): string | undefined { return this._videoId.value; }
Expand Down Expand Up @@ -155,25 +165,28 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
@Input() showBeforeIframeApiLoads: boolean | undefined;

/** Outputs are direct proxies from the player itself. */
@Output() ready = new EventEmitter<YT.PlayerEvent>();
@Output() stateChange = new EventEmitter<YT.OnStateChangeEvent>();
@Output() error = new EventEmitter<YT.OnErrorEvent>();
@Output() apiChange = new EventEmitter<YT.PlayerEvent>();
@Output() playbackQualityChange = new EventEmitter<YT.OnPlaybackQualityChangeEvent>();
@Output() playbackRateChange = new EventEmitter<YT.OnPlaybackRateChangeEvent>();
@Output() ready: Observable<YT.PlayerEvent> =
this._getLazyEmitter<YT.PlayerEvent>('onReady');

@Output() stateChange: Observable<YT.OnStateChangeEvent> =
this._getLazyEmitter<YT.OnStateChangeEvent>('onStateChange');

@Output() error: Observable<YT.OnErrorEvent> =
this._getLazyEmitter<YT.OnErrorEvent>('onError');

@Output() apiChange: Observable<YT.PlayerEvent> =
this._getLazyEmitter<YT.PlayerEvent>('onApiChange');

@Output() playbackQualityChange: Observable<YT.OnPlaybackQualityChangeEvent> =
this._getLazyEmitter<YT.OnPlaybackQualityChangeEvent>('onPlaybackQualityChange');

@Output() playbackRateChange: Observable<YT.OnPlaybackRateChangeEvent> =
this._getLazyEmitter<YT.OnPlaybackRateChangeEvent>('onPlaybackRateChange');

/** The element that will be replaced by the iframe. */
@ViewChild('youtubeContainer')
youtubeContainer: ElementRef<HTMLElement>;

/** Whether we're currently rendering inside a browser. */
private _isBrowser: boolean;
private _youtubeContainer = new Subject<HTMLElement>();
private _destroyed = new Subject<void>();
private _player: Player | undefined;
private _existingApiReadyCallback: (() => void) | undefined;
private _pendingPlayerState: PendingPlayerState | undefined;

constructor(
private _ngZone: NgZone,
/**
Expand Down Expand Up @@ -221,7 +234,6 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
iframeApiAvailableObs,
this._width,
this._height,
this.createEventsBoundInZone(),
this._ngZone
).pipe(waitUntilReady(player => {
// Destroy the player if loading was aborted so that we don't end up leaking memory.
Expand All @@ -233,6 +245,7 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
// Set up side effects to bind inputs to the player.
playerObs.subscribe(player => {
this._player = player;
this._playerChanges.next(player);

if (player && this._pendingPlayerState) {
this._initializePlayer(player, this._pendingPlayerState);
Expand All @@ -257,25 +270,12 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
(playerObs as ConnectableObservable<Player>).connect();
}

/**
* @deprecated No longer being used. To be removed.
* @breaking-change 11.0.0
*/
createEventsBoundInZone(): YT.Events {
const output: YT.Events = {};
const events = new Map<keyof YT.Events, EventEmitter<any>>([
['onReady', this.ready],
['onStateChange', this.stateChange],
['onPlaybackQualityChange', this.playbackQualityChange],
['onPlaybackRateChange', this.playbackRateChange],
['onError', this.error],
['onApiChange', this.apiChange]
]);

events.forEach((emitter, name) => {
// Since these events all trigger change detection, only bind them if something is subscribed.
if (emitter.observers.length) {
output[name] = this._runInZone(event => emitter.emit(event));
}
});

return output;
return {};
}

ngAfterViewInit() {
Expand All @@ -288,6 +288,7 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
window.onYouTubeIframeAPIReady = this._existingApiReadyCallback;
}

this._playerChanges.complete();
this._videoId.complete();
this._height.complete();
this._width.complete();
Expand All @@ -299,13 +300,6 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
this._destroyed.complete();
}

private _runInZone<T extends (...args: any[]) => void>(callback: T):
(...args: Parameters<T>) => void {
return (...args: Parameters<T>) => this._ngZone.run(() => callback(...args));
}

/** Proxied methods. */

/** See https://developers.google.com/youtube/iframe_api_reference#playVideo */
playVideo() {
if (this._player) {
Expand Down Expand Up @@ -518,6 +512,37 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
player.seekTo(seek.seconds, seek.allowSeekAhead);
}
}

/** Gets an observable that adds an event listener to the player when a user subscribes to it. */
private _getLazyEmitter<T extends YT.PlayerEvent>(name: keyof YT.Events): Observable<T> {
// Start with the stream of players. This way the events will be transferred
// over to the new player if it gets swapped out under-the-hood.
return this._playerChanges.pipe(
// Switch to the bound event. `switchMap` ensures that the old event is removed when the
// player is changed. If there's no player, return an observable that never emits.
switchMap(player => {
return player ? fromEventPattern<T>((listener: (event: T) => void) => {
player.addEventListener(name, listener);
}, (listener: (event: T) => void) => {
// The API seems to throw when we try to unbind from a destroyed player and it doesn't
// expose whether the player has been destroyed so we have to wrap it in a try/catch to
// prevent the entire stream from erroring out.
try {
player.removeEventListener(name, listener);
} catch {}
}) : observableOf<T>();
}),
// By default we run all the API interactions outside the zone
// so we have to bring the events back in manually when they emit.
(source: Observable<T>) => new Observable<T>(observer => source.subscribe({
next: value => this._ngZone.run(() => observer.next(value)),
error: error => observer.error(error),
complete: () => observer.complete()
})),
// Ensures that everything is cleared out on destroy.
takeUntil(this._destroyed)
);
}
}

/** Listens to changes to the given width and height and sets it on the player. */
Expand Down Expand Up @@ -593,15 +618,14 @@ function createPlayerObservable(
iframeApiAvailableObs: Observable<boolean>,
widthObs: Observable<number>,
heightObs: Observable<number>,
events: YT.Events,
ngZone: NgZone
): Observable<UninitializedPlayer | undefined> {

const playerOptions =
videoIdObs
.pipe(
withLatestFrom(combineLatest([widthObs, heightObs])),
map(([videoId, [width, height]]) => videoId ? ({videoId, width, height, events}) : undefined),
map(([videoId, [width, height]]) => videoId ? ({videoId, width, height}) : undefined),
);

return combineLatest([youtubeContainer, playerOptions, of(ngZone)])
Expand Down
12 changes: 6 additions & 6 deletions tools/public_api_guard/youtube-player/youtube-player.d.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
export declare class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
apiChange: EventEmitter<YT.PlayerEvent>;
apiChange: Observable<YT.PlayerEvent>;
set endSeconds(endSeconds: number | undefined);
error: EventEmitter<YT.OnErrorEvent>;
error: Observable<YT.OnErrorEvent>;
get height(): number | undefined;
set height(height: number | undefined);
playbackQualityChange: EventEmitter<YT.OnPlaybackQualityChangeEvent>;
playbackRateChange: EventEmitter<YT.OnPlaybackRateChangeEvent>;
ready: EventEmitter<YT.PlayerEvent>;
playbackQualityChange: Observable<YT.OnPlaybackQualityChangeEvent>;
playbackRateChange: Observable<YT.OnPlaybackRateChangeEvent>;
ready: Observable<YT.PlayerEvent>;
showBeforeIframeApiLoads: boolean | undefined;
set startSeconds(startSeconds: number | undefined);
stateChange: EventEmitter<YT.OnStateChangeEvent>;
stateChange: Observable<YT.OnStateChangeEvent>;
set suggestedQuality(suggestedQuality: YT.SuggestedVideoQuality | undefined);
get videoId(): string | undefined;
set videoId(videoId: string | undefined);
Expand Down

0 comments on commit 0695e82

Please sign in to comment.