Skip to content

Commit

Permalink
fix(FEV-1127): slide appear in child container when changing slides i…
Browse files Browse the repository at this point in the history
…n webcast application while producer set "hide slides" mode (#44)

lock state was not saved between cuepoints change and because of the ignore last handled mechanism the view was shown again after switching slide
  • Loading branch information
RoyBregman authored Nov 3, 2021
1 parent 853f2bd commit 914225e
Show file tree
Hide file tree
Showing 24 changed files with 113 additions and 117 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/semantic-pr-title.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: "Semantic PR title"
name: 'Semantic PR title'
on:
pull_request_target:
types:
Expand Down
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ before_install:
script: ./scripts/travis.sh

stages:
# - Tests
# - Tests
- Release canary
- Release
- Deploy
Expand All @@ -33,7 +33,7 @@ jobs:
include:
# https://docs.travis-ci.com/user/build-stages/deploy-github-releases/
- stage: Deploy
name: "Deploying a new version"
name: 'Deploying a new version'
if: tag IS present
env: TRAVIS_MODE=deploy
deploy:
Expand Down
22 changes: 7 additions & 15 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,25 @@

### Table of Contents

- [DualScreenConfigObject][1]
- [Parameters][2]

- [DualScreenConfigObject][1]
- [Parameters][2]

## DualScreenConfigObject

Defines the configuration of the dual screen. Except `childSizePercentage`, the configuration only defines the appearance when the player loads. After that, the user can adjust and change it.
Defines the configuration of the dual screen. Except `childSizePercentage`, the configuration only defines the appearance when the player loads. After that, the user can adjust and change it.

Type: [Object][4]

### Parameters

- `position` **(`"bottom-left"` \| `"bottom-right"` \| `"top-left"` \| `"top-right"`)** The position where the child player will be displayed (optional, default `"bottom-right"`)
- `layout` **(`"PIP"` \| `"SingleMedia"` \| `"SideBySide"`)** The layout of the parent and child players (optional, default `"PIP"`)
- `inverse` **[boolean][5]** When set to true, the Parent and Secondary players will swap positions (optional, default `false`)
- `childSizePercentage` **[number][6]** Relevant only for PIP layout - Sets the height of the child player as percentage of the parent player height (optional, default `30`)


- `position` **(`"bottom-left"` \| `"bottom-right"` \| `"top-left"` \| `"top-right"`)** The position where the child player will be displayed (optional, default `"bottom-right"`)
- `layout` **(`"PIP"` \| `"SingleMedia"` \| `"SideBySide"`)** The layout of the parent and child players (optional, default `"PIP"`)
- `inverse` **[boolean][5]** When set to true, the Parent and Secondary players will swap positions (optional, default `false`)
- `childSizePercentage` **[number][6]** Relevant only for PIP layout - Sets the height of the child player as percentage of the parent player height (optional, default `30`)

[1]: #dualscreenconfigobject

[2]: #parameters

[3]: #eventtype

[4]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object

[5]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean

[6]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number
2 changes: 1 addition & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ module.exports = function (config) {
}

config.set(karmaConf);
};
};
2 changes: 1 addition & 1 deletion src/components/button/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "./button";
export * from './button';
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
&.prePlayback {
bottom: -50px;
}
}
}
2 changes: 1 addition & 1 deletion src/components/drag-and-snap-manager/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "./drag-and-snap-manager";
export * from './drag-and-snap-manager';
2 changes: 1 addition & 1 deletion src/components/pip-minimized/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "./pip-minimized";
export * from './pip-minimized';
4 changes: 2 additions & 2 deletions src/components/pip/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export * from "./pip-child";
export * from "./pip-parent";
export * from './pip-child';
export * from './pip-parent';
2 changes: 1 addition & 1 deletion src/components/responsive-manager/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "./responsive-manager";
export * from './responsive-manager';
2 changes: 1 addition & 1 deletion src/components/side-by-side/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "./side-by-side";
export * from './side-by-side';
15 changes: 11 additions & 4 deletions src/dualscreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,13 @@ export class DualScreen extends KalturaPlayer.core.BasePlugin implements IEngine
}

loadMedia(): void {
const kalturaCuePointService: any = this._player.getService('kalturaCuepoints');
this._getSecondaryMedia();
this._getThumbs();
if (kalturaCuePointService) {
this._getThumbs(kalturaCuePointService);
} else {
this.logger.warn('kalturaCuepoints service is not registered');
}
}

reset(): void {
Expand Down Expand Up @@ -473,8 +478,7 @@ export class DualScreen extends KalturaPlayer.core.BasePlugin implements IEngine
}
};

private _getThumbs() {
const kalturaCuePointService: any = this._player.getService('kalturaCuepoints');
private _getThumbs(kalturaCuePointService: any) {
kalturaCuePointService?.registerTypes([kalturaCuePointService.CuepointType.SLIDE, kalturaCuePointService.CuepointType.VIEW_CHANGE]);
}

Expand Down Expand Up @@ -536,7 +540,10 @@ export class DualScreen extends KalturaPlayer.core.BasePlugin implements IEngine
this.logger.warn('Secondary entry id not found');
// subscribe on timed metadata events for image player
this._secondaryPlayerType = PlayerType.IMAGE;
this._imageSyncManager = new ImageSyncManager(this.eventManager, this.player, this._imagePlayer, this.logger, this._onSlideViewChanged);

if (this.player.getService('kalturaCuepoints')) {
this._imageSyncManager = new ImageSyncManager(this.eventManager, this.player, this._imagePlayer, this.logger, this._onSlideViewChanged);
}
this._resolveReadyPromise();
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/enums/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export * from "./animation";
export * from "./layoutEnum";
export * from "./positionEnum";
export * from "./reservedPresetAreasEnum";
export * from "./labels";
export * from "./players";
export * from './animation';
export * from './layoutEnum';
export * from './positionEnum';
export * from './reservedPresetAreasEnum';
export * from './labels';
export * from './players';
4 changes: 2 additions & 2 deletions src/image-player/image-player.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ export class ImagePlayer {
return this._activeImage;
}


public preLoadImages = () => {
if (
!this._preloadEnabled ||
(!this._images.length || this._preloadIndex === this._images.length - 1) || // No images or all images preloaded
!this._images.length ||
this._preloadIndex === this._images.length - 1 || // No images or all images preloaded
this._images[this._preloadIndex]?.loading // we are already in a pre load process
)
return;
Expand Down
14 changes: 8 additions & 6 deletions src/image-sync-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class ImageSyncManager {
_kalturaCuePointService: any;
_previouslyHandledViewChanges: Map<string, VTTCue> = new Map();
_firstPlaying: boolean = false;
_lock: boolean = false;

constructor(
eventManager: KalturaPlayerTypes.EventManager,
Expand Down Expand Up @@ -70,16 +71,17 @@ export class ImageSyncManager {
const activeSlide = activeCuePoints.find(cue => {
return cue.value?.data?.cuePointType === this._kalturaCuePointService.KalturaCuePointType.THUMB;
});

const viewChanges = activeCuePoints.filter(cue => {
return cue.value?.data?.cuePointType === this._kalturaCuePointService.KalturaCuePointType.CODE;
});
this._lock = !!viewChanges.find(viewChange => viewChange.value!.data?.partnerData?.viewModeLockState === 'locked');
// TODO: consider set single layout from view-change cue-points
this._imagePlayer.setActive(activeSlide ? activeSlide.value!.data.id : null);
this._imagePlayer.setActive(!this._lock && activeSlide ? activeSlide.value!.data.id : null);
if (activeSlide) {
const viewChanges = activeCuePoints.filter(cue => {
return cue.value?.data?.cuePointType === this._kalturaCuePointService.KalturaCuePointType.CODE;
});
const lock = viewChanges.find(viewChange => viewChange.value!.data?.partnerData?.viewModeLockState === 'locked');
viewChanges.forEach(viewChange => {
if (!this._previouslyHandledViewChanges.has(viewChange.id)) {
this._onSlideViewChanged(viewChange.value!.data.partnerData, !!lock);
this._onSlideViewChanged(viewChange.value!.data.partnerData, this._lock);
}
currHandledViewChanges.set(viewChange.id, viewChange);
});
Expand Down
2 changes: 1 addition & 1 deletion src/types/Gui.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export type GuiClientRect = {height: number; width: number; x: number; y: number; top: number; right: number, left: number};
export type GuiClientRect = {height: number; width: number; x: number; y: number; top: number; right: number; left: number};
2 changes: 1 addition & 1 deletion src/types/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from "./Gui";
export * from './Gui';
2 changes: 1 addition & 1 deletion test/setup/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ import prepareTestEnvironment from './prepare-test-environment';
import loadSpecs from './load-specs';

prepareTestEnvironment();
loadSpecs();
loadSpecs();
18 changes: 9 additions & 9 deletions test/setup/load-specs.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/**
* @returns {void}
*/
export function loadSpecs() {
const context = require.context('../src/', true, /\.spec\.js$/);
for (const key of context.keys()) {
describe(key, () => {
context(key);
});
}
export function loadSpecs() {
const context = require.context('../src/', true, /\.spec\.js$/);
for (const key of context.keys()) {
describe(key, () => {
context(key);
});
}

export default loadSpecs;
}

export default loadSpecs;
2 changes: 1 addition & 1 deletion test/setup/prepare-test-environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ export function prepareTestEnvironment() {
global.sinon = sinon;
}

export default prepareTestEnvironment;
export default prepareTestEnvironment;
34 changes: 15 additions & 19 deletions test/src/image-player/image-player.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,15 @@ describe('KDualscreenPlugin', function () {
});

it('should try and preload an two images', done => {
sandbox.stub(Image.prototype, "src").set(url=> {
setTimeout(()=>{
sandbox.stub(Image.prototype, 'src').set(url => {
setTimeout(() => {
this.onloadCallback();
if (imageItemInstance1.loaded && imageItemInstance2.loaded){
if (imageItemInstance1.loaded && imageItemInstance2.loaded) {
done();
}
} ,10);

}, 10);
});
sandbox.stub(Image.prototype, "onload").set(fn => {
sandbox.stub(Image.prototype, 'onload').set(fn => {
this.onloadCallback = fn;
});
const imagePlayer = new ImagePlayer(TestUtils.noop, true);
Expand All @@ -105,28 +104,25 @@ describe('KDualscreenPlugin', function () {
});

it('should try and preload an two images, but fail on the first image', done => {
sandbox.stub(Image.prototype, "src").set(url=> {
if (url === imageItemInstance1.imageUrl){
setTimeout(()=>{
sandbox.stub(Image.prototype, 'src').set(url => {
if (url === imageItemInstance1.imageUrl) {
setTimeout(() => {
this.onerrorCallback();
} ,10);
}
else{
setTimeout(()=>{
}, 10);
} else {
setTimeout(() => {
this.onloadCallback();
if (!imageItemInstance1.loaded && imageItemInstance2.loaded){
if (!imageItemInstance1.loaded && imageItemInstance2.loaded) {
done();
}
} ,10);
}, 10);
}


});
sandbox.stub(Image.prototype, "onload").set(fn => {
sandbox.stub(Image.prototype, 'onload').set(fn => {
this.onloadCallback = fn;
});

sandbox.stub(Image.prototype, "onerror").set(fn => {
sandbox.stub(Image.prototype, 'onerror').set(fn => {
this.onerrorCallback = fn;
});
const imagePlayer = new ImagePlayer(TestUtils.noop, true);
Expand Down
4 changes: 2 additions & 2 deletions test/src/image-sync-manager.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ describe('KDualscreenPlugin', function () {
it('should handle TIMED_METADATA_ADDED event', done => {
player.addEventListener(EventType.TIMED_METADATA_ADDED, ({payload}) => {
expect(payload.cues[0].value.key).to.eql(cuepoint.CUE_POINT_KEY);
expect(payload.cues[0].value.data.cuePointType).to.eql("thumbCuePoint.Thumb");
expect(payload.cues[0].value.data.cuePointType).to.eql('thumbCuePoint.Thumb');
done();
});
player.addEventListener(EventType.MEDIA_LOADED, () => {
player.cuePointManager.addCuePoints([kalturaCuePoint]);
player.cuePointManager.addCuePoints([kalturaCuePoint]);
});
player.play();
});
Expand Down
70 changes: 35 additions & 35 deletions test/src/utils/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,43 @@
* @param {string} opt_parentId - Optional parent id.
* @returns {HTMLDivElement}
*/
function createElement(type, id, opt_parentId) {
let element = document.createElement(type);
element.id = id;
if (!opt_parentId) {
document.body.appendChild(element);
function createElement(type, id, opt_parentId) {
let element = document.createElement(type);
element.id = id;
if (!opt_parentId) {
document.body.appendChild(element);
} else {
let parent = document.getElementById(opt_parentId);
if (parent) {
parent.appendChild(element);
} else {
let parent = document.getElementById(opt_parentId);
if (parent) {
parent.appendChild(element);
} else {
document.body.appendChild(element);
}
document.body.appendChild(element);
}
return element;
}

/**
* Removes a dom element.
* @param {string} id - The element id.
* @returns {void}
*/
function removeElement(id) {
let element = document.getElementById(id);
element.parentNode.removeChild(element);
}

/**
* Removes all the video elements that created by the test from the document.
* @returns {void}
*/
function removeVideoElementsFromTestPage() {
let element = document.getElementsByTagName('video');
for (let i = element.length - 1; i >= 0; i--) {
element[i].parentNode.removeChild(element[i]);
}
return element;
}

/**
* Removes a dom element.
* @param {string} id - The element id.
* @returns {void}
*/
function removeElement(id) {
let element = document.getElementById(id);
element.parentNode.removeChild(element);
}

/**
* Removes all the video elements that created by the test from the document.
* @returns {void}
*/
function removeVideoElementsFromTestPage() {
let element = document.getElementsByTagName('video');
for (let i = element.length - 1; i >= 0; i--) {
element[i].parentNode.removeChild(element[i]);
}
}

const noop = () => {};

const noop = () => {};

export {removeVideoElementsFromTestPage, createElement, removeElement, noop};
export {removeVideoElementsFromTestPage, createElement, removeElement, noop};
Loading

0 comments on commit 914225e

Please sign in to comment.