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.
dhis2 · Apr 21, 2020 · 45b3adb · 45b3adb
1 parent 7bc25f7
commit 45b3adb
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 65 deletions.
diff --git 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
@@ -2,14 +2,14 @@ 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'
 
 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
@@ -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
@@ -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
@@ -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
diff --git a/packages/app/src/components/MenuButton/MenuButton.js 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
@@ -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
@@ -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>