From 45b3adb6b01b8ac95761650d603ff92cfda410d7 Mon Sep 17 00:00:00 2001
From: Martin <martin@moid.se>
Date: Tue, 21 Apr 2020 18:46:13 +0200
Subject: [PATCH] fix: rewrite MenuBar buttons (TECH-320) (#879)

* refactor: new MenuButton component to replace MUI buttons
* refactor: menubar style js to css module

Introduces a new custom component MenuButton to replace the old MUI buttons for Options, Download and Interpretations in the MenuBar.
https://jira.dhis2.org/browse/TECH-320

Note: As the buttons needs to match the color scheme of FileMenu (@dhis2/d2-ui-file-menu), the colors aren't fetched from ui-core just yet. There's an inline comment with a TODO about this.
---
 .../components/DownloadMenu/DownloadMenu.js   | 10 ++---
 .../Interpretations/InterpretationsButton.js  | 14 ++-----
 .../__tests__/IterpretationsButton.spec.js    | 12 +++---
 .../app/src/components/MenuBar/MenuBar.js     | 23 +++++------
 .../MenuBar/styles/MenuBar.module.css         | 13 +++++++
 .../MenuBar/styles/MenuBar.style.js           | 22 -----------
 .../src/components/MenuButton/MenuButton.js   | 38 +++++++++++++++++++
 .../MenuButton/styles/MenuButton.module.css   | 30 +++++++++++++++
 .../VisualizationOptionsManager.js            |  8 ++--
 9 files changed, 105 insertions(+), 65 deletions(-)
 create mode 100644 packages/app/src/components/MenuBar/styles/MenuBar.module.css
 delete mode 100644 packages/app/src/components/MenuBar/styles/MenuBar.style.js
 create mode 100644 packages/app/src/components/MenuButton/MenuButton.js
 create mode 100644 packages/app/src/components/MenuButton/styles/MenuButton.module.css

diff --git a/packages/app/src/components/DownloadMenu/DownloadMenu.js b/packages/app/src/components/DownloadMenu/DownloadMenu.js
index 2c71cdb5e0..ae6c087bd0 100644
--- a/packages/app/src/components/DownloadMenu/DownloadMenu.js
+++ b/packages/app/src/components/DownloadMenu/DownloadMenu.js
@@ -3,7 +3,6 @@ import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
 import { createSelector } from 'reselect'
 
-import Button from '@material-ui/core/Button'
 import Divider from '@material-ui/core/Divider'
 import Menu from '@material-ui/core/Menu'
 import MenuItem from '@material-ui/core/MenuItem'
@@ -35,6 +34,7 @@ import {
     apiDownloadData,
     apiDownloadTable,
 } from '../../api/analytics'
+import MenuButton from '../MenuButton/MenuButton'
 
 export class DownloadMenu extends Component {
     constructor(props) {
@@ -168,15 +168,12 @@ export class DownloadMenu extends Component {
     render() {
         return (
             <Fragment>
-                <Button
-                    className={this.props.className}
+                <MenuButton
                     onClick={event => this.toggleMenu(event.currentTarget)}
-                    disableRipple={true}
-                    disableFocusRipple={true}
                     disabled={!this.props.current}
                 >
                     {i18n.t('Download')}
-                </Button>
+                </MenuButton>
                 <Menu
                     open={Boolean(this.state.anchorEl)}
                     anchorEl={this.state.anchorEl}
@@ -345,7 +342,6 @@ const relativePeriodDateSelector = createSelector(
 
 DownloadMenu.propTypes = {
     chart: PropTypes.string,
-    className: PropTypes.string,
     columns: PropTypes.array,
     current: PropTypes.object,
     relativePeriodDate: PropTypes.string,
diff --git a/packages/app/src/components/Interpretations/InterpretationsButton.js b/packages/app/src/components/Interpretations/InterpretationsButton.js
index 5dbbf9b3e1..74db69eae3 100644
--- a/packages/app/src/components/Interpretations/InterpretationsButton.js
+++ b/packages/app/src/components/Interpretations/InterpretationsButton.js
@@ -2,7 +2,6 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
 import { withStyles } from '@material-ui/core/styles'
-import Button from '@material-ui/core/Button'
 import KeyboardArrowLeftIcon from '@material-ui/icons/KeyboardArrowLeft'
 import KeyboardArrowRightIcon from '@material-ui/icons/KeyboardArrowRight'
 import i18n from '@dhis2/d2-i18n'
@@ -10,6 +9,7 @@ import i18n from '@dhis2/d2-i18n'
 import { sGetUiRightSidebarOpen } from '../../reducers/ui'
 import { sGetCurrent } from '../../reducers/current'
 import { acToggleUiRightSidebarOpen } from '../../actions/ui'
+import MenuButton from '../MenuButton/MenuButton'
 
 const styles = theme => ({
     icon: {
@@ -18,26 +18,18 @@ const styles = theme => ({
     },
 })
 export const InterpretationsButton = props => (
-    <Button
-        className={props.className}
-        size="small"
-        disabled={!props.id}
-        disableRipple={true}
-        disableFocusRipple={true}
-        onClick={props.onClick}
-    >
+    <MenuButton disabled={!props.id} onClick={props.onClick}>
         {props.rightSidebarOpen ? (
             <KeyboardArrowRightIcon className={props.classes.icon} />
         ) : (
             <KeyboardArrowLeftIcon className={props.classes.icon} />
         )}
         {i18n.t('Interpretations')}
-    </Button>
+    </MenuButton>
 )
 
 InterpretationsButton.propTypes = {
     classes: PropTypes.object.isRequired,
-    className: PropTypes.string,
     id: PropTypes.string,
     rightSidebarOpen: PropTypes.bool,
     onClick: PropTypes.func,
diff --git a/packages/app/src/components/Interpretations/__tests__/IterpretationsButton.spec.js b/packages/app/src/components/Interpretations/__tests__/IterpretationsButton.spec.js
index 09414c84ab..948c66d1f2 100644
--- a/packages/app/src/components/Interpretations/__tests__/IterpretationsButton.spec.js
+++ b/packages/app/src/components/Interpretations/__tests__/IterpretationsButton.spec.js
@@ -1,9 +1,9 @@
 import React from 'react'
 import { shallow } from 'enzyme'
 import { InterpretationsButton } from '../InterpretationsButton'
-import Button from '@material-ui/core/Button'
 import KeyboardArrowLeftIcon from '@material-ui/icons/KeyboardArrowLeft'
 import KeyboardArrowRightIcon from '@material-ui/icons/KeyboardArrowRight'
+import MenuButton from '../../MenuButton/MenuButton'
 
 describe('InterpretationsButton component', () => {
     let props
@@ -31,27 +31,27 @@ describe('InterpretationsButton component', () => {
         shallowInterpretationsButton = undefined
     })
 
-    it('renders a <Button>', () => {
+    it('renders a <MenuButton>', () => {
         expect(
             interpretationsButton()
-                .find(Button)
+                .find(MenuButton)
                 .first().length
         ).toEqual(1)
     })
 
-    it('renders a disabled <Button> if no id is passed', () => {
+    it('renders a disabled <MenuButton> if no id is passed', () => {
         props.id = null
 
         expect(
             interpretationsButton()
-                .find(Button)
+                .find(MenuButton)
                 .prop('disabled')
         ).toEqual(true)
     })
 
     it('it triggers onClick when the button is clicked', () => {
         interpretationsButton()
-            .find(Button)
+            .find(MenuButton)
             .simulate('click')
 
         expect(onClick).toHaveBeenCalled()
diff --git a/packages/app/src/components/MenuBar/MenuBar.js b/packages/app/src/components/MenuBar/MenuBar.js
index db95b13b3d..ab9edd6a90 100644
--- a/packages/app/src/components/MenuBar/MenuBar.js
+++ b/packages/app/src/components/MenuBar/MenuBar.js
@@ -2,7 +2,6 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
 import FileMenu from '@dhis2/d2-ui-file-menu'
-import { withStyles } from '@material-ui/core/styles'
 
 import UpdateButton from '../UpdateButton/UpdateButton'
 import DownloadMenu from '../DownloadMenu/DownloadMenu'
@@ -13,7 +12,7 @@ import * as fromActions from '../../actions'
 import { sGetCurrent } from '../../reducers/current'
 import history from '../../modules/history'
 import { parseError } from '../../modules/error'
-import styles from './styles/MenuBar.style'
+import styles from './styles/MenuBar.module.css'
 
 const onOpen = id => {
     const path = `/${id}`
@@ -30,12 +29,12 @@ const getOnSaveAs = props => details => props.onSaveVisualization(details, true)
 const getOnDelete = props => () => props.onDeleteVisualization()
 const getOnError = props => error => props.onError(error)
 
-export const MenuBar = ({ classes, ...props }, context) => (
-    <div className={classes.menuBar}>
+export const MenuBar = ({ ...props }, context) => (
+    <div className={styles.menuBar}>
         <UpdateVisualizationContainer
             renderComponent={handler => (
                 <UpdateButton
-                    className={classes.updateButton}
+                    className={styles.updateButton}
                     small
                     onClick={handler}
                 />
@@ -53,15 +52,14 @@ export const MenuBar = ({ classes, ...props }, context) => (
             onDelete={getOnDelete(props)}
             onError={getOnError(props)}
         />
-        <VisualizationOptionsManager className={classes.label} />
-        <DownloadMenu className={classes.label} />
-        <div className={classes.grow} />
-        <InterpretationsButton className={classes.label} />
+        <VisualizationOptionsManager />
+        <DownloadMenu />
+        <div className={styles.grow} />
+        <InterpretationsButton />
     </div>
 )
 
 MenuBar.propTypes = {
-    classes: PropTypes.object.isRequired,
     apiObjectName: PropTypes.string,
     id: PropTypes.string,
 }
@@ -92,7 +90,4 @@ const mapDispatchToProps = dispatch => ({
     },
 })
 
-export default connect(
-    mapStateToProps,
-    mapDispatchToProps
-)(withStyles(styles)(MenuBar))
+export default connect(mapStateToProps, mapDispatchToProps)(MenuBar)
diff --git a/packages/app/src/components/MenuBar/styles/MenuBar.module.css b/packages/app/src/components/MenuBar/styles/MenuBar.module.css
new file mode 100644
index 0000000000..1ff0da6243
--- /dev/null
+++ b/packages/app/src/components/MenuBar/styles/MenuBar.module.css
@@ -0,0 +1,13 @@
+.menuBar {
+    background: var(--colors-white);
+    display: flex;
+    align-items: center;
+    padding: 0 var(--spacers-dp8);
+    height: 38px;
+}
+.updateButton {
+    margin-right: var(--spacers-dp8);
+}
+.grow {
+    flex: 1 1 0%;
+}
diff --git a/packages/app/src/components/MenuBar/styles/MenuBar.style.js b/packages/app/src/components/MenuBar/styles/MenuBar.style.js
deleted file mode 100644
index 474aec239b..0000000000
--- a/packages/app/src/components/MenuBar/styles/MenuBar.style.js
+++ /dev/null
@@ -1,22 +0,0 @@
-import { colors } from '@dhis2/ui-core'
-
-export default theme => ({
-    menuBar: {
-        background: colors.white,
-        display: 'flex',
-        alignItems: 'center',
-        padding: `0 ${theme.spacing.unit}px`,
-        height: '38px',
-    },
-    updateButton: {
-        marginRight: theme.spacing.unit,
-    },
-    label: {
-        fontSize: '15px',
-        fontWeight: 400,
-        textTransform: 'none',
-    },
-    grow: {
-        flex: '1 1 0%',
-    },
-})
diff --git a/packages/app/src/components/MenuButton/MenuButton.js b/packages/app/src/components/MenuButton/MenuButton.js
new file mode 100644
index 0000000000..16dbc75693
--- /dev/null
+++ b/packages/app/src/components/MenuButton/MenuButton.js
@@ -0,0 +1,38 @@
+import PropTypes from '@dhis2/prop-types'
+import React from 'react'
+
+import styles from './styles/MenuButton.module.css'
+
+const MenuButton = ({
+    children,
+    dataTest,
+    disabled,
+    name,
+    onBlur,
+    onClick,
+    onFocus,
+}) => (
+    <button
+        className={styles.menuButton}
+        data-test={dataTest}
+        disabled={disabled}
+        name={name}
+        onBlur={onBlur}
+        onClick={onClick}
+        onFocus={onFocus}
+    >
+        {children}
+    </button>
+)
+
+MenuButton.propTypes = {
+    children: PropTypes.node,
+    dataTest: PropTypes.string,
+    disabled: PropTypes.bool,
+    name: PropTypes.string,
+    onBlur: PropTypes.func,
+    onClick: PropTypes.func,
+    onFocus: PropTypes.func,
+}
+
+export default MenuButton
diff --git a/packages/app/src/components/MenuButton/styles/MenuButton.module.css b/packages/app/src/components/MenuButton/styles/MenuButton.module.css
new file mode 100644
index 0000000000..47982eac27
--- /dev/null
+++ b/packages/app/src/components/MenuButton/styles/MenuButton.module.css
@@ -0,0 +1,30 @@
+/* TODO: Fetch the colors from ui-core, once the same is done for the FileMenu (@dhis2/d2-ui-file-menu)  */
+
+.menuButton {
+    display: inline-flex;
+    position: relative;
+    align-items: center;
+    justify-content: center;
+    font-size: 15px;
+    font-weight: 400;
+    text-transform: none;
+    padding: 6px 8px;
+    color: #000000; /* var(--colors-grey900) */
+    min-width: 64px;
+    box-sizing: border-box;
+    line-height: 1.75;
+    background: none;
+    border: none;
+    transition: background-color 250ms cubic-bezier(0.4, 0, 0.2, 1) 0ms;
+}
+
+.menuButton:hover:enabled {
+    background-color: rgba(0, 0, 0, 0.08); /* var(--colors-grey300) */
+}
+
+.menuButton:disabled {
+    color: #e0e0e0; /* var(--colors-grey400); */
+}
+.menuButton:focus {
+    outline: none;
+}
diff --git a/packages/app/src/components/VisualizationOptions/VisualizationOptionsManager.js b/packages/app/src/components/VisualizationOptions/VisualizationOptionsManager.js
index 744fae1394..60705dc7f2 100644
--- a/packages/app/src/components/VisualizationOptions/VisualizationOptionsManager.js
+++ b/packages/app/src/components/VisualizationOptions/VisualizationOptionsManager.js
@@ -1,8 +1,5 @@
 import React, { Component, Fragment } from 'react'
 import PropTypes from 'prop-types'
-
-import { default as MuiButton } from '@material-ui/core/Button'
-
 import i18n from '@dhis2/d2-i18n'
 import {
     ButtonStrip,
@@ -16,6 +13,7 @@ import UpdateButton from '../UpdateButton/UpdateButton'
 import HideButton from '../HideButton/HideButton'
 import UpdateVisualizationContainer from '../UpdateButton/UpdateVisualizationContainer'
 import VisualizationOptions from './VisualizationOptions'
+import MenuButton from '../MenuButton/MenuButton'
 
 class VisualizationOptionsManager extends Component {
     constructor(props) {
@@ -42,12 +40,12 @@ class VisualizationOptionsManager extends Component {
     render() {
         return (
             <Fragment>
-                <MuiButton
+                <MenuButton
                     className={this.props.className}
                     onClick={this.toggleVisualizationOptionsDialog}
                 >
                     {i18n.t('Options')}
-                </MuiButton>
+                </MenuButton>
                 {this.state.dialogIsOpen && (
                     <Modal onClose={this.onClose} position="top" large>
                         <ModalTitle>{i18n.t('Options')}</ModalTitle>