From b874fe3b01a51310368e4c9199806f0a343313d2 Mon Sep 17 00:00:00 2001 From: Mattias Buelens <649348+MattiasBuelens@users.noreply.github.com> Date: Wed, 14 Feb 2024 11:16:54 +0100 Subject: [PATCH] Fix `closeMenu()` when not yet fully upgraded (#49) --- CHANGELOG.md | 4 ++++ src/UIContainer.ts | 14 ++++++++++---- src/components/LanguageMenu.ts | 4 +++- src/components/MenuGroup.ts | 19 +++++++++---------- src/components/PlaybackRateMenu.ts | 2 ++ src/components/RadioGroup.ts | 19 +++++++++---------- src/components/SettingsMenu.ts | 2 ++ src/components/TextTrackStyleMenu.ts | 2 ++ src/components/TimeRange.ts | 6 ++++-- src/util/CommonUtils.ts | 12 ++++++++++++ 10 files changed, 57 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 557116df..60d09167 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ > - 🏠 Internal > - 💅 Polish +## Unreleased + +- 🐛 Fixed an issue where `` could throw an error when the player changes sources before all custom elements are properly registered. ([#49](https://github.com/THEOplayer/web-ui/pull/49)) + ## v1.6.0 (2024-02-08) - 🚀 Introducing [Open Video UI for React](https://www.npmjs.com/package/@theoplayer/react-ui). ([#48](https://github.com/THEOplayer/web-ui/pull/48)) diff --git a/src/UIContainer.ts b/src/UIContainer.ts index d4648576..76716825 100644 --- a/src/UIContainer.ts +++ b/src/UIContainer.ts @@ -22,21 +22,22 @@ import { fullscreenAPI } from './util/FullscreenUtils'; import { Attribute } from './util/Attribute'; import { isMobile, isTv } from './util/Environment'; import { Rectangle } from './util/GeometryUtils'; -import './components/GestureReceiver'; import { PREVIEW_TIME_CHANGE_EVENT, type PreviewTimeChangeEvent } from './events/PreviewTimeChangeEvent'; import type { StreamType } from './util/StreamType'; import type { StreamTypeChangeEvent } from './events/StreamTypeChangeEvent'; import { STREAM_TYPE_CHANGE_EVENT } from './events/StreamTypeChangeEvent'; import { createCustomEvent } from './util/EventUtils'; import { getTargetQualities } from './util/TrackUtils'; -import type { MenuGroup } from './components'; -import './components/MenuGroup'; +import { MenuGroup } from './components/MenuGroup'; import { MENU_CHANGE_EVENT } from './events/MenuChangeEvent'; import type { DeviceType } from './util/DeviceType'; import { getFocusedChild, navigateByArrowKey } from './util/KeyboardNavigation'; import { isArrowKey, isBackKey, KeyCode } from './util/KeyCode'; import { READY_EVENT } from './events/ReadyEvent'; +// Load components used in template +import './components/GestureReceiver'; + const template = document.createElement('template'); template.innerHTML = `${elementHtml}`; shadyCss.prepareTemplate(template, 'theoplayer-ui'); @@ -378,6 +379,10 @@ export class UIContainer extends HTMLElement { connectedCallback(): void { shadyCss.styleElement(this); + if (!(this._menuGroup instanceof MenuGroup)) { + customElements.upgrade(this._menuGroup); + } + if (!this.hasAttribute(Attribute.DEVICE_TYPE)) { const deviceType: DeviceType = isMobile() ? 'mobile' : isTv() ? 'tv' : 'desktop'; this.setAttribute(Attribute.DEVICE_TYPE, deviceType); @@ -661,7 +666,8 @@ export class UIContainer extends HTMLElement { } private closeMenu_(): void { - this._menuGroup.closeMenu(); + // Menu group might not be upgraded yet + this._menuGroup.closeMenu?.(); this._menuOpener?.focus(); this._menuOpener = undefined; } diff --git a/src/components/LanguageMenu.ts b/src/components/LanguageMenu.ts index cae5f665..e9a327ed 100644 --- a/src/components/LanguageMenu.ts +++ b/src/components/LanguageMenu.ts @@ -6,9 +6,11 @@ import { StateReceiverMixin } from './StateReceiverMixin'; import type { ChromelessPlayer, MediaTrack, TextTrack } from 'theoplayer/chromeless'; import { isSubtitleTrack } from '../util/TrackUtils'; import { Attribute } from '../util/Attribute'; +import { toggleAttribute } from '../util/CommonUtils'; + +// Load components used in template import './TrackRadioGroup'; import './TextTrackStyleMenu'; -import { toggleAttribute } from '../util/CommonUtils'; const template = document.createElement('template'); template.innerHTML = menuGroupTemplate(languageMenuHtml, languageMenuCss); diff --git a/src/components/MenuGroup.ts b/src/components/MenuGroup.ts index 4af149ef..03fa2a67 100644 --- a/src/components/MenuGroup.ts +++ b/src/components/MenuGroup.ts @@ -1,10 +1,10 @@ import * as shadyCss from '@webcomponents/shadycss'; import menuGroupCss from './MenuGroup.css'; import { Attribute } from '../util/Attribute'; -import { arrayFind, arrayFindIndex, fromArrayLike, isElement, isHTMLElement, noOp } from '../util/CommonUtils'; +import { arrayFind, arrayFindIndex, fromArrayLike, isElement, isHTMLElement, noOp, upgradeCustomElementIfNeeded } from '../util/CommonUtils'; import { CLOSE_MENU_EVENT, type CloseMenuEvent } from '../events/CloseMenuEvent'; import { TOGGLE_MENU_EVENT, type ToggleMenuEvent } from '../events/ToggleMenuEvent'; -import { isBackKey, KeyCode } from '../util/KeyCode'; +import { isBackKey } from '../util/KeyCode'; import { createCustomEvent } from '../util/EventUtils'; import type { MenuChangeEvent } from '../events/MenuChangeEvent'; import { MENU_CHANGE_EVENT } from '../events/MenuChangeEvent'; @@ -260,19 +260,18 @@ export class MenuGroup extends HTMLElement { ...fromArrayLike(this.shadowRoot!.children), ...(this._menuSlot ? this._menuSlot.assignedNodes({ flatten: true }).filter(isElement) : []) ]; + const upgradePromises: Array> = []; for (const child of children) { if (!isMenuElement(child)) { - // Upgrade custom elements if needed - const childName = child.nodeName.toLowerCase(); - if (childName.indexOf('-') >= 0) { - if (customElements.get(childName)) { - customElements.upgrade(child); - } else { - customElements.whenDefined(childName).then(this._onMenuListChange, noOp); - } + const promise = upgradeCustomElementIfNeeded(child); + if (promise) { + upgradePromises.push(promise); } } } + if (upgradePromises.length > 0) { + Promise.all(upgradePromises).then(this._onMenuListChange, noOp); + } const newMenus = children.filter(isMenuElement); // Close all removed menus for (const oldMenu of this._menus) { diff --git a/src/components/PlaybackRateMenu.ts b/src/components/PlaybackRateMenu.ts index 6ed1415f..14502121 100644 --- a/src/components/PlaybackRateMenu.ts +++ b/src/components/PlaybackRateMenu.ts @@ -1,6 +1,8 @@ import * as shadyCss from '@webcomponents/shadycss'; import { Menu, menuTemplate } from './Menu'; import playbackRateMenuHtml from './PlaybackRateMenu.html'; + +// Load components used in template import './PlaybackRateRadioGroup'; const template = document.createElement('template'); diff --git a/src/components/RadioGroup.ts b/src/components/RadioGroup.ts index 58d3f1e9..cc4585fb 100644 --- a/src/components/RadioGroup.ts +++ b/src/components/RadioGroup.ts @@ -2,8 +2,7 @@ import * as shadyCss from '@webcomponents/shadycss'; import { isArrowKey, KeyCode } from '../util/KeyCode'; import { RadioButton } from './RadioButton'; import { createEvent } from '../util/EventUtils'; -import { arrayFind, isElement, noOp } from '../util/CommonUtils'; -import './RadioButton'; +import { arrayFind, isElement, noOp, upgradeCustomElementIfNeeded } from '../util/CommonUtils'; import { StateReceiverMixin } from './StateReceiverMixin'; import { Attribute } from '../util/Attribute'; import type { DeviceType } from '../util/DeviceType'; @@ -78,19 +77,19 @@ export class RadioGroup extends StateReceiverMixin(HTMLElement, ['deviceType']) private readonly _onSlotChange = () => { const children = this._slot.assignedNodes({ flatten: true }).filter(isElement); + const upgradePromises: Array> = []; for (const child of children) { if (!isRadioButton(child)) { - // Upgrade custom elements if needed - const childName = child.nodeName.toLowerCase(); - if (childName.indexOf('-') >= 0) { - if (customElements.get(childName)) { - customElements.upgrade(child); - } else { - customElements.whenDefined(childName).then(this._onSlotChange, noOp); - } + const promise = upgradeCustomElementIfNeeded(child); + if (promise) { + upgradePromises.push(promise); } } } + if (upgradePromises.length > 0) { + Promise.all(upgradePromises).then(this._onSlotChange, noOp); + } + this._radioButtons = children.filter(isRadioButton); let firstFocusedButton = this.focusedRadioButton; diff --git a/src/components/SettingsMenu.ts b/src/components/SettingsMenu.ts index 2812255b..092c8bef 100644 --- a/src/components/SettingsMenu.ts +++ b/src/components/SettingsMenu.ts @@ -2,6 +2,8 @@ import * as shadyCss from '@webcomponents/shadycss'; import { MenuGroup, menuGroupTemplate } from './MenuGroup'; import settingsMenuHtml from './SettingsMenu.html'; import menuTableCss from './MenuTable.css'; + +// Load components used in template import './ActiveQualityDisplay'; import './PlaybackRateDisplay'; import './PlaybackRateMenu'; diff --git a/src/components/TextTrackStyleMenu.ts b/src/components/TextTrackStyleMenu.ts index 1be07b9b..73f13818 100644 --- a/src/components/TextTrackStyleMenu.ts +++ b/src/components/TextTrackStyleMenu.ts @@ -3,6 +3,8 @@ import { MenuGroup, menuGroupTemplate } from './MenuGroup'; import textTrackStyleMenuHtml from './TextTrackStyleMenu.html'; import textTrackStyleMenuCss from './TextTrackStyleMenu.css'; import menuTableCss from './MenuTable.css'; + +// Load components used in template import './TextTrackStyleDisplay'; import './TextTrackStyleRadioGroup'; diff --git a/src/components/TimeRange.ts b/src/components/TimeRange.ts index 43b6234b..e8da2ed2 100644 --- a/src/components/TimeRange.ts +++ b/src/components/TimeRange.ts @@ -10,12 +10,14 @@ import type { PreviewTimeChangeEvent } from '../events/PreviewTimeChangeEvent'; import { PREVIEW_TIME_CHANGE_EVENT } from '../events/PreviewTimeChangeEvent'; import { Attribute } from '../util/Attribute'; import type { StreamType } from '../util/StreamType'; -import './PreviewThumbnail'; -import './PreviewTimeDisplay'; import { isLinearAd } from '../util/AdUtils'; import type { ColorStops } from '../util/ColorStops'; import { KeyCode } from '../util/KeyCode'; +// Load components used in template +import './PreviewThumbnail'; +import './PreviewTimeDisplay'; + const template = document.createElement('template'); template.innerHTML = rangeTemplate(timeRangeHtml, timeRangeCss); shadyCss.prepareTemplate(template, 'theoplayer-time-range'); diff --git a/src/util/CommonUtils.ts b/src/util/CommonUtils.ts index 10835926..eba52858 100644 --- a/src/util/CommonUtils.ts +++ b/src/util/CommonUtils.ts @@ -231,3 +231,15 @@ export function getActiveElement(): Element | null { } return activeElement; } + +export function upgradeCustomElementIfNeeded(element: Element): Promise | undefined { + const elementName = element.nodeName.toLowerCase(); + if (elementName.indexOf('-') >= 0) { + if (customElements.get(elementName)) { + customElements.upgrade(element); + } else { + return customElements.whenDefined(elementName); + } + } + return undefined; +}