From cacb33e600653b136fd4b4a376ca51fb358045e0 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Mon, 2 Feb 2026 15:22:16 +0200 Subject: [PATCH 01/32] chore: rewrote action menu and added hook to handle navigation --- .../components/action-menu/action-menu.jsx | 310 ++++++++++-------- .../stage-selector/stage-selector.jsx | 1 - .../src/hooks/use-action-menu-navigation.js | 85 +++++ 3 files changed, 253 insertions(+), 143 deletions(-) create mode 100644 packages/scratch-gui/src/hooks/use-action-menu-navigation.js diff --git a/packages/scratch-gui/src/components/action-menu/action-menu.jsx b/packages/scratch-gui/src/components/action-menu/action-menu.jsx index a09fd906819..8453af4b078 100644 --- a/packages/scratch-gui/src/components/action-menu/action-menu.jsx +++ b/packages/scratch-gui/src/components/action-menu/action-menu.jsx @@ -1,154 +1,175 @@ +import React, {useState, useRef, useEffect, useCallback} from 'react'; import PropTypes from 'prop-types'; -import React from 'react'; import classNames from 'classnames'; -import bindAll from 'lodash.bindall'; import ReactTooltip from 'react-tooltip'; - import styles from './action-menu.css'; +import useActionMenuNavigation from '../../hooks/use-action-menu-navigation'; const CLOSE_DELAY = 300; // ms -class ActionMenu extends React.Component { - constructor (props) { - super(props); - bindAll(this, [ - 'clickDelayer', - 'handleClosePopover', - 'handleToggleOpenState', - 'handleTouchStart', - 'handleTouchOutside', - 'setButtonRef', - 'setContainerRef' - ]); - this.state = { - isOpen: false, - forceHide: false - }; - this.mainTooltipId = `tooltip-${Math.random()}`; - } - componentDidMount () { - // Touch start on the main button is caught to trigger open and not click - this.buttonRef.addEventListener('touchstart', this.handleTouchStart); - // Touch start on document is used to trigger close if it is outside - document.addEventListener('touchstart', this.handleTouchOutside); - } - shouldComponentUpdate (newProps, newState) { - // This check prevents re-rendering while the project is updating. - // @todo check only the state and the title because it is enough to know - // if anything substantial has changed - // This is needed because of the sloppy way the props are passed as a new object, - // which should be refactored. - return newState.isOpen !== this.state.isOpen || - newState.forceHide !== this.state.forceHide || - newProps.title !== this.props.title; - } - componentWillUnmount () { - this.buttonRef.removeEventListener('touchstart', this.handleTouchStart); - document.removeEventListener('touchstart', this.handleTouchOutside); - } - handleClosePopover () { - this.closeTimeoutId = setTimeout(() => { - this.setState({isOpen: false}); - this.closeTimeoutId = null; +const ActionMenu = ({ + className, + img: mainImg, + title: mainTitle, + moreButtons, + tooltipPlace, + onClick +}) => { + const [forceHide, setForceHide] = useState(false); + + const closeTimeoutRef = useRef(null); + const mainTooltipId = useRef(`tooltip-${Math.random()}`).current; + + const { + containerRef, + buttonRef, + isExpanded, + setIsExpanded, + handleOnFocus, + handleKeyDown + } = useActionMenuNavigation(); + + const handleClosePopover = useCallback(() => { + closeTimeoutRef.current = setTimeout(() => { + console.log("collapse popover"); + setIsExpanded(false); + closeTimeoutRef.current = null; }, CLOSE_DELAY); - } - handleToggleOpenState () { + }, []); + + const handleToggleOpenState = useCallback(() => { // Mouse enter back in after timeout was started prevents it from closing. - if (this.closeTimeoutId) { - clearTimeout(this.closeTimeoutId); - this.closeTimeoutId = null; - } else if (!this.state.isOpen) { - this.setState({ - isOpen: true, - forceHide: false - }); + + if (closeTimeoutRef.current) { + clearTimeout(closeTimeoutRef.current); + closeTimeoutRef.current = null; + } else if (!isExpanded) { + setIsExpanded(true); + setForceHide(false); } - } - handleTouchOutside (e) { - if (this.state.isOpen && !this.containerRef.contains(e.target)) { - this.setState({isOpen: false}); - ReactTooltip.hide(); - } - } - clickDelayer (fn) { + }, [isExpanded]); + + const clickDelayer = useCallback( // Return a wrapped action that manages the menu closing. // @todo we may be able to use react-transition for this in the future // for now all this work is to ensure the menu closes BEFORE the // (possibly slow) action is started. - return event => { + fn => (event => { ReactTooltip.hide(); if (fn) fn(event); // Blur the button so it does not keep focus after being clicked // This prevents keyboard events from triggering the button - this.buttonRef.blur(); - this.setState({forceHide: true, isOpen: false}, () => { - setTimeout(() => this.setState({forceHide: false})); - }); - }; - } - handleTouchStart (e) { - // Prevent this touch from becoming a click if menu is closed - if (!this.state.isOpen) { + buttonRef.current?.blur(); + setForceHide(true); + console.log("collapse click delayer"); + setIsExpanded(false); + setTimeout(() => setForceHide(false), 0); + }), + [] + ); + + const handleTouchStart = useCallback(e => { + // Prevent this touch from becoming a click if menu is closed + if (!isExpanded) { e.preventDefault(); - this.handleToggleOpenState(); + handleToggleOpenState(); } - } - setButtonRef (ref) { - this.buttonRef = ref; - } - setContainerRef (ref) { - this.containerRef = ref; - } - render () { - const { - className, - img: mainImg, - title: mainTitle, - moreButtons, - tooltipPlace, - onClick - } = this.props; - - return ( -
{ + const handleTouchOutside = e => { + console.log("is touch out"); + if (containerRef.current && !containerRef.current.contains(e.target)) { + console.log("collapse handle touch outside"); + setIsExpanded(false); + ReactTooltip.hide(); + } + }; + + const handleFocusIn = e => { + if (containerRef.current && !containerRef.current.contains(e.target)) { + console.log("collapse handle focus in"); + setIsExpanded(false); + } + }; + + document.addEventListener('touchstart', handleTouchOutside); + document.addEventListener('focusin', handleFocusIn); + + return () => { + document.removeEventListener('touchstart', handleTouchOutside); + document.removeEventListener('focusin', handleFocusIn); + }; + }, [containerRef, setIsExpanded]); + + useEffect(() => { + const buttonEl = buttonRef.current; + + if (!buttonEl) return; + + buttonEl.addEventListener('touchstart', handleTouchStart); + + return () => { + buttonEl.removeEventListener('touchstart', handleTouchStart); + }; + }, [handleTouchStart]); + + return ( +
+ - -
-
    - {(moreButtons || []).map(({img, title, onClick: handleClick, - fileAccept, fileChange, fileInput, fileMultiple}, keyId) => { + + +
    +
      + {(moreButtons || []).map( + ( + { + img, + title, + onClick: handleClick, + fileAccept, + fileChange, + fileInput, + fileMultiple + }, + keyId + ) => { const isComingSoon = !handleClick; const hasFileInput = fileInput; - const tooltipId = `${this.mainTooltipId}-${title}`; + const tooltipId = `${mainTooltipId}-${title}`; return ( -
    • +
      • ); - })} -
    -
    + } + )} +
- ); - } -} +
+ ); +}; ActionMenu.propTypes = { className: PropTypes.string, img: PropTypes.string, - moreButtons: PropTypes.arrayOf(PropTypes.shape({ - img: PropTypes.string, - title: PropTypes.node.isRequired, - onClick: PropTypes.func, // Optional, "coming soon" if no callback provided - fileAccept: PropTypes.string, // Optional, only for file upload - fileChange: PropTypes.func, // Optional, only for file upload - fileInput: PropTypes.func, // Optional, only for file upload - fileMultiple: PropTypes.bool // Optional, only for file upload - })), + moreButtons: PropTypes.arrayOf( + PropTypes.shape({ + img: PropTypes.string, + title: PropTypes.node.isRequired, + onClick: PropTypes.func, // Optional, "coming soon" if no callback provided + fileAccept: PropTypes.string, // Optional, only for file upload + fileChange: PropTypes.func, // Optional, only for file upload + fileInput: PropTypes.func, // Optional, only for file upload + fileMultiple: PropTypes.bool // Optional, only for file upload + }) + ), onClick: PropTypes.func.isRequired, title: PropTypes.node.isRequired, tooltipPlace: PropTypes.string diff --git a/packages/scratch-gui/src/components/stage-selector/stage-selector.jsx b/packages/scratch-gui/src/components/stage-selector/stage-selector.jsx index 12f3fa97dc6..904e6d8e2cf 100644 --- a/packages/scratch-gui/src/components/stage-selector/stage-selector.jsx +++ b/packages/scratch-gui/src/components/stage-selector/stage-selector.jsx @@ -110,7 +110,6 @@ const StageSelector = props => { title: intl.formatMessage(messages.addBackdropFromSurprise), img: surpriseIcon, onClick: onSurpriseBackdropClick - }, { title: intl.formatMessage(messages.addBackdropFromPaint), img: paintIcon, diff --git a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js new file mode 100644 index 00000000000..0acccb4eae3 --- /dev/null +++ b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js @@ -0,0 +1,85 @@ +import {useCallback, useState, useRef} from 'react'; +import {KEY} from '../lib/navigation-keys'; + +const MENU_ITEM_SELECTOR = '[data-action-menu-item="true"]'; + +export default function useActionMenuNavigation () { + const containerRef = useRef(null); + const buttonRef = useRef(null); + const [isExpanded, setIsExpanded] = useState(false); + + // BFS to find first children with attribute + const findSubitems = useCallback(() => { + if (!containerRef?.current) return []; + const subitems = []; + const root = containerRef.current; + const children = [...root.children]; + + while (children.length > 0) { + // if child is a menu item itself + const element = children.shift(); + if (element.matches(MENU_ITEM_SELECTOR) && element !== buttonRef.current) { + subitems.push(element); + } else { + children.push(...element.children); + } + } + + console.log(subitems); + return subitems; + }, [containerRef, buttonRef]); + + const focusItem = useCallback(item => { + if (item) { + item.focus(); + } + }, []); + + const handleOnFocus = useCallback(() => { + console.log("Should expand"); + setIsExpanded(true); + const items = findSubitems(); + if (!items.length) return; + + focusItem(items[0]); + }, [findSubitems, focusItem]); + + const handleMove = useCallback(direction => { + const items = findSubitems(); + if (!items.length) return; + + const currentIndex = items.indexOf(document.activeElement); + const nextIndex = (currentIndex + direction + items.length) % items.length; + focusItem(items[nextIndex]); + }, [findSubitems, focusItem]); + + const handleKeyDown = useCallback(e => { + switch (e.key) { + case KEY.ARROW_DOWN: + e.preventDefault(); + e.stopPropagation(); + handleMove(1); + break; + case KEY.ARROW_UP: + e.preventDefault(); + e.stopPropagation(); + handleMove(-1); + break; + case KEY.TAB: + console.log("Should collapse"); + if (isExpanded) setIsExpanded(false); + buttonRef?.current?.blur(); + + return; + } + }, [handleMove, setIsExpanded, buttonRef]); + + return { + containerRef, + buttonRef, + isExpanded, + setIsExpanded, + handleKeyDown, + handleOnFocus + }; +} From 4791d3bdf78feb7be8b219e525fd03aa7dd8c5e6 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Tue, 3 Feb 2026 14:29:31 +0200 Subject: [PATCH 02/32] chore: completed refocus on subelement logic and focus background like on hover --- .../src/components/action-menu/action-menu.css | 3 ++- .../src/components/action-menu/action-menu.jsx | 18 +++++++----------- .../src/hooks/use-action-menu-navigation.js | 16 ++++++++-------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/packages/scratch-gui/src/components/action-menu/action-menu.css b/packages/scratch-gui/src/components/action-menu/action-menu.css index 3c9bfa0ddff..ce2fc087558 100644 --- a/packages/scratch-gui/src/components/action-menu/action-menu.css +++ b/packages/scratch-gui/src/components/action-menu/action-menu.css @@ -29,7 +29,8 @@ button::-moz-focus-inner { border: 0; } -.button:hover { +.button:hover, +.button:focus { background: $extensions-primary; } diff --git a/packages/scratch-gui/src/components/action-menu/action-menu.jsx b/packages/scratch-gui/src/components/action-menu/action-menu.jsx index 8453af4b078..26168d2ea6d 100644 --- a/packages/scratch-gui/src/components/action-menu/action-menu.jsx +++ b/packages/scratch-gui/src/components/action-menu/action-menu.jsx @@ -37,6 +37,10 @@ const ActionMenu = ({ }, CLOSE_DELAY); }, []); + useEffect(() => { + console.log("isExpanded changed to " + isExpanded); + }, [isExpanded]); + const handleToggleOpenState = useCallback(() => { // Mouse enter back in after timeout was started prevents it from closing. @@ -44,6 +48,7 @@ const ActionMenu = ({ clearTimeout(closeTimeoutRef.current); closeTimeoutRef.current = null; } else if (!isExpanded) { + console.log("we do set it as expanded though"); setIsExpanded(true); setForceHide(false); } @@ -87,19 +92,10 @@ const ActionMenu = ({ } }; - const handleFocusIn = e => { - if (containerRef.current && !containerRef.current.contains(e.target)) { - console.log("collapse handle focus in"); - setIsExpanded(false); - } - }; - - document.addEventListener('touchstart', handleTouchOutside); - document.addEventListener('focusin', handleFocusIn); + document.addEventListener('mousedown', handleTouchOutside); return () => { - document.removeEventListener('touchstart', handleTouchOutside); - document.removeEventListener('focusin', handleFocusIn); + document.removeEventListener('mousedown', handleTouchOutside); }; }, [containerRef, setIsExpanded]); diff --git a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js index 0acccb4eae3..c6d4f800521 100644 --- a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js +++ b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js @@ -1,5 +1,6 @@ import {useCallback, useState, useRef} from 'react'; import {KEY} from '../lib/navigation-keys'; +import ReactTooltip from 'react-tooltip'; const MENU_ITEM_SELECTOR = '[data-action-menu-item="true"]'; @@ -18,16 +19,15 @@ export default function useActionMenuNavigation () { while (children.length > 0) { // if child is a menu item itself const element = children.shift(); - if (element.matches(MENU_ITEM_SELECTOR) && element !== buttonRef.current) { + if (element.matches(MENU_ITEM_SELECTOR)) { subitems.push(element); } else { children.push(...element.children); } } - console.log(subitems); return subitems; - }, [containerRef, buttonRef]); + }, [containerRef]); const focusItem = useCallback(item => { if (item) { @@ -36,13 +36,14 @@ export default function useActionMenuNavigation () { }, []); const handleOnFocus = useCallback(() => { - console.log("Should expand"); setIsExpanded(true); const items = findSubitems(); if (!items.length) return; - focusItem(items[0]); - }, [findSubitems, focusItem]); + // default to last item (first above) + const lastItem = items[items.length - 1]; + focusItem(lastItem); + }, [findSubitems, focusItem, setIsExpanded]); const handleMove = useCallback(direction => { const items = findSubitems(); @@ -66,13 +67,12 @@ export default function useActionMenuNavigation () { handleMove(-1); break; case KEY.TAB: - console.log("Should collapse"); if (isExpanded) setIsExpanded(false); buttonRef?.current?.blur(); return; } - }, [handleMove, setIsExpanded, buttonRef]); + }, [handleMove, isExpanded, setIsExpanded, buttonRef]); return { containerRef, From fb66c83c1b11312f4c6690eed78bc96b783ea861 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Tue, 3 Feb 2026 16:13:58 +0200 Subject: [PATCH 03/32] chore: jsdoc and refactoring --- .../components/action-menu/action-menu.jsx | 29 +------------ .../src/hooks/use-action-menu-navigation.js | 42 +++++++++++++++++-- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/packages/scratch-gui/src/components/action-menu/action-menu.jsx b/packages/scratch-gui/src/components/action-menu/action-menu.jsx index 26168d2ea6d..8629d73e208 100644 --- a/packages/scratch-gui/src/components/action-menu/action-menu.jsx +++ b/packages/scratch-gui/src/components/action-menu/action-menu.jsx @@ -31,16 +31,11 @@ const ActionMenu = ({ const handleClosePopover = useCallback(() => { closeTimeoutRef.current = setTimeout(() => { - console.log("collapse popover"); setIsExpanded(false); closeTimeoutRef.current = null; }, CLOSE_DELAY); }, []); - useEffect(() => { - console.log("isExpanded changed to " + isExpanded); - }, [isExpanded]); - const handleToggleOpenState = useCallback(() => { // Mouse enter back in after timeout was started prevents it from closing. @@ -48,7 +43,6 @@ const ActionMenu = ({ clearTimeout(closeTimeoutRef.current); closeTimeoutRef.current = null; } else if (!isExpanded) { - console.log("we do set it as expanded though"); setIsExpanded(true); setForceHide(false); } @@ -66,7 +60,6 @@ const ActionMenu = ({ // This prevents keyboard events from triggering the button buttonRef.current?.blur(); setForceHide(true); - console.log("collapse click delayer"); setIsExpanded(false); setTimeout(() => setForceHide(false), 0); }), @@ -80,32 +73,12 @@ const ActionMenu = ({ handleToggleOpenState(); } }, [isExpanded, handleToggleOpenState]); - - // Handle clicks/touches outside to close menu - useEffect(() => { - const handleTouchOutside = e => { - console.log("is touch out"); - if (containerRef.current && !containerRef.current.contains(e.target)) { - console.log("collapse handle touch outside"); - setIsExpanded(false); - ReactTooltip.hide(); - } - }; - - document.addEventListener('mousedown', handleTouchOutside); - - return () => { - document.removeEventListener('mousedown', handleTouchOutside); - }; - }, [containerRef, setIsExpanded]); - + useEffect(() => { const buttonEl = buttonRef.current; - if (!buttonEl) return; buttonEl.addEventListener('touchstart', handleTouchStart); - return () => { buttonEl.removeEventListener('touchstart', handleTouchStart); }; diff --git a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js index c6d4f800521..95e702fbeba 100644 --- a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js +++ b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js @@ -1,14 +1,47 @@ -import {useCallback, useState, useRef} from 'react'; +import {useCallback, useState, useRef, useEffect} from 'react'; import {KEY} from '../lib/navigation-keys'; import ReactTooltip from 'react-tooltip'; const MENU_ITEM_SELECTOR = '[data-action-menu-item="true"]'; -export default function useActionMenuNavigation () { +/** + * Custom hook to handle keyboard and focus navigation for an action menu. + * Supports focusing menu items, navigating with arrow keys, and closing the menu on outside clicks. + * Also provides refs for the menu container and the main button. + * @param {object} [options] - Optional configuration object. + * @param {number|null} [options.defaultItemIndex] - The index of the menu item to focus by default when + * the menu receives focus. If `null`, the last menu item will be focused. + * @returns {object} Navigation helpers and refs for use in an action menu component. + * - containerRef - Ref to the container element that holds all menu items. + * - buttonRef - Ref to the main action button. + * - isExpanded - Current expanded state of the menu. + * - setIsExpanded - Setter to manually expand/collapse the menu. + * - handleKeyDown - Keydown handler to navigate through menu items with arrow keys and handle tab. + * - handleOnFocus - Focus handler to expand the menu and focus the default item. + */ +export default function useActionMenuNavigation ( + {defaultItemIndex = null} = {} +) { const containerRef = useRef(null); const buttonRef = useRef(null); const [isExpanded, setIsExpanded] = useState(false); + // Handle clicks/touches outside to close menu + useEffect(() => { + const handleTouchOutside = e => { + if (containerRef.current && !containerRef.current.contains(e.target)) { + setIsExpanded(false); + ReactTooltip.hide(); + } + }; + + document.addEventListener('mousedown', handleTouchOutside); + return () => { + document.removeEventListener('mousedown', handleTouchOutside); + }; + }, [containerRef, setIsExpanded]); + + // BFS to find first children with attribute const findSubitems = useCallback(() => { if (!containerRef?.current) return []; @@ -41,8 +74,9 @@ export default function useActionMenuNavigation () { if (!items.length) return; // default to last item (first above) - const lastItem = items[items.length - 1]; - focusItem(lastItem); + const defaultItem = items[defaultItemIndex] ?? items[items.length - 1]; + focusItem(defaultItem); + // TODO: refresh tooltip so it repositions correctly }, [findSubitems, focusItem, setIsExpanded]); const handleMove = useCallback(direction => { From 0c94fbadbb012d19d5986aedff6b2611ec7de59e Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Thu, 5 Feb 2026 18:06:21 +0200 Subject: [PATCH 04/32] chore: fixed focus + hover bug and updated integration test --- .../src/hooks/use-action-menu-navigation.js | 16 ++++++++--- .../test/integration/sounds.test.js | 27 ++++++++++++------- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js index 95e702fbeba..260d0e59105 100644 --- a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js +++ b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js @@ -26,6 +26,17 @@ export default function useActionMenuNavigation ( const buttonRef = useRef(null); const [isExpanded, setIsExpanded] = useState(false); + useEffect(() => { + if (!isExpanded) { + // If menu is closed, blur any focused element to prevent keyboard events from affecting it + if (document.activeElement !== document.body) { + document.activeElement.blur(); + } + + buttonRef?.current?.blur(); + } + }, [isExpanded]); + // Handle clicks/touches outside to close menu useEffect(() => { const handleTouchOutside = e => { @@ -41,7 +52,6 @@ export default function useActionMenuNavigation ( }; }, [containerRef, setIsExpanded]); - // BFS to find first children with attribute const findSubitems = useCallback(() => { if (!containerRef?.current) return []; @@ -102,11 +112,9 @@ export default function useActionMenuNavigation ( break; case KEY.TAB: if (isExpanded) setIsExpanded(false); - buttonRef?.current?.blur(); - return; } - }, [handleMove, isExpanded, setIsExpanded, buttonRef]); + }, [handleMove, isExpanded, setIsExpanded]); return { containerRef, diff --git a/packages/scratch-gui/test/integration/sounds.test.js b/packages/scratch-gui/test/integration/sounds.test.js index 57bf11e861a..6afde13df5a 100644 --- a/packages/scratch-gui/test/integration/sounds.test.js +++ b/packages/scratch-gui/test/integration/sounds.test.js @@ -173,15 +173,24 @@ describe('Working with sounds', () => { await loadUri(uri); await clickText('Sounds'); - const el = await findByXpath('//button[@aria-label="Choose a Sound"]'); - await el.sendKeys(Key.chord(cmdCtrl, 'a')); // Select all - await findByText('0.85', scope.soundsTab); // Meow sound duration - await el.sendKeys(Key.DELETE); - await findByText('0.00', scope.soundsTab); // Sound is now empty - await el.sendKeys(Key.chord(cmdCtrl, 'z')); // undo - await findByText('0.85', scope.soundsTab); // Meow sound is back - await el.sendKeys(Key.chord(cmdCtrl, Key.SHIFT, 'z')); // redo - await findByText('0.00', scope.soundsTab); // Sound is empty again + await clickText('Meow'); + + // Send shortcuts to the app, not a specific button + await driver.actions().sendKeys(Key.chord(cmdCtrl, 'a')) + .perform(); + await findByText('0.85', scope.soundsTab); + + await driver.actions().sendKeys(Key.DELETE) + .perform(); + await findByText('0.00', scope.soundsTab); + + await driver.actions().sendKeys(Key.chord(cmdCtrl, 'z')) + .perform(); + await findByText('0.85', scope.soundsTab); + + await driver.actions().sendKeys(Key.chord(cmdCtrl, Key.SHIFT, 'z')) + .perform(); + await findByText('0.00', scope.soundsTab); const logs = await getLogs(); await expect(logs).toEqual([]); From f25eb2b99fe994e628b021171f59702ee41eb952 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Thu, 5 Feb 2026 18:22:15 +0200 Subject: [PATCH 05/32] chore: brought back old test --- .../test/integration/sounds.test.js | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/scratch-gui/test/integration/sounds.test.js b/packages/scratch-gui/test/integration/sounds.test.js index 6afde13df5a..57bf11e861a 100644 --- a/packages/scratch-gui/test/integration/sounds.test.js +++ b/packages/scratch-gui/test/integration/sounds.test.js @@ -173,24 +173,15 @@ describe('Working with sounds', () => { await loadUri(uri); await clickText('Sounds'); - await clickText('Meow'); - - // Send shortcuts to the app, not a specific button - await driver.actions().sendKeys(Key.chord(cmdCtrl, 'a')) - .perform(); - await findByText('0.85', scope.soundsTab); - - await driver.actions().sendKeys(Key.DELETE) - .perform(); - await findByText('0.00', scope.soundsTab); - - await driver.actions().sendKeys(Key.chord(cmdCtrl, 'z')) - .perform(); - await findByText('0.85', scope.soundsTab); - - await driver.actions().sendKeys(Key.chord(cmdCtrl, Key.SHIFT, 'z')) - .perform(); - await findByText('0.00', scope.soundsTab); + const el = await findByXpath('//button[@aria-label="Choose a Sound"]'); + await el.sendKeys(Key.chord(cmdCtrl, 'a')); // Select all + await findByText('0.85', scope.soundsTab); // Meow sound duration + await el.sendKeys(Key.DELETE); + await findByText('0.00', scope.soundsTab); // Sound is now empty + await el.sendKeys(Key.chord(cmdCtrl, 'z')); // undo + await findByText('0.85', scope.soundsTab); // Meow sound is back + await el.sendKeys(Key.chord(cmdCtrl, Key.SHIFT, 'z')); // redo + await findByText('0.00', scope.soundsTab); // Sound is empty again const logs = await getLogs(); await expect(logs).toEqual([]); From e80ff2f8368273ae0851d4145b259b0d5435d1ee Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Thu, 5 Feb 2026 18:32:45 +0200 Subject: [PATCH 06/32] chore: test fix --- packages/scratch-gui/test/integration/sounds.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/scratch-gui/test/integration/sounds.test.js b/packages/scratch-gui/test/integration/sounds.test.js index 57bf11e861a..ab5614a6677 100644 --- a/packages/scratch-gui/test/integration/sounds.test.js +++ b/packages/scratch-gui/test/integration/sounds.test.js @@ -173,7 +173,7 @@ describe('Working with sounds', () => { await loadUri(uri); await clickText('Sounds'); - const el = await findByXpath('//button[@aria-label="Choose a Sound"]'); + const el = await findByText('Meow', scope.soundsTab); await el.sendKeys(Key.chord(cmdCtrl, 'a')); // Select all await findByText('0.85', scope.soundsTab); // Meow sound duration await el.sendKeys(Key.DELETE); From 489159e5de4492deaeab06b7d603294b0bbc3058 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Thu, 5 Feb 2026 18:56:58 +0200 Subject: [PATCH 07/32] chore: fix the test by selecting editor --- packages/scratch-gui/test/integration/sounds.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/scratch-gui/test/integration/sounds.test.js b/packages/scratch-gui/test/integration/sounds.test.js index ab5614a6677..84751032ae3 100644 --- a/packages/scratch-gui/test/integration/sounds.test.js +++ b/packages/scratch-gui/test/integration/sounds.test.js @@ -173,7 +173,7 @@ describe('Working with sounds', () => { await loadUri(uri); await clickText('Sounds'); - const el = await findByText('Meow', scope.soundsTab); + const el = await findByXpath('//div[contains(@class, "sound-editor")]'); await el.sendKeys(Key.chord(cmdCtrl, 'a')); // Select all await findByText('0.85', scope.soundsTab); // Meow sound duration await el.sendKeys(Key.DELETE); From a8c4cad81c1dd4e52729f86590eeeaef37c4dcdd Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Fri, 6 Feb 2026 09:32:14 +0200 Subject: [PATCH 08/32] chore: made sound test work again --- packages/scratch-gui/test/integration/sounds.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/scratch-gui/test/integration/sounds.test.js b/packages/scratch-gui/test/integration/sounds.test.js index 84751032ae3..4a9ef82d421 100644 --- a/packages/scratch-gui/test/integration/sounds.test.js +++ b/packages/scratch-gui/test/integration/sounds.test.js @@ -173,7 +173,8 @@ describe('Working with sounds', () => { await loadUri(uri); await clickText('Sounds'); - const el = await findByXpath('//div[contains(@class, "sound-editor")]'); + // Just select any button inside the tab so the controls impact the sounds editor + const el = await findByXpath('//button[@aria-label="Delete"]'); await el.sendKeys(Key.chord(cmdCtrl, 'a')); // Select all await findByText('0.85', scope.soundsTab); // Meow sound duration await el.sendKeys(Key.DELETE); From bf0c57f29f9ed44dcc69faa3ea5bdebbe1693f80 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Fri, 6 Feb 2026 13:41:02 +0200 Subject: [PATCH 09/32] chore: fixed shift + tag bug artificially --- .../src/hooks/use-action-menu-navigation.js | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js index 260d0e59105..b189edb352d 100644 --- a/packages/scratch-gui/src/hooks/use-action-menu-navigation.js +++ b/packages/scratch-gui/src/hooks/use-action-menu-navigation.js @@ -25,9 +25,15 @@ export default function useActionMenuNavigation ( const containerRef = useRef(null); const buttonRef = useRef(null); const [isExpanded, setIsExpanded] = useState(false); + const skipNextBlurRef = useRef(false); useEffect(() => { if (!isExpanded) { + if (skipNextBlurRef.current) { + // Skip the blur this time + skipNextBlurRef.current = false; + return; + } // If menu is closed, blur any focused element to prevent keyboard events from affecting it if (document.activeElement !== document.body) { document.activeElement.blur(); @@ -35,7 +41,7 @@ export default function useActionMenuNavigation ( buttonRef?.current?.blur(); } - }, [isExpanded]); + }, [isExpanded, buttonRef, skipNextBlurRef]); // Handle clicks/touches outside to close menu useEffect(() => { @@ -102,16 +108,36 @@ export default function useActionMenuNavigation ( switch (e.key) { case KEY.ARROW_DOWN: e.preventDefault(); - e.stopPropagation(); handleMove(1); break; case KEY.ARROW_UP: e.preventDefault(); - e.stopPropagation(); handleMove(-1); break; case KEY.TAB: - if (isExpanded) setIsExpanded(false); + setIsExpanded(false); + // A little bit hacky logic for shift + tab to move focus to previous element + if (e.shiftKey) { + e.preventDefault(); + const focusables = Array.from( + document.querySelectorAll('a, button, input, select, textarea, [tabindex]') + ); + + const filteredFocusables = focusables.filter(el => { + // Skip disabled, hidden, or tabindex=-1 + if (el.disabled) return false; + if (el.offsetParent === null) return false; + const tabindex = el.getAttribute('tabindex'); + if (tabindex === '-1') return false; + return true; + }); + const currentIndex = filteredFocusables.indexOf(buttonRef.current); + if (currentIndex > 0) { + filteredFocusables[currentIndex - 1].focus(); + } + + skipNextBlurRef.current = true; + } return; } }, [handleMove, isExpanded, setIsExpanded]); From 71d9e63b316a1be7cf25b87817ea2a83dc69db06 Mon Sep 17 00:00:00 2001 From: Krum Angelov Date: Mon, 16 Feb 2026 12:58:42 +0200 Subject: [PATCH 10/32] chore: refactoring logic back inside action menu --- .../components/action-menu/action-menu.jsx | 115 +++++++++++-- .../src/hooks/use-action-menu-navigation.js | 153 ------------------ 2 files changed, 105 insertions(+), 163 deletions(-) delete mode 100644 packages/scratch-gui/src/hooks/use-action-menu-navigation.js diff --git a/packages/scratch-gui/src/components/action-menu/action-menu.jsx b/packages/scratch-gui/src/components/action-menu/action-menu.jsx index 8629d73e208..6a9969caca7 100644 --- a/packages/scratch-gui/src/components/action-menu/action-menu.jsx +++ b/packages/scratch-gui/src/components/action-menu/action-menu.jsx @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import ReactTooltip from 'react-tooltip'; import styles from './action-menu.css'; -import useActionMenuNavigation from '../../hooks/use-action-menu-navigation'; +import {KEY} from '../../lib/navigation-keys'; const CLOSE_DELAY = 300; // ms @@ -20,14 +20,106 @@ const ActionMenu = ({ const closeTimeoutRef = useRef(null); const mainTooltipId = useRef(`tooltip-${Math.random()}`).current; - const { - containerRef, - buttonRef, - isExpanded, - setIsExpanded, - handleOnFocus, - handleKeyDown - } = useActionMenuNavigation(); + const containerRef = useRef(null); + const buttonRef = useRef(null); + const [isExpanded, setIsExpanded] = useState(false); + const skipNextBlurRef = useRef(false); + const itemRefs = useRef([]); + + useEffect(() => { + if (!isExpanded) { + if (skipNextBlurRef.current) { + // Skip the blur this time + skipNextBlurRef.current = false; + return; + } + // If menu is closed, blur any focused element to prevent keyboard events from affecting it + if (document.activeElement !== document.body) { + document.activeElement.blur(); + } + + buttonRef?.current?.blur(); + } + }, [isExpanded, buttonRef, skipNextBlurRef]); + + // Handle clicks/touches outside to close menu + useEffect(() => { + const handleTouchOutside = e => { + if (containerRef.current && !containerRef.current.contains(e.target)) { + setIsExpanded(false); + ReactTooltip.hide(); + } + }; + + document.addEventListener('mousedown', handleTouchOutside); + return () => { + document.removeEventListener('mousedown', handleTouchOutside); + }; + }, [containerRef, setIsExpanded]); + + const focusItem = useCallback(item => { + if (item) { + item.focus(); + } + }, []); + + const handleOnFocus = useCallback(() => { + setIsExpanded(true); + const items = itemRefs.current; + if (!items.length) return; + + // default to last item (first above) + const defaultItem = items[items.length - 1]; + focusItem(defaultItem); + // TODO: refresh tooltip so it repositions correctly + }, [itemRefs, focusItem, setIsExpanded]); + + const handleMove = useCallback(direction => { + const items = itemRefs.current; + if (!items.length) return; + + const currentIndex = items.indexOf(document.activeElement); + const nextIndex = (currentIndex + direction + items.length) % items.length; + focusItem(items[nextIndex]); + }, [itemRefs, focusItem]); + + const handleKeyDown = useCallback(e => { + switch (e.key) { + case KEY.ARROW_DOWN: + e.preventDefault(); + handleMove(1); + break; + case KEY.ARROW_UP: + e.preventDefault(); + handleMove(-1); + break; + case KEY.TAB: + setIsExpanded(false); + // A little bit hacky logic for shift + tab to move focus to previous element + if (e.shiftKey) { + e.preventDefault(); + const focusables = Array.from( + document.querySelectorAll('a, button, input, select, textarea, [tabindex]') + ); + + const filteredFocusables = focusables.filter(el => { + // Skip disabled, hidden, or tabindex=-1 + if (el.disabled) return false; + if (el.offsetParent === null) return false; + const tabindex = el.getAttribute('tabindex'); + if (tabindex === '-1') return false; + return true; + }); + const currentIndex = filteredFocusables.indexOf(buttonRef.current); + if (currentIndex > 0) { + filteredFocusables[currentIndex - 1].focus(); + } + + skipNextBlurRef.current = true; + } + return; + } + }, [handleMove, isExpanded, setIsExpanded]); const handleClosePopover = useCallback(() => { closeTimeoutRef.current = setTimeout(() => { @@ -92,6 +184,7 @@ const ActionMenu = ({ })} onMouseEnter={handleToggleOpenState} onMouseLeave={handleClosePopover} + onBlur={handleClosePopover} ref={containerRef} >