Use click event of mouseup to open menus

closes https://github.com/TurboWarp/scratch-gui/issues/873

src/components/menu-bar/menu-bar-menu.jsx

@@ -5,7 +5,6 @@ import Menu from '../../containers/menu.jsx';
 const MenuBarMenu = ({
     children,
     className,
-    onRequestClose,
     open,
     place = 'right'
 }) => (
@@ -13,7 +12,6 @@ const MenuBarMenu = ({
         <Menu
             open={open}
             place={place}
-            onRequestClose={onRequestClose}
         >
             {children}
         </Menu>
@@ -23,7 +21,6 @@ const MenuBarMenu = ({
 MenuBarMenu.propTypes = {
     children: PropTypes.node,
     className: PropTypes.string,
-    onRequestClose: PropTypes.func,
     open: PropTypes.bool,
     place: PropTypes.oneOf(['left', 'right'])
 };

src/components/menu-bar/menu-bar.jsx

@@ -18,6 +18,7 @@ import Divider from '../divider/divider.jsx';
 import SaveStatus from './save-status.jsx';
 import ProjectWatcher from '../../containers/project-watcher.jsx';
 import MenuBarMenu from './menu-bar-menu.jsx';
+import MenuLabel from './tw-menu-label.jsx';
 import {MenuItem, MenuSection} from '../menu/menu.jsx';
 import ProjectTitleInput from './project-title-input.jsx';
 import AuthorInfo from './author-info.jsx';
@@ -391,11 +392,10 @@ class MenuBar extends React.Component {
         // each item must have a 'title' FormattedMessage and a 'handleClick' function
         // generate a menu with items for each object in the array
         return (
-            <div
-                className={classNames(styles.menuBarItem, styles.hoverable, {
-                    [styles.active]: this.props.aboutMenuOpen
-                })}
-                onMouseUp={this.props.onRequestOpenAbout}
+            <MenuLabel
+                open={this.props.aboutMenuOpen}
+                onOpen={this.props.onRequestOpenAbout}
+                onClose={this.props.onRequestCloseAbout}
             >
                 <img
                     className={styles.aboutIcon}
@@ -406,7 +406,6 @@ class MenuBar extends React.Component {
                     className={classNames(styles.menuBarMenu)}
                     open={this.props.aboutMenuOpen}
                     place={this.props.isRtl ? 'right' : 'left'}
-                    onRequestClose={this.props.onRequestCloseAbout}
                 >
                     {
                         onClickAbout.map(itemProps => (
@@ -420,7 +419,7 @@ class MenuBar extends React.Component {
                         ))
                     }
                 </MenuBarMenu>
-            </div>
+            </MenuLabel>
         );
     }
     wrapAboutMenuCallback (callback) {
@@ -483,11 +482,10 @@ class MenuBar extends React.Component {
                 <div className={styles.mainMenu}>
                     <div className={styles.fileGroup}>
                         {this.props.errors.length > 0 && <div>
-                            <div
-                                className={classNames(styles.menuBarItem, styles.hoverable, {
-                                    [styles.active]: this.props.errorsMenuOpen
-                                })}
-                                onMouseUp={this.props.onClickErrors}
+                            <MenuLabel
+                                open={this.props.errorsMenuOpen}
+                                onOpen={this.props.onClickErrors}
+                                onClose={this.props.onRequestCloseErrors}
                             >
                                 <img
                                     src={errorIcon}
@@ -505,7 +503,6 @@ class MenuBar extends React.Component {
                                     className={classNames(styles.menuBarMenu)}
                                     open={this.props.errorsMenuOpen}
                                     place={this.props.isRtl ? 'left' : 'right'}
-                                    onRequestClose={this.props.onRequestCloseErrors}
                                 >
                                     <MenuSection>
                                         <MenuItemLink href="https://scratch.mit.edu/users/GarboMuffin/#comments">
@@ -534,7 +531,7 @@ class MenuBar extends React.Component {
                                         ))}
                                     </MenuSection>
                                 </MenuBarMenu>
-                            </div>
+                            </MenuLabel>
                         </div>}
                         {(this.props.canChangeTheme || this.props.canChangeLanguage) && (<SettingsMenu
                             canChangeLanguage={this.props.canChangeLanguage}
@@ -547,11 +544,10 @@ class MenuBar extends React.Component {
                             settingsMenuOpen={this.props.settingsMenuOpen}
                         />)}
                         {(this.props.canManageFiles) && (
-                            <div
-                                className={classNames(styles.menuBarItem, styles.hoverable, {
-                                    [styles.active]: this.props.fileMenuOpen
-                                })}
-                                onMouseUp={this.props.onClickFile}
+                            <MenuLabel
+                                open={this.props.fileMenuOpen}
+                                onOpen={this.props.onClickFile}
+                                onClose={this.props.onRequestCloseFile}
                             >
                                 <img
                                     src={fileIcon}
@@ -576,7 +572,6 @@ class MenuBar extends React.Component {
                                     className={classNames(styles.menuBarMenu)}
                                     open={this.props.fileMenuOpen}
                                     place={this.props.isRtl ? 'left' : 'right'}
-                                    onRequestClose={this.props.onRequestCloseFile}
                                 >
                                     <MenuItem
                                         isRtl={this.props.isRtl}
@@ -696,13 +691,12 @@ class MenuBar extends React.Component {
                                         </MenuItem>
                                     </MenuSection>
                                 </MenuBarMenu>
-                            </div>
+                            </MenuLabel>
                         )}
-                        <div
-                            className={classNames(styles.menuBarItem, styles.hoverable, {
-                                [styles.active]: this.props.editMenuOpen
-                            })}
-                            onMouseUp={this.props.onClickEdit}
+                        <MenuLabel
+                            open={this.props.editMenuOpen}
+                            onOpen={this.props.onClickEdit}
+                            onClose={this.props.onRequestCloseEdit}
                         >
                             <img
                                 src={editIcon}
@@ -727,7 +721,6 @@ class MenuBar extends React.Component {
                                 className={classNames(styles.menuBarMenu)}
                                 open={this.props.editMenuOpen}
                                 place={this.props.isRtl ? 'left' : 'right'}
-                                onRequestClose={this.props.onRequestCloseEdit}
                             >
                                 {this.props.isPlayerOnly ? null : (
                                     <DeletionRestorer>{(handleRestore, {restorable, deletedItem}) => (
@@ -823,26 +816,22 @@ class MenuBar extends React.Component {
                                     </MenuItem>
                                 </MenuSection>
                             </MenuBarMenu>
-                        </div>
+                        </MenuLabel>
                         {this.props.isTotallyNormal && (
-                            <div
-                                className={classNames(styles.menuBarItem, styles.hoverable, {
-                                    [styles.active]: this.props.modeMenuOpen
-                                })}
-                                onMouseUp={this.props.onClickMode}
+                            <MenuLabel
+                                open={this.props.modeMenuOpen}
+                                onOpen={this.props.onClickMode}
+                                onClose={this.props.onRequestCloseMode}
                             >
-                                <div className={classNames(styles.editMenu)}>
-                                    <FormattedMessage
-                                        defaultMessage="Mode"
-                                        description="Mode menu item in the menu bar"
-                                        id="gui.menuBar.modeMenu"
-                                    />
-                                </div>
+                                <FormattedMessage
+                                    defaultMessage="Mode"
+                                    description="Mode menu item in the menu bar"
+                                    id="gui.menuBar.modeMenu"
+                                />
                                 <MenuBarMenu
                                     className={classNames(styles.menuBarMenu)}
                                     open={this.props.modeMenuOpen}
                                     place={this.props.isRtl ? 'left' : 'right'}
-                                    onRequestClose={this.props.onRequestCloseMode}
                                 >
                                     <MenuSection>
                                         <MenuItem onClick={this.handleSetMode('NOW')}>
@@ -869,13 +858,13 @@ class MenuBar extends React.Component {
                                         </MenuItem>
                                     </MenuSection>
                                 </MenuBarMenu>
-                            </div>
+                            </MenuLabel>
                         )}
 
                         {this.props.onClickAddonSettings && (
                             <div
                                 className={classNames(styles.menuBarItem, styles.hoverable)}
-                                onMouseUp={this.props.onClickAddonSettings}
+                                onClick={this.props.onClickAddonSettings}
                             >
                                 <img
                                     src={addonsIcon}
@@ -895,7 +884,7 @@ class MenuBar extends React.Component {
                         {this.props.onClickSettingsModal && (
                             <div
                                 className={classNames(styles.menuBarItem, styles.hoverable)}
-                                onMouseUp={this.props.onClickSettingsModal}
+                                onClick={this.props.onClickSettingsModal}
                             >
                                 <img
                                     src={advancedIcon}

src/components/menu-bar/settings-menu.jsx

@@ -6,6 +6,7 @@ import {FormattedMessage} from 'react-intl';
 import LanguageMenu from './language-menu.jsx';
 import MenuBarMenu from './menu-bar-menu.jsx';
 import {MenuSection} from '../menu/menu.jsx';
+import MenuLabel from './tw-menu-label.jsx';
 import TWAccentThemeMenu from './tw-theme-accent.jsx';
 import TWGuiThemeMenu from './tw-theme-gui.jsx';
 import TWBlocksThemeMenu from './tw-theme-blocks.jsx';
@@ -25,11 +26,10 @@ const SettingsMenu = ({
     onRequestOpen,
     settingsMenuOpen
 }) => (
-    <div
-        className={classNames(menuBarStyles.menuBarItem, menuBarStyles.hoverable, menuBarStyles.themeMenu, {
-            [menuBarStyles.active]: settingsMenuOpen
-        })}
-        onMouseUp={onRequestOpen}
+    <MenuLabel
+        open={settingsMenuOpen}
+        onOpen={onRequestOpen}
+        onClose={onRequestClose}
     >
         <img
             src={settingsIcon}
@@ -54,7 +54,6 @@ const SettingsMenu = ({
             className={menuBarStyles.menuBarMenu}
             open={settingsMenuOpen}
             place={isRtl ? 'left' : 'right'}
-            onRequestClose={onRequestClose}
         >
             <MenuSection>
                 {canChangeLanguage && <LanguageMenu onRequestCloseSettings={onRequestClose} />}
@@ -69,7 +68,7 @@ const SettingsMenu = ({
                 )}
             </MenuSection>
         </MenuBarMenu>
-    </div>
+    </MenuLabel>
 );
 
 SettingsMenu.propTypes = {

src/components/menu-bar/tw-menu-label.jsx

@@ -0,0 +1,75 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import bindAll from 'lodash.bindall';
+import classNames from 'classnames';
+
+import styles from './menu-bar.css';
+
+class MenuLabel extends React.Component {
+    constructor (props) {
+        super(props);
+        bindAll(this, [
+            'handleClick',
+            'handleMouseUp',
+            'menuRef'
+        ]);
+    }
+    componentDidMount () {
+        if (this.props.open) this.addListeners();
+    }
+    componentDidUpdate (prevProps) {
+        if (this.props.open && !prevProps.open) this.addListeners();
+        if (!this.props.open && prevProps.open) this.removeListeners();
+    }
+    componentWillUnmount () {
+        this.removeListeners();
+    }
+    addListeners () {
+        document.addEventListener('mouseup', this.handleMouseUp);
+    }
+    removeListeners () {
+        document.removeEventListener('mouseup', this.handleMouseUp);
+    }
+    handleClick (e) {
+        // this is a bit sketchy, but we want to allow clicking on the menu itself and the images
+        // and text directly inside it, but not the items inside the menu, which are under the button
+        // in the DOM.
+        if (e.target.closest('div') === this.menuEl) {
+            if (this.props.open) {
+                this.props.onClose();
+            } else {
+                this.props.onOpen();
+            }
+        }
+    }
+    handleMouseUp (e) {
+        if (this.props.open && !this.menuEl.contains(e.target)) {
+            this.props.onClose();
+        }
+    }
+    menuRef (c) {
+        this.menuEl = c;
+    }
+    render () {
+        return (
+            <div
+                className={classNames(styles.menuBarItem, styles.hoverable, {
+                    [styles.active]: this.props.open
+                })}
+                onClick={this.handleClick}
+                ref={this.menuRef}
+            >
+                {this.props.children}
+            </div>
+        );
+    }
+}
+
+MenuLabel.propTypes = {
+    children: PropTypes.node,
+    open: PropTypes.bool,
+    onOpen: PropTypes.func,
+    onClose: PropTypes.func
+};
+
+export default MenuLabel;

src/containers/menu.jsx

@@ -1,64 +1,18 @@
-import bindAll from 'lodash.bindall';
 import PropTypes from 'prop-types';
 import React from 'react';
 
 import MenuComponent from '../components/menu/menu.jsx';
 
-class Menu extends React.Component {
-    constructor (props) {
-        super(props);
-        bindAll(this, [
-            'addListeners',
-            'removeListeners',
-            'handleClick',
-            'ref'
-        ]);
-    }
-    componentDidMount () {
-        if (this.props.open) this.addListeners();
-    }
-    componentDidUpdate (prevProps) {
-        if (this.props.open && !prevProps.open) this.addListeners();
-        if (!this.props.open && prevProps.open) this.removeListeners();
-    }
-    componentWillUnmount () {
-        this.removeListeners();
-    }
-    addListeners () {
-        document.addEventListener('mouseup', this.handleClick);
-    }
-    removeListeners () {
-        document.removeEventListener('mouseup', this.handleClick);
-    }
-    handleClick (e) {
-        if (this.props.open && !this.menu.contains(e.target)) {
-            this.props.onRequestClose();
-        }
-    }
-    ref (c) {
-        this.menu = c;
-    }
-    render () {
-        const {
-            open,
-            children,
-            ...props
-        } = this.props;
-        if (!open) return null;
-        return (
-            <MenuComponent
-                componentRef={this.ref}
-                {...props}
-            >
-                {children}
-            </MenuComponent>
-        );
-    }
-}
+const Menu = ({open, children, ...props}) => (
+    open ? (
+        <MenuComponent {...props}>
+            {children}
+        </MenuComponent>
+    ) : null
+);
 
 Menu.propTypes = {
     children: PropTypes.node,
-    onRequestClose: PropTypes.func.isRequired,
     open: PropTypes.bool.isRequired
 };