Give menus more control over how items are rendered

Initial implementation had `createMenu` take a `mapPropsToItems`, which
returned an array of objects containing properties `key`, `isEnabled`,
and `props`. The higher-order component was then passed a component
class which actually rendered items.

As well as setting up a structure that’s both rigid and not particularly
discoverable, this meant that menus were assumed to consist of a
homogeneous collection of items, each of which is rendered in the same
way and behaves in the same way when clicked. This is not true, however,
of things like the hamburger menu.

So, instead, `createMenu` exports a `<MenuItem>` component, which is
used to wrap the actual contents of a menu item and takes `key`,
`isEnabled`, and `onClick` props. The factory now takes a `renderItems`
function, which can return any valid JSX node. This also improves the
interface of generated menu components, since the click handler can now
be called whatever feels natural.

Author: Mat Brown

src/components/TopBar/LibraryPicker.js

@@ -0,0 +1,38 @@
+import classnames from 'classnames';
+import map from 'lodash/map';
+import partial from 'lodash/partial';
+import PropTypes from 'prop-types';
+import React from 'react';
+import libraries from '../../config/libraries';
+import createMenu, {MenuItem} from './createMenu';
+import LibraryPickerButton from './LibraryPickerButton';
+
+const LibraryPicker = createMenu({
+  name: 'libraryPicker',
+
+  renderItems({enabledLibraries, onToggleLibrary}) {
+    return map(libraries, (library, key) => {
+      const isEnabled = enabledLibraries.includes(key);
+
+      return (
+        <MenuItem
+          isEnabled={isEnabled}
+          key={key}
+          onClick={partial(onToggleLibrary, key)}
+        >
+          <span className={classnames('u__icon', {u__invisible: !isEnabled})}>
+            &#xf00c;{' '}
+          </span>
+          {library.name}
+        </MenuItem>
+      );
+    });
+  },
+})(LibraryPickerButton);
+
+LibraryPicker.propTypes = {
+  enabledLibraries: PropTypes.arrayOf(PropTypes.string).isRequired,
+  onToggleLibrary: PropTypes.func.isRequired,
+};
+
+export default LibraryPicker;

src/components/TopBar/LibraryPicker.jsx

@@ -1,8 +1,10 @@
 import map from 'lodash/map';
+import partial from 'lodash/partial';
 import PropTypes from 'prop-types';
+import React from 'react';
 import ProjectPreview from '../../containers/ProjectPreview';
 import ProjectPickerButton from './ProjectPickerButton';
-import createMenu from './createMenu';
+import createMenu, {MenuItem} from './createMenu';
 
 const ProjectPicker = createMenu({
   name: 'projectPicker',
@@ -11,12 +13,16 @@ const ProjectPicker = createMenu({
     return currentProjectKey && projectKeys.length > 1;
   },
 
-  mapPropsToItems({currentProjectKey, projectKeys}) {
-    return map(projectKeys, projectKey => ({
-      key: projectKey,
-      isEnabled: projectKey === currentProjectKey,
-      props: {projectKey},
-    }));
+  renderItems({currentProjectKey, projectKeys, onChangeCurrentProject}) {
+    return map(projectKeys, projectKey => (
+      <MenuItem
+        isEnabled={projectKey === currentProjectKey}
+        key={projectKey}
+        onClick={partial(onChangeCurrentProject, projectKey)}
+      >
+        <ProjectPreview projectKey={projectKey} />
+      </MenuItem>
+    ));
   },
 })(ProjectPickerButton, ProjectPreview);
 

src/components/TopBar/LibraryPickerItem.jsx

@@ -1,18 +1,38 @@
+/* eslint-disable react/no-multi-comp */
+
 import classnames from 'classnames';
 import {connect} from 'react-redux';
 import constant from 'lodash/constant';
 import onClickOutside from 'react-onclickoutside';
-import partial from 'lodash/partial';
 import preventClickthrough from 'react-prevent-clickthrough';
 import property from 'lodash/property';
 import PropTypes from 'prop-types';
 import React from 'react';
 import {closeTopBarMenu, toggleTopBarMenu} from '../../actions';
 import {getOpenTopBarMenu} from '../../selectors';
 
+export function MenuItem({children, isEnabled, onClick}) {
+  return (
+    <div
+      className={classnames('top-bar__menu-item',
+        {'top-bar__menu-item_active': isEnabled},
+      )}
+      onClick={onClick}
+    >
+      {children}
+    </div>
+  );
+}
+
+MenuItem.propTypes = {
+  children: PropTypes.node.isRequired,
+  isEnabled: PropTypes.bool.isRequired,
+  onClick: PropTypes.func.isRequired,
+};
+
 export default function createMenu({
   isVisible = constant(true),
-  mapPropsToItems,
+  renderItems,
   name,
 }) {
   function mapStateToProps(state) {
@@ -35,31 +55,20 @@ export default function createMenu({
     };
   }
 
-  return function createMenuWithMappedProps(Label, Item) {
+  return function createMenuWithMappedProps(Label) {
     function Menu(props) {
       if (!isVisible(props)) {
         return null;
       }
 
-      const {isOpen, onClickItem, onToggle} = props;
-      const items = mapPropsToItems(props);
+      const {isOpen, onToggle} = props;
       const menu = isOpen ?
         (
           <div
             className="top-bar__menu"
             onClick={preventClickthrough}
           >
-            {items.map(({key, enabled, props: itemProps}) => (
-              <div
-                className={classnames('top-bar__menu-item',
-                  {'top-bar__menu-item_active': enabled},
-                )}
-                key={key}
-                onClick={partial(onClickItem, key)}
-              >
-                <Item {...itemProps} />
-              </div>
-            ))}
+            {renderItems(props)}
           </div>
         ) : null;
 
@@ -81,7 +90,6 @@ export default function createMenu({
 
     Menu.propTypes = {
       isOpen: PropTypes.bool.isRequired,
-      onClickItem: PropTypes.func.isRequired,
       onToggle: PropTypes.func.isRequired,
     };
 

src/components/TopBar/ProjectPicker.jsx

@@ -45,9 +45,9 @@ export default function TopBar({
   onCreateSnapshot,
   onExportGist,
   onExportRepo,
-  onLibraryToggled,
   onLogOut,
   onStartLogIn,
+  onToggleLibrary,
   onToggleTextSize,
 }) {
   const {popVariant, modifier} = uiVariants({validationState, isUserTyping});
@@ -77,11 +77,11 @@ export default function TopBar({
       <ProjectPicker
         currentProjectKey={currentProjectKey}
         projectKeys={projectKeys}
-        onClickItem={onChangeCurrentProject}
+        onChangeCurrentProject={onChangeCurrentProject}
       />
       <LibraryPicker
         enabledLibraries={enabledLibraries}
-        onClickItem={partial(onLibraryToggled, currentProjectKey)}
+        onToggleLibrary={partial(onToggleLibrary, currentProjectKey)}
       />
       <SnapshotButton
         isInProgress={isSnapshotInProgress}
@@ -120,9 +120,9 @@ TopBar.propTypes = {
   onCreateSnapshot: PropTypes.func.isRequired,
   onExportGist: PropTypes.func.isRequired,
   onExportRepo: PropTypes.func.isRequired,
-  onLibraryToggled: PropTypes.func.isRequired,
   onLogOut: PropTypes.func.isRequired,
   onStartLogIn: PropTypes.func.isRequired,
+  onToggleLibrary: PropTypes.func.isRequired,
   onToggleTextSize: PropTypes.func.isRequired,
 };
 

src/components/TopBar/createMenu.jsx

@@ -81,7 +81,7 @@ function mapDispatchToProps(dispatch) {
       dispatch(exportRepo());
     },
 
-    onLibraryToggled(projectKey, libraryKey) {
+    onToggleLibrary(projectKey, libraryKey) {
       dispatch(toggleLibrary(projectKey, libraryKey));
     },