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 a09fd906819..dad14ae88d3 100644 --- a/packages/scratch-gui/src/components/action-menu/action-menu.jsx +++ b/packages/scratch-gui/src/components/action-menu/action-menu.jsx @@ -1,152 +1,207 @@ +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 {KEY} from '../../lib/navigation-keys'; 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; - }, CLOSE_DELAY); - } - handleToggleOpenState () { +const ActionMenu = ({ + className, + img: mainImg, + title: mainTitle, + moreButtons, + tooltipPlace, + onClick +}) => { + const [forceHide, setForceHide] = useState(false); + const [isExpanded, setIsExpanded] = useState(false); + + const closeTimeoutRef = useRef(null); + const mainTooltipId = useRef(`tooltip-${Math.random()}`).current; + // refs to handle custom keyboard navigation behavior + const containerRef = useRef(null); + const buttonRef = useRef(null); + const itemRefs = useRef([]); + + const focusItem = useCallback(item => { + if (item) { + item.focus(); + } + }, []); + + 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); + focusItem(buttonRef.current); } - } - handleTouchOutside (e) { - if (this.state.isOpen && !this.containerRef.contains(e.target)) { - this.setState({isOpen: false}); - ReactTooltip.hide(); + }, [isExpanded]); + + const handleTouchStart = useCallback(e => { + // Prevent this touch from becoming a click if menu is closed + if (!isExpanded) { + e.preventDefault(); + handleToggleOpenState(); } - } - clickDelayer (fn) { - // 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 => { - 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})); - }); + }, [isExpanded, handleToggleOpenState]); + + useEffect(() => { + const buttonEl = buttonRef.current; + if (!buttonEl) return; + + buttonEl.addEventListener('touchstart', handleTouchStart); + return () => { + buttonEl.removeEventListener('touchstart', handleTouchStart); }; - } - handleTouchStart (e) { - // Prevent this touch from becoming a click if menu is closed - if (!this.state.isOpen) { + }, [handleTouchStart]); + + // 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 handleMove = useCallback(direction => { + const items = itemRefs.current; + if (!items.length) return; + + if (!items.includes(document.activeElement)) { + focusItem(direction === 1 ? items[0] : items[items.length - 1]); + return; + } + + const currentIndex = items.indexOf(document.activeElement); + const nextIndex = (currentIndex + direction + items.length) % items.length; + focusItem(items[nextIndex]); + }, [itemRefs, focusItem]); + + const handleClosePopover = useCallback(() => { + closeTimeoutRef.current = setTimeout(() => { + setIsExpanded(false); + closeTimeoutRef.current = null; + }, CLOSE_DELAY); + }, []); + + const handleKeyDown = useCallback(e => { + if (e.key === KEY.ARROW_DOWN || e.key === KEY.ARROW_UP) { + const direction = e.key === KEY.ARROW_UP ? -1 : 1; e.preventDefault(); - this.handleToggleOpenState(); + if (isExpanded) { + handleMove(direction); + } else { + setIsExpanded(true); + // wait to expand before moving focus to menu item and displaying tooltip + setTimeout(() => { + handleMove(direction); + }, CLOSE_DELAY); + } + } else if (e.key === KEY.TAB) { + setIsExpanded(false); + focusItem(buttonRef.current); + } else if (e.key === KEY.ESCAPE) { + focusItem(buttonRef.current); + } else if (e.key === KEY.ENTER) { + ReactTooltip.hide(); + setIsExpanded(false); + setForceHide(true); + buttonRef.current.blur(); + } + }, [handleMove, isExpanded, setIsExpanded]); + + // needed to resolve collision of styling based on mouse hovering and keyboard movement, + // so as not to highlight multiple items at the same time + const handleItemMouseEnter = useCallback(index => () => { + const items = itemRefs.current; + const currentFocusedIndex = items.indexOf(document.activeElement); + if (currentFocusedIndex === index) return; + + if (items[currentFocusedIndex]) { + items[currentFocusedIndex].blur(); + } + if (items[index]) { + focusItem(items[index]); + } else { + // Not a menu item, so it must be the main button + focusItem(buttonRef.current); } - } - setButtonRef (ref) { - this.buttonRef = ref; - } - setContainerRef (ref) { - this.containerRef = ref; - } - render () { - const { - className, - img: mainImg, - title: mainTitle, - moreButtons, - tooltipPlace, - onClick - } = this.props; - - return ( -
+ - -
-
    - {(moreButtons || []).map(({img, title, onClick: handleClick, - fileAccept, fileChange, fileInput, fileMultiple}, keyId) => { - const isComingSoon = !handleClick; + + +
    +
      + {(moreButtons || []).map( + ( + { + img, + title, + onClick: onClickItem, + fileAccept, + fileChange, + fileInput, + fileMultiple + }, + keyId + ) => { + const isComingSoon = !onClickItem; const hasFileInput = fileInput; - const tooltipId = `${this.mainTooltipId}-${title}`; + const tooltipId = `${mainTooltipId}-${title}`; + + const handleClick = useCallback(e => { + onClickItem(e); + ReactTooltip.hide(); + setIsExpanded(false); + setForceHide(true); + buttonRef.current.blur(); + }, [onClickItem]); + 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 }; -export default ActionMenu; +export default React.memo(ActionMenu, (prevProps, nextProps) => + // This check prevents re-rendering while the project is updating. + // This is needed because of the sloppy way the props are passed as a new object, + // which should be refactored. + // Only re-render if the title changes + prevProps.title === nextProps.title +); 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/test/integration/sounds.test.js b/packages/scratch-gui/test/integration/sounds.test.js index 57bf11e861a..5be2271b6e8 100644 --- a/packages/scratch-gui/test/integration/sounds.test.js +++ b/packages/scratch-gui/test/integration/sounds.test.js @@ -173,6 +173,7 @@ describe('Working with sounds', () => { await loadUri(uri); await clickText('Sounds'); + // Just select any button inside the tab so the controls impact the sounds editor 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