From 01f1977b2caef4a9eb7f88180d8924716c9626b2 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:49:28 +0200 Subject: [PATCH 1/9] Migrated video to functional component --- VideoPlayer/Video.js | 472 ++++++++++++++++++++++--------------------- 1 file changed, 241 insertions(+), 231 deletions(-) diff --git a/VideoPlayer/Video.js b/VideoPlayer/Video.js index 362619d8c..476fee2c8 100644 --- a/VideoPlayer/Video.js +++ b/VideoPlayer/Video.js @@ -4,12 +4,43 @@ import {Media, getKeyFromSource} from '@enact/ui/Media'; import EnactPropTypes from '@enact/core/internal/prop-types'; import Slottable from '@enact/ui/Slottable'; import compose from 'ramda/src/compose'; -import {isValidElement, Component, Fragment} from 'react'; +import {isValidElement, useCallback, useLayoutEffect, useRef, useState} from 'react'; import css from './VideoPlayer.module.less'; import PropTypes from 'prop-types'; +const initialKeyState = { + keys: ['media-1', 'media-2'], + prevSourceKey: null, + prevPreloadKey: null +}; + +// Returns the source and preload keys for the current props. If either the source or preload +// existed previously in the other "slot", the keys are swapped so the preload video becomes the +// active video and vice versa. +const getKeys = (source, preloadSource, {keys, prevSourceKey, prevPreloadKey}) => { + const sourceKey = source && getKeyFromSource(source); + let preloadKey = preloadSource && getKeyFromSource(preloadSource); + + // If the same source is used for both, clear the preload key to avoid rendering duplicate + // video elements. + if (sourceKey === preloadKey) { + preloadKey = null; + } + + const swap = ( + (sourceKey === prevPreloadKey && prevPreloadKey) || + (preloadKey === prevSourceKey && prevSourceKey) + ); + + return { + keys: swap ? [keys[1], keys[0]] : keys, + prevSourceKey: sourceKey, + prevPreloadKey: preloadKey + }; +}; + /** * Adds support for preloading a video source for `VideoPlayer`. * @@ -18,266 +49,245 @@ import PropTypes from 'prop-types'; * @ui * @private */ -const VideoBase = class extends Component { - static displayName = 'Video'; - - static propTypes = /** @lends limestone/VideoPlayer.Video.prototype */ { - /** - * Video plays automatically. - * - * @type {Boolean} - * @default false - * @public - */ - autoPlay: PropTypes.bool, - - /** - * Video component to use. - * - * The default (`'video'`) renders an `HTMLVideoElement`. Custom video components must have - * a similar API structure, exposing the following APIs: - * - * Properties: - * * `currentTime` {Number} - Playback index of the media in seconds - * * `duration` {Number} - Media's entire duration in seconds - * * `error` {Boolean} - `true` if video playback has errored. - * * `loading` {Boolean} - `true` if video playback is loading. - * * `paused` {Boolean} - Playing vs paused state. `true` means the media is paused - * * `playbackRate` {Number} - Current playback rate, as a number - * * `proportionLoaded` {Number} - A value between `0` and `1` - * representing the proportion of the media that has loaded - * * `proportionPlayed` {Number} - A value between `0` and `1` representing the - * proportion of the media that has already been shown - * - * Events: - * * `onLoadStart` - Called when the video starts to load - * * `onPlay` - Sent when playback of the media starts after having been paused - * * `onUpdate` - Sent when any of the properties were updated - * - * Methods: - * * `play()` - play video - * * `pause()` - pause video - * * `load()` - load video - * - * The {@link limestone/VideoPlayer.Video.source|source} property is passed to - * the video component as a child node. - * - * @type {String|Component|Element} - * @default 'video' - * @public - */ - mediaComponent: EnactPropTypes.renderableOverride, - - /** - * The video source to be preloaded. Expects a `` node. - * - * @type {Node} - * @public - */ - preloadSource: PropTypes.node, - - /** - * Called with a reference to the active {@link ui/Media.Media|Media} component. - * - * @type {Function} - * @private - */ - setMedia: PropTypes.func, - - /** - * The video source to be played. - * - * Any children `` elements will be sent directly to the `mediaComponent` as video - * sources. - * - * See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source - * - * @type {String|Node} - * @public - */ - source: PropTypes.oneOfType([PropTypes.string, PropTypes.node]) - }; +const VideoBase = (props) => { + const { + mediaComponent = 'video', + preloadSource, + setMedia, + source, + ...rest + } = props; + + const videoRef = useRef(null); + const preloadVideoRef = useRef(null); + const preloadLoadStartRef = useRef(null); + const prevPropsRef = useRef(null); + + // Keys from the previous render. They replace the instance fields the class mutated in + // `getKeys()`. State is only updated during render when the sources change. + const [keyState, setKeyState] = useState(initialKeyState); + const nextKeyState = getKeys(source, preloadSource, keyState); + const keysChanged = ( + nextKeyState.prevSourceKey !== keyState.prevSourceKey || + nextKeyState.prevPreloadKey !== keyState.prevPreloadKey + ); + + if (keysChanged) { + // cache the previous keys so we know if the sources change the next time + setKeyState(nextKeyState); + } - static defaultProps = { - mediaComponent: 'video' - }; + const {keys, prevPreloadKey: currentPreloadKey} = keysChanged ? nextKeyState : keyState; + + // if preload is unset, clear the key so we don't render that media node at all + const [sourceKey, preloadKey] = currentPreloadKey ? keys : keys.slice(0, 1); + + const handlePreloadLoadStart = useCallback((ev) => { + // persist the event so we can cache it to re-emit when the preload becomes active + ev.persist(); + preloadLoadStartRef.current = ev; + + // prevent the from bubbling to upstream handlers + ev.stopPropagation(); + }, []); + + // Changing `setMedia` makes React call the previous ref callback with `null` (clearing the + // previous `setMedia`) and the new one with the node, as `componentDidUpdate` did. + const setVideoRef = useCallback((node) => { + videoRef.current = node; + + if (setMedia) { + setMedia(node); + } + }, [setMedia]); + + const setPreloadRef = useCallback((node) => { + if (node) { + node.load(); + } + preloadVideoRef.current = node; + }, []); + + // Equivalent of componentWillUnmount + useLayoutEffect(() => () => { + if (setMedia) { + setMedia(null); + } + }, [setMedia]); + + // Equivalent of componentDidUpdate. A layout effect keeps the class timing. + useLayoutEffect(() => { + const prevProps = prevPropsRef.current; + prevPropsRef.current = {source, preloadSource}; + + // componentDidUpdate does not run on mount + if (!prevProps) return; - componentDidUpdate (prevProps) { - const {source, preloadSource} = this.props; const {source: prevSource, preloadSource: prevPreloadSource} = prevProps; const key = getKeyFromSource(source); const prevKey = getKeyFromSource(prevSource); - const preloadKey = getKeyFromSource(preloadSource); + const preloadKeyFromSource = getKeyFromSource(preloadSource); const prevPreloadKey = getKeyFromSource(prevPreloadSource); - if (this.props.setMedia !== prevProps.setMedia) { - this.clearMedia(prevProps); - this.setMedia(); - } + const autoPlay = () => { + if (!props.autoPlay) return; + + const playPromise = videoRef.current.play(); + + if (playPromise) { + playPromise.then(() => { + // Autoplay started + }).catch(() => { + // Autoplay was prevented + }); + } + }; if (source) { - if (key === prevPreloadKey && preloadKey !== prevPreloadKey) { + if (key === prevPreloadKey && preloadKeyFromSource !== prevPreloadKey) { // if there's source and it was the preload source // if the preloaded video didn't error, notify VideoPlayer it is ready to reset - if (this.preloadLoadStart) { - forward('onLoadStart', this.preloadLoadStart, this.props); + if (preloadLoadStartRef.current) { + forward('onLoadStart', preloadLoadStartRef.current, props); } // emit onUpdate to give VideoPlayer an opportunity to updates its internal state // since it won't receive the onLoadStart or onError event - forward('onUpdate', {type: 'onUpdate'}, this.props); + forward('onUpdate', {type: 'onUpdate'}, props); - this.autoPlay(); + autoPlay(); } else if (key !== prevKey) { // if there's source and it has changed. - this.autoPlay(); + autoPlay(); } } - if (preloadSource && preloadKey !== prevPreloadKey) { - this.preloadLoadStart = null; + if (preloadSource && preloadKeyFromSource !== prevPreloadKey) { + preloadLoadStartRef.current = null; // In the case that the previous source equalled the previous preload (causing the // preload video node to not be created) and then the preload source was changed, we // need to guard against accessing the preloadVideo node. - if (this.preloadVideo) { - this.preloadVideo.load(); + if (preloadVideoRef.current) { + preloadVideoRef.current.load(); } } - } - - componentWillUnmount () { - this.clearMedia(); - } - - keys = ['media-1', 'media-2']; - prevSourceKey = null; - prevPreloadKey = null; - - handlePreloadLoadStart = (ev) => { - // persist the event so we can cache it to re-emit when the preload becomes active - ev.persist(); - this.preloadLoadStart = ev; - - // prevent the from bubbling to upstream handlers - ev.stopPropagation(); - }; - - clearMedia ({setMedia} = this.props) { - if (setMedia) { - setMedia(null); - } - } - - setMedia ({setMedia} = this.props) { - if (setMedia) { - setMedia(this.video); - } - } - - autoPlay () { - if (!this.props.autoPlay) return; - - const playPromise = this.video.play(); - - if (playPromise) { - playPromise.then(() => { - // Autoplay started - }).catch(() => { - // Autoplay was prevented - }); - } - } - - setVideoRef = (node) => { - this.video = node; - this.setMedia(); - }; - - setPreloadRef = (node) => { - if (node) { - node.load(); - } - this.preloadVideo = node; - }; - - getKeys () { - const {source, preloadSource} = this.props; - - const sourceKey = source && getKeyFromSource(source); - let preloadKey = preloadSource && getKeyFromSource(preloadSource); - - // If the same source is used for both, clear the preload key to avoid rendering duplicate - // video elements. - if (sourceKey === preloadKey) { - preloadKey = null; - } - - // if either the source or preload existed previously in the other "slot", swap the keys so - // the preload video becomes the active video and vice versa - if ( - (sourceKey === this.prevPreloadKey && this.prevPreloadKey) || - (preloadKey === this.prevSourceKey && this.prevSourceKey) - ) { - this.keys.reverse(); - } - - // cache the previous keys so we know if the sources change the next time - this.prevSourceKey = sourceKey; - this.prevPreloadKey = preloadKey; - - // if preload is unset, clear the key so we don't render that media node at all - return preloadKey ? this.keys : this.keys.slice(0, 1); - } + }); + + return ( + <> + {sourceKey ? ( + + )} + /> + ) : null} + {preloadKey ? ( + + )} + /> + ) : null} + + ); +}; - render () { - const { - preloadSource, - source, - mediaComponent, - ...rest - } = this.props; - - delete rest.setMedia; - - const [sourceKey, preloadKey] = this.getKeys(); - - return ( - - {sourceKey ? ( - - )} - /> - ) : null} - {preloadKey ? ( - - )} - /> - ) : null} - - ); - } +VideoBase.displayName = 'Video'; + +VideoBase.propTypes = /** @lends limestone/VideoPlayer.Video.prototype */ { + /** + * Video plays automatically. + * + * @type {Boolean} + * @default false + * @public + */ + autoPlay: PropTypes.bool, + + /** + * Video component to use. + * + * The default (`'video'`) renders an `HTMLVideoElement`. Custom video components must have + * a similar API structure, exposing the following APIs: + * + * Properties: + * * `currentTime` {Number} - Playback index of the media in seconds + * * `duration` {Number} - Media's entire duration in seconds + * * `error` {Boolean} - `true` if video playback has errored. + * * `loading` {Boolean} - `true` if video playback is loading. + * * `paused` {Boolean} - Playing vs paused state. `true` means the media is paused + * * `playbackRate` {Number} - Current playback rate, as a number + * * `proportionLoaded` {Number} - A value between `0` and `1` + * representing the proportion of the media that has loaded + * * `proportionPlayed` {Number} - A value between `0` and `1` representing the + * proportion of the media that has already been shown + * + * Events: + * * `onLoadStart` - Called when the video starts to load + * * `onPlay` - Sent when playback of the media starts after having been paused + * * `onUpdate` - Sent when any of the properties were updated + * + * Methods: + * * `play()` - play video + * * `pause()` - pause video + * * `load()` - load video + * + * The {@link limestone/VideoPlayer.Video.source|source} property is passed to + * the video component as a child node. + * + * @type {String|Component|Element} + * @default 'video' + * @public + */ + mediaComponent: EnactPropTypes.renderableOverride, + + /** + * The video source to be preloaded. Expects a `` node. + * + * @type {Node} + * @public + */ + preloadSource: PropTypes.node, + + /** + * Called with a reference to the active {@link ui/Media.Media|Media} component. + * + * @type {Function} + * @private + */ + setMedia: PropTypes.func, + + /** + * The video source to be played. + * + * Any children `` elements will be sent directly to the `mediaComponent` as video + * sources. + * + * See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source + * + * @type {String|Node} + * @public + */ + source: PropTypes.oneOfType([PropTypes.string, PropTypes.node]) }; const VideoDecorator = compose( From 21a9792d51efb1834f6965a1be62dd02a89435c6 Mon Sep 17 00:00:00 2001 From: alexandru-morariu-lgp <84004128+alexandrumorariu@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:25:50 +0200 Subject: [PATCH 2/9] Added unit tests. --- VideoPlayer/tests/Video-specs.js | 239 ++++++++++++++++++++++++++++++- 1 file changed, 238 insertions(+), 1 deletion(-) diff --git a/VideoPlayer/tests/Video-specs.js b/VideoPlayer/tests/Video-specs.js index 6b13b7473..99fdb0d60 100644 --- a/VideoPlayer/tests/Video-specs.js +++ b/VideoPlayer/tests/Video-specs.js @@ -1,7 +1,7 @@ /* global HTMLMediaElement */ import '@testing-library/jest-dom'; -import {render, screen} from '@testing-library/react'; +import {fireEvent, render, screen} from '@testing-library/react'; import Video from '../Video'; @@ -244,4 +244,241 @@ describe('VideoPlayer.Video', () => { expect(newPreload).toBe(source); }); }); + + describe('media reference', () => { + test('should call `setMedia` with the video node when mounted', () => { + const setMedia = jest.fn(); + + render( +