Skip to content

Commit

Permalink
fix(youtube-player): not picking up static startSeconds and endSeconds (
Browse files Browse the repository at this point in the history
angular#18214)

The way the `youtube-player` is set up is that the start and end seconds are observables which means that the consumer will miss out on any values that were emitted before they subscribed. These changes switch them to a `BehaviorSubject` which emits its last value upon subscription.

Also does some internal cleanup in the component.

Fixes angular#18212.
  • Loading branch information
crisbeto authored and yifange committed Jan 30, 2020
1 parent 569e0f0 commit df43365
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 43 deletions.
23 changes: 22 additions & 1 deletion src/youtube-player/youtube-player.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('YoutubePlayer', () => {

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

TestBed.compileComponents();
Expand Down Expand Up @@ -326,6 +326,17 @@ describe('YoutubePlayer', () => {
});
});

it('should pick up static startSeconds and endSeconds values', () => {
const staticSecondsApp = TestBed.createComponent(StaticStartEndSecondsApp);
staticSecondsApp.detectChanges();

playerSpy.getPlayerState.and.returnValue(window.YT!.PlayerState.CUED);
events.onReady({target: playerSpy});

expect(playerSpy.cueVideoById).toHaveBeenCalledWith(
jasmine.objectContaining({startSeconds: 42, endSeconds: 1337}));
});

});

/** Test component that contains a YouTubePlayer. */
Expand Down Expand Up @@ -359,3 +370,13 @@ class TestApp {
onApiChange = jasmine.createSpy('onApiChange');
@ViewChild('player') youtubePlayer: YouTubePlayer;
}


@Component({
template: `
<youtube-player [videoId]="videoId" [startSeconds]="42"[endSeconds]="1337"></youtube-player>
`
})
class StaticStartEndSecondsApp {
videoId = VIDEO_ID;
}
73 changes: 31 additions & 42 deletions src/youtube-player/youtube-player.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
pipe,
Subject,
of,
BehaviorSubject,
} from 'rxjs';

import {
Expand Down Expand Up @@ -91,51 +92,48 @@ type UninitializedPlayer = Pick<Player, 'videoId' | 'destroy' | 'addEventListene
export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
/** YouTube Video ID to view */
@Input()
get videoId(): string | undefined { return this._videoId; }
get videoId(): string | undefined { return this._videoId.value; }
set videoId(videoId: string | undefined) {
this._videoId = videoId;
this._videoIdObs.next(videoId);
this._videoId.next(videoId);
}
private _videoId: string | undefined;
private _videoIdObs = new Subject<string | undefined>();
private _videoId = new BehaviorSubject<string | undefined>(undefined);

/** Height of video player */
@Input()
get height(): number | undefined { return this._height; }
get height(): number | undefined { return this._height.value; }
set height(height: number | undefined) {
this._height = height || DEFAULT_PLAYER_HEIGHT;
this._heightObs.next(this._height);
this._height.next(height || DEFAULT_PLAYER_HEIGHT);
}
private _height = DEFAULT_PLAYER_HEIGHT;
private _heightObs = new Subject<number>();
private _height = new BehaviorSubject<number>(DEFAULT_PLAYER_HEIGHT);

/** Width of video player */
@Input()
get width(): number | undefined { return this._width; }
get width(): number | undefined { return this._width.value; }
set width(width: number | undefined) {
this._width = width || DEFAULT_PLAYER_WIDTH;
this._widthObs.next(this._width);
this._width.next(width || DEFAULT_PLAYER_WIDTH);
}
private _width = DEFAULT_PLAYER_WIDTH;
private _widthObs = new Subject<number>();
private _width = new BehaviorSubject<number>(DEFAULT_PLAYER_WIDTH);

/** The moment when the player is supposed to start playing */
@Input() set startSeconds(startSeconds: number | undefined) {
@Input()
set startSeconds(startSeconds: number | undefined) {
this._startSeconds.next(startSeconds);
}
private _startSeconds = new Subject<number | undefined>();
private _startSeconds = new BehaviorSubject<number | undefined>(undefined);

/** The moment when the player is supposed to stop playing */
@Input() set endSeconds(endSeconds: number | undefined) {
@Input()
set endSeconds(endSeconds: number | undefined) {
this._endSeconds.next(endSeconds);
}
private _endSeconds = new Subject<number | undefined>();
private _endSeconds = new BehaviorSubject<number | undefined>(undefined);

/** The suggested quality of the player */
@Input() set suggestedQuality(suggestedQuality: YT.SuggestedVideoQuality | undefined) {
@Input()
set suggestedQuality(suggestedQuality: YT.SuggestedVideoQuality | undefined) {
this._suggestedQuality.next(suggestedQuality);
}
private _suggestedQuality = new Subject<YT.SuggestedVideoQuality | undefined>();
private _suggestedQuality = new BehaviorSubject<YT.SuggestedVideoQuality | undefined>(undefined);

/**
* Whether the iframe will attempt to load regardless of the status of the api on the
Expand Down Expand Up @@ -202,40 +200,31 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
iframeApiAvailableObs = iframeApiAvailableSubject.pipe(take(1), startWith(false));
}

// Add initial values to all of the inputs.
const videoIdObs = this._videoIdObs.pipe(startWith(this._videoId));
const widthObs = this._widthObs.pipe(startWith(this._width));
const heightObs = this._heightObs.pipe(startWith(this._height));

const startSecondsObs = this._startSeconds.pipe(startWith(undefined));
const endSecondsObs = this._endSeconds.pipe(startWith(undefined));
const suggestedQualityObs = this._suggestedQuality.pipe(startWith(undefined));

// An observable of the currently loaded player.
const playerObs =
createPlayerObservable(
this._youtubeContainer,
videoIdObs,
this._videoId,
iframeApiAvailableObs,
widthObs,
heightObs,
this._width,
this._height,
this.createEventsBoundInZone(),
this._ngZone
).pipe(waitUntilReady(), takeUntil(this._destroyed), publish());

// Set up side effects to bind inputs to the player.
playerObs.subscribe(player => this._player = player);

bindSizeToPlayer(playerObs, widthObs, heightObs);
bindSizeToPlayer(playerObs, this._width, this._height);

bindSuggestedQualityToPlayer(playerObs, suggestedQualityObs);
bindSuggestedQualityToPlayer(playerObs, this._suggestedQuality);

bindCueVideoCall(
playerObs,
videoIdObs,
startSecondsObs,
endSecondsObs,
suggestedQualityObs,
this._videoId,
this._startSeconds,
this._endSeconds,
this._suggestedQuality,
this._destroyed);

// After all of the subscriptions are set up, connect the observable.
Expand Down Expand Up @@ -273,9 +262,9 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
window.onYouTubeIframeAPIReady = this._existingApiReadyCallback;
}

this._videoIdObs.complete();
this._heightObs.complete();
this._widthObs.complete();
this._videoId.complete();
this._height.complete();
this._width.complete();
this._startSeconds.complete();
this._endSeconds.complete();
this._suggestedQuality.complete();
Expand Down

0 comments on commit df43365

Please sign in to comment.