From 99bf918c275a0d68f56a647b102ce099fbb7428e Mon Sep 17 00:00:00 2001 From: Kenny Date: Mon, 17 Apr 2023 12:11:42 +0200 Subject: [PATCH 01/15] bump version --- CHANGELOG.md | 2 +- package-lock.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 867d182ff..4cfd94a3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -849,7 +849,7 @@ Version 2.0 of the UI framework is built for player 7.1. If absolutely necessary ## 1.0.0 (2017-02-03) - First release -[3.47.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.46.0...develop +[3.47.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.46.0...v3.47.0 [3.46.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.45.0...v3.46.0 [3.45.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.44.0...v3.45.0 [3.44.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.43.0...v3.44.0 diff --git a/package-lock.json b/package-lock.json index 314e78660..687bce327 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "bitmovin-player-ui", - "version": "3.46.0", + "version": "3.47.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "bitmovin-player-ui", - "version": "3.46.0", + "version": "3.47.0", "license": "MIT", "devDependencies": { "@inrupt/jest-jsdom-polyfills": "^1.6.0", From c1c9eaa225a040ac287edb194cf3dc04a28de6ee Mon Sep 17 00:00:00 2001 From: Kenny Date: Mon, 17 Apr 2023 12:26:34 +0200 Subject: [PATCH 02/15] add development compare link --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cfd94a3b..28121cc18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -849,6 +849,7 @@ Version 2.0 of the UI framework is built for player 7.1. If absolutely necessary ## 1.0.0 (2017-02-03) - First release +[develop]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.47.0...develop [3.47.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.46.0...v3.47.0 [3.46.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.45.0...v3.46.0 [3.45.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.44.0...v3.45.0 From bfe68466b19ec01ee4c6a98048a904b280003715 Mon Sep 17 00:00:00 2001 From: Kenny Date: Tue, 27 Jun 2023 12:18:48 +0200 Subject: [PATCH 03/15] only handle actions on the active group --- src/ts/spatialnavigation/spatialnavigation.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ts/spatialnavigation/spatialnavigation.ts b/src/ts/spatialnavigation/spatialnavigation.ts index 6cc43a100..97022bf5b 100644 --- a/src/ts/spatialnavigation/spatialnavigation.ts +++ b/src/ts/spatialnavigation/spatialnavigation.ts @@ -121,14 +121,19 @@ export class SpatialNavigation { private handleKeyEvent = (e: KeyboardEvent): void => { const event: Direction | Action | undefined = this.keyMap[getKeyCode(e)]; + const active = this.getActiveNavigationGroup(); + if (!active || !active.container || active.container.isHidden() || active.container.isDisabled()) { + return; + } + if (isDirection(event)) { - this.getActiveNavigationGroup().handleNavigation(event); + active.handleNavigation(event); e.preventDefault(); e.stopPropagation(); } if (isAction(event)) { - this.getActiveNavigationGroup().handleAction(event); + active.handleAction(event); e.preventDefault(); e.stopPropagation(); From e3ed47ce98a66bb9da53e52d56b2e36c98290773 Mon Sep 17 00:00:00 2001 From: Kenny Date: Tue, 27 Jun 2023 14:04:03 +0200 Subject: [PATCH 04/15] hide all but the active ui --- src/ts/uimanager.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ts/uimanager.ts b/src/ts/uimanager.ts index bfcc0cad9..b51825e88 100644 --- a/src/ts/uimanager.ts +++ b/src/ts/uimanager.ts @@ -451,9 +451,12 @@ export class UIManager { // Select new UI variant // If no variant condition is fulfilled, we switch to *no* UI for (let uiVariant of this.uiVariants) { - if (uiVariant.condition == null || uiVariant.condition(switchingContext) === true) { + const matchesCondition = uiVariant.condition == null || uiVariant.condition(switchingContext) === true; + if (nextUiVariant == null && matchesCondition) { nextUiVariant = uiVariant; - break; + } else { + // hide all UIs besides the one which should be active + uiVariant.ui.hide(); } } From bb400443070ad6bb8460d0b21c74d1d969a96745 Mon Sep 17 00:00:00 2001 From: Kenny Date: Tue, 27 Jun 2023 14:04:23 +0200 Subject: [PATCH 05/15] hide before show to fix initial state --- src/ts/uimanager.ts | 47 ++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/ts/uimanager.ts b/src/ts/uimanager.ts index b51825e88..9eac6e00b 100644 --- a/src/ts/uimanager.ts +++ b/src/ts/uimanager.ts @@ -384,39 +384,38 @@ export class UIManager { let uiVariantIndex = this.uiVariants.indexOf(uiVariant); const nextUi: InternalUIInstanceManager = this.uiInstanceManagers[uiVariantIndex]; - let uiVariantChanged = false; - // Determine if the UI variant is changing - if (nextUi !== this.currentUi) { - uiVariantChanged = true; + // Only if the UI variant is changing, we need to do some stuff. Else we just leave everything as-is. + if (nextUi === this.currentUi) { + return; // console.log('switched from ', this.currentUi ? this.currentUi.getUI() : 'none', // ' to ', nextUi ? nextUi.getUI() : 'none'); } - // Only if the UI variant is changing, we need to do some stuff. Else we just leave everything as-is. - if (uiVariantChanged) { - // Hide the currently active UI variant - if (this.currentUi) { - this.currentUi.getUI().hide(); - } + // Hide the currently active UI variant + if (this.currentUi) { + this.currentUi.getUI().hide(); + } - // Assign the new UI variant as current UI - this.currentUi = nextUi; + // Assign the new UI variant as current UI + this.currentUi = nextUi; - // When we switch to a different UI instance, there's some additional stuff to manage. If we do not switch - // to an instance, we're done here. - if (this.currentUi != null) { - // Add the UI to the DOM (and configure it) the first time it is selected - if (!this.currentUi.isConfigured()) { - this.addUi(this.currentUi); - } - - if (onShow) { - onShow(); - } + // When we switch to a different UI instance, there's some additional stuff to manage. If we do not switch + // to an instance, we're done here. + if (this.currentUi != null) { + // Add the UI to the DOM (and configure it) the first time it is selected + if (!this.currentUi.isConfigured()) { + this.addUi(this.currentUi); + } + if (onShow) { + onShow(); + } - this.currentUi.getUI().show(); + if (!this.currentUi.getUI().isHidden()) { + console.debug('Hiding UI before showing it. Maybe the initial state is wrong'); + this.currentUi.getUI().hide(); } + this.currentUi.getUI().show(); } } From ac5ba3aad59efee44e690a9b59249203330e95c7 Mon Sep 17 00:00:00 2001 From: Kenny Date: Tue, 27 Jun 2023 14:16:52 +0200 Subject: [PATCH 06/15] show the container before triggering the keyEvent --- spec/spatialnavigation/spatialnavigation.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/spatialnavigation/spatialnavigation.spec.ts b/spec/spatialnavigation/spatialnavigation.spec.ts index b609776c7..7c377092d 100644 --- a/spec/spatialnavigation/spatialnavigation.spec.ts +++ b/spec/spatialnavigation/spatialnavigation.spec.ts @@ -68,6 +68,7 @@ describe('SpatialNavigation', () => { describe('handleKeyEvent', () => { it('should call handle navigation on active group on key event', () => { + rootNavigationContainer.show(); const rootHandleNavigationSpy = jest.spyOn(rootNavigationGroup, 'handleNavigation'); spatialNavigation['handleKeyEvent'](new KeyboardEvent('keydown', { key:'Up', keyCode: 38 } as any)); @@ -75,6 +76,7 @@ describe('SpatialNavigation', () => { }); it('should call handle action on active group on key event', () => { + rootNavigationContainer.show(); const rootHandleActionSpy = jest.spyOn(rootNavigationGroup, 'handleAction'); spatialNavigation['handleKeyEvent'](new KeyboardEvent('keydown', { key:'Escape', keyCode: 27 } as any)); From 22d8002b74f67d60cbd32a233fa83b66a9a86c72 Mon Sep 17 00:00:00 2001 From: Kenny Date: Wed, 28 Jun 2023 13:32:21 +0200 Subject: [PATCH 07/15] hide UIs which got unhidden by configure --- src/ts/uimanager.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ts/uimanager.ts b/src/ts/uimanager.ts index 9eac6e00b..26c3af52d 100644 --- a/src/ts/uimanager.ts +++ b/src/ts/uimanager.ts @@ -402,21 +402,21 @@ export class UIManager { // When we switch to a different UI instance, there's some additional stuff to manage. If we do not switch // to an instance, we're done here. - if (this.currentUi != null) { - // Add the UI to the DOM (and configure it) the first time it is selected - if (!this.currentUi.isConfigured()) { - this.addUi(this.currentUi); - } - if (onShow) { - onShow(); - } - + if (this.currentUi == null) { + return; + } + // Add the UI to the DOM (and configure it) the first time it is selected + if (!this.currentUi.isConfigured()) { + this.addUi(this.currentUi); + // ensure that the internal state is ready for the upcoming show call if (!this.currentUi.getUI().isHidden()) { - console.debug('Hiding UI before showing it. Maybe the initial state is wrong'); this.currentUi.getUI().hide(); } - this.currentUi.getUI().show(); } + if (onShow) { + onShow(); + } + this.currentUi.getUI().show(); } /** From bc7a4a5e24ee2e4f8ad07d69f84b8303a9035241 Mon Sep 17 00:00:00 2001 From: Kenny Date: Wed, 28 Jun 2023 13:32:30 +0200 Subject: [PATCH 08/15] fix type error --- spec/uimanager.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/uimanager.spec.ts b/spec/uimanager.spec.ts index a4736f313..612d35ee6 100644 --- a/spec/uimanager.spec.ts +++ b/spec/uimanager.spec.ts @@ -8,7 +8,7 @@ jest.mock('../src/ts/dom'); // This just simulates a Class that can be wrapped by our PlayerWrapper. // To enable this simple class structure we need a lot of any casts in the tests. class A { - private value: object = undefined; + private value?: object = undefined; get a() { return this.value; From a527920ac18828de34001574513818dd1ee3397b Mon Sep 17 00:00:00 2001 From: Kenny Date: Wed, 28 Jun 2023 14:09:40 +0200 Subject: [PATCH 09/15] test ui switching --- spec/uimanager.spec.ts | 50 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/spec/uimanager.spec.ts b/spec/uimanager.spec.ts index 612d35ee6..0064e31ff 100644 --- a/spec/uimanager.spec.ts +++ b/spec/uimanager.spec.ts @@ -1,7 +1,14 @@ -import { InternalUIConfig, PlayerWrapper, UIManager } from '../src/ts/uimanager'; +import { + InternalUIConfig, + PlayerWrapper, + UIManager, + UIVariant, +} from '../src/ts/uimanager'; import { PlayerAPI } from 'bitmovin-player'; import { MockHelper, TestingPlayerAPI } from './helper/MockHelper'; import { MobileV3PlayerEvent } from '../src/ts/mobilev3playerapi'; +import { UIContainer } from '../src/ts/components/uicontainer'; +import { Container } from '../src/ts/components/container'; jest.mock('../src/ts/dom'); @@ -66,6 +73,47 @@ describe('UIManager', () => { }); }); }); + + describe('switchToUiVariant', () => { + let firstUi: UIVariant, secondUI: UIVariant, defaultUI: UIVariant; + let playerMock: TestingPlayerAPI; + + beforeEach(() => { + playerMock = MockHelper.getPlayerMock(); + firstUi = { + ui: new UIContainer({ components: [new Container({})]}), + condition: (context) => context.isPlaying, + }; + secondUI = { + ui: new UIContainer({ components: [new Container({})]}), + condition: (context) => context.isAd, + } + defaultUI = { + ui: new UIContainer({ components: [new Container({})]}), + } + + }); + + it('should mark invisible UIs as hidden', () => { + new UIManager(playerMock, [firstUi, secondUI, defaultUI]); + + expect(firstUi.ui.isHidden()).toBeTruthy() + expect(secondUI.ui.isHidden()).toBeTruthy() + expect(defaultUI.ui.isHidden()).toBeFalsy() + }); + + it('should switch UIs when the player emits an event', () => { + new UIManager(playerMock, [firstUi, secondUI, defaultUI]); + + (playerMock.isPlaying as jest.Mock).mockReturnValue(true); + playerMock.eventEmitter.firePlayEvent(); + + expect(firstUi.ui.isHidden()).toBeFalsy() + expect(secondUI.ui.isHidden()).toBeTruthy() + expect(defaultUI.ui.isHidden()).toBeTruthy() + }); + }); + describe('mobile v3 handling', () => { let playerMock: TestingPlayerAPI; beforeEach(() => { From d4cd719b4545ee8cd06e364a151bd9f20f9925cc Mon Sep 17 00:00:00 2001 From: Kenny Date: Thu, 29 Jun 2023 11:37:05 +0200 Subject: [PATCH 10/15] make test name more specific --- spec/uimanager.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/uimanager.spec.ts b/spec/uimanager.spec.ts index 0064e31ff..5797db1ff 100644 --- a/spec/uimanager.spec.ts +++ b/spec/uimanager.spec.ts @@ -102,7 +102,7 @@ describe('UIManager', () => { expect(defaultUI.ui.isHidden()).toBeFalsy() }); - it('should switch UIs when the player emits an event', () => { + it('should switch to the corresponding ui when a play event is fired', () => { new UIManager(playerMock, [firstUi, secondUI, defaultUI]); (playerMock.isPlaying as jest.Mock).mockReturnValue(true); From 8769ba01a1059af920f0f2a21e3caceb6d9cd131 Mon Sep 17 00:00:00 2001 From: Kenny Date: Mon, 3 Jul 2023 08:43:09 +0200 Subject: [PATCH 11/15] add missing changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28121cc18..1ad0f107d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [develop] + +### Fixed +- When more than one UI with spatial navigation is managed by the UI manager, only the active one will handle key events. + ## [3.47.0] ### Changed From 57ea4b3a6a71a311ae1df8b4cfb00dbaf036167b Mon Sep 17 00:00:00 2001 From: Kenny Date: Mon, 3 Jul 2023 08:51:07 +0200 Subject: [PATCH 12/15] describe the problem, not the solution --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ad0f107d..0aa4ccf8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [develop] ### Fixed -- When more than one UI with spatial navigation is managed by the UI manager, only the active one will handle key events. +- When more than one UI with spatial navigation is managed by the UI manager, all UIs would handle key events, instead of only the active one. ## [3.47.0] From 5733cb82515ddb7e39492e1cbbbb96c72bdec142 Mon Sep 17 00:00:00 2001 From: Felix Hochgruber Date: Mon, 3 Jul 2023 09:05:23 +0200 Subject: [PATCH 13/15] Update package.json version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4adee85b8..394137320 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bitmovin-player-ui", - "version": "3.47.0", + "version": "3.48.0", "description": "Bitmovin Player UI Framework", "main": "./dist/js/framework/main.js", "types": "./dist/js/framework/main.d.ts", From 15c5ecfb0d4e47e3c01c6f839156a5c07ece0119 Mon Sep 17 00:00:00 2001 From: Felix Hochgruber Date: Mon, 3 Jul 2023 09:05:25 +0200 Subject: [PATCH 14/15] Update CHANGELOG.md version --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aa4ccf8d..9226279be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [develop] +## [3.48.0] ### Fixed - When more than one UI with spatial navigation is managed by the UI manager, all UIs would handle key events, instead of only the active one. @@ -854,7 +854,7 @@ Version 2.0 of the UI framework is built for player 7.1. If absolutely necessary ## 1.0.0 (2017-02-03) - First release -[develop]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.47.0...develop +[3.48.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.47.0...develop [3.47.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.46.0...v3.47.0 [3.46.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.45.0...v3.46.0 [3.45.0]: https://github.com/bitmovin/bitmovin-player-ui/compare/v3.44.0...v3.45.0 From 49b25e6572317aa5408816edc2cf045a1802455b Mon Sep 17 00:00:00 2001 From: Felix Hochgruber Date: Mon, 3 Jul 2023 09:22:53 +0200 Subject: [PATCH 15/15] Update CHANGELOG.md date --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9226279be..f234fb87a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [3.48.0] +## [3.48.0] - 2023-07-03 ### Fixed - When more than one UI with spatial navigation is managed by the UI manager, all UIs would handle key events, instead of only the active one.