Merge pull request #579 from adzhindzhi/task/uepr-477-tips-library-return-focus-inconsistency-fix

[UEPR-477] Focus video on tutorial library choice with keyboard

Author: adzhindzhi

packages/scratch-gui/src/components/cards/cards.jsx

@@ -1,5 +1,5 @@
 import PropTypes from 'prop-types';
-import React, {Fragment, useRef, useEffect} from 'react';
+import React, {Fragment} from 'react';
 import classNames from 'classnames';
 import {FormattedMessage} from 'react-intl';
 import Draggable from 'react-draggable';
@@ -99,6 +99,20 @@ class VideoStep extends React.Component {
         script2.async = true;
         script2.setAttribute('id', 'wistia-video-api');
         document.body.appendChild(script2);
+
+        // The Wistia API doesn't provide a callback for when the video is ready,
+        // so we use the global _wq queue that Wistia provides for this purpose.
+        // See the below for more details.
+        // https://docs.wistia.com/docs/javascript-player-api#with-standard-embeds
+        window._wq = window._wq || [];
+        window._wq.push({
+            id: this.props.video,
+            onReady: video => {
+                if (video) {
+                    video.focus();
+                }
+            }});
+
     }
 
     // We use the Wistia API here to update or pause the video dynamically:
@@ -127,6 +141,10 @@ class VideoStep extends React.Component {
 
         const script2 = document.getElementById('wistia-video-api');
         script2.parentNode.removeChild(script2);
+
+        // Clean up the _wq queue to prevent old callbacks from firing
+        // if the component is unmounted before the video is ready
+        window._wq = window._wq.filter(video => video.id !== this.props.video);
     }
 
     render () {
@@ -354,21 +372,6 @@ const Cards = props => {
 
     if (activeDeckId === null) return;
 
-    const cardRef = useRef(null);
-
-    useEffect(() => {
-        // Only focus if there’s an actual tutorial step (image/video)
-        const steps = content[activeDeckId]?.steps;
-        const isTutorialStep = steps?.[step]?.video;
-
-        if (isTutorialStep && cardRef.current) {
-            // Defer focus to next animation frame for layout stability
-            requestAnimationFrame(() => {
-                cardRef.current.focus();
-            });
-        }
-    }, [activeDeckId, step, content]);
-
     // Tutorial cards need to calculate their own dragging bounds
     // to allow for dragging the cards off the left, right and bottom
     // edges of the workspace.
@@ -400,8 +403,6 @@ const Cards = props => {
                 top: `${menuBarHeight}px`,
                 left: `${-cardHorizontalDragOffset}px`
             }}
-            ref={cardRef}
-            tabIndex={-1}
         >
             <Draggable
                 bounds="parent"

packages/scratch-gui/test/unit/components/__snapshots__/cards.test.jsx.snap

@@ -3,7 +3,6 @@
 exports[`Cards component showVideos=false shows the title image/name instead of video step 1`] = `
 <div
   style="width: 1824px; height: 977px; top: 48px; left: -400px;"
-  tabindex="-1"
 >
   <div
     class="react-draggable"
@@ -52,7 +51,6 @@ exports[`Cards component showVideos=false shows the title image/name instead of
 exports[`Cards component showVideos=true shows the video step 1`] = `
 <div
   style="width: 1824px; height: 977px; top: 48px; left: -400px;"
-  tabindex="-1"
 >
   <div
     class="react-draggable"