diff --git a/__tests__/src/components/WindowThumbnailSettings.test.js b/__tests__/src/components/WindowThumbnailSettings.test.js index 807b3e40de5c40d567437eb34df16f42d986814d..1777fc508f01e5e05e3e42132bc6432c44fad0d4 100644 --- a/__tests__/src/components/WindowThumbnailSettings.test.js +++ b/__tests__/src/components/WindowThumbnailSettings.test.js @@ -1,14 +1,15 @@ import React from 'react'; import { shallow } from 'enzyme'; import FormControlLabel from '@material-ui/core/FormControlLabel'; -import RadioGroup from '@material-ui/core/RadioGroup'; -import Typography from '@material-ui/core/Typography'; +import ListSubheader from '@material-ui/core/ListSubheader'; +import MenuItem from '@material-ui/core/MenuItem'; import { WindowThumbnailSettings } from '../../../src/components/WindowThumbnailSettings'; /** create wrapper */ function createWrapper(props) { return shallow( <WindowThumbnailSettings + classes={{}} windowId="xyz" setWindowThumbnailPosition={() => {}} thumbnailNavigationPosition="off" @@ -20,8 +21,7 @@ function createWrapper(props) { describe('WindowThumbnailSettings', () => { it('renders all elements correctly', () => { const wrapper = createWrapper(); - expect(wrapper.find(Typography).length).toBe(1); - expect(wrapper.find(RadioGroup).length).toBe(1); + expect(wrapper.find(ListSubheader).length).toBe(1); const labels = wrapper.find(FormControlLabel); expect(labels.length).toBe(3); expect(labels.at(0).props().value).toBe('off'); @@ -29,19 +29,22 @@ describe('WindowThumbnailSettings', () => { expect(labels.at(2).props().value).toBe('far-right'); }); - it('should set the correct label active', () => { + it('should set the correct label active (by setting the secondary color)', () => { let wrapper = createWrapper({ thumbnailNavigationPosition: 'far-bottom' }); - expect(wrapper.find(RadioGroup).props().value).toBe('far-bottom'); + expect(wrapper.find(FormControlLabel).at(1).props().control.props.color).toEqual('secondary'); + expect(wrapper.find(FormControlLabel).at(2).props().control.props.color).not.toEqual('secondary'); + wrapper = createWrapper({ thumbnailNavigationPosition: 'far-right' }); - expect(wrapper.find(RadioGroup).props().value).toBe('far-right'); + expect(wrapper.find(FormControlLabel).at(2).props().control.props.color).toEqual('secondary'); }); it('updates state when the thumbnail config selection changes', () => { const setWindowThumbnailPosition = jest.fn(); const wrapper = createWrapper({ setWindowThumbnailPosition }); - wrapper.find(RadioGroup).first().simulate('change', { target: { value: 'off' } }); + + wrapper.find(MenuItem).at(0).simulate('click'); expect(setWindowThumbnailPosition).toHaveBeenCalledWith('xyz', 'off'); - wrapper.find(RadioGroup).first().simulate('change', { target: { value: 'far-right' } }); + wrapper.find(MenuItem).at(2).simulate('click'); expect(setWindowThumbnailPosition).toHaveBeenCalledWith('xyz', 'far-right'); }); }); diff --git a/__tests__/src/components/WindowTopMenu.test.js b/__tests__/src/components/WindowTopMenu.test.js index a49b4638178ce3ba76c7abe738f0fd77cfe979e5..6a8e7292e86e731f021307adcbb2633765bb1d22 100644 --- a/__tests__/src/components/WindowTopMenu.test.js +++ b/__tests__/src/components/WindowTopMenu.test.js @@ -1,6 +1,5 @@ import React from 'react'; import { shallow } from 'enzyme'; -import ListItem from '@material-ui/core/ListItem'; import Menu from '@material-ui/core/Menu'; import WindowThumbnailSettings from '../../../src/containers/WindowThumbnailSettings'; import WindowViewSettings from '../../../src/containers/WindowViewSettings'; @@ -23,7 +22,6 @@ describe('WindowTopMenu', () => { it('renders all needed elements', () => { const wrapper = createWrapper(); expect(wrapper.find(Menu).length).toBe(1); - expect(wrapper.find(ListItem).length).toBe(2); expect(wrapper.find(WindowThumbnailSettings).length).toBe(1); expect(wrapper.find(WindowViewSettings).length).toBe(1); }); diff --git a/__tests__/src/components/WindowViewSettings.test.js b/__tests__/src/components/WindowViewSettings.test.js index 7900f51238b6f394f40322629689dd38217bcfe0..5a1fcc2572d9f3f14e7c5e693295665cf79db65f 100644 --- a/__tests__/src/components/WindowViewSettings.test.js +++ b/__tests__/src/components/WindowViewSettings.test.js @@ -1,14 +1,15 @@ import React from 'react'; -import { shallow } from 'enzyme'; +import { mount, shallow } from 'enzyme'; import FormControlLabel from '@material-ui/core/FormControlLabel'; -import RadioGroup from '@material-ui/core/RadioGroup'; -import Typography from '@material-ui/core/Typography'; +import ListSubheader from '@material-ui/core/ListSubheader'; +import MenuItem from '@material-ui/core/MenuItem'; import { WindowViewSettings } from '../../../src/components/WindowViewSettings'; /** create wrapper */ function createWrapper(props) { return shallow( <WindowViewSettings + classes={{}} windowId="xyz" setWindowViewType={() => {}} windowViewType="single" @@ -20,27 +21,55 @@ function createWrapper(props) { describe('WindowViewSettings', () => { it('renders all elements correctly', () => { const wrapper = createWrapper(); - expect(wrapper.find(Typography).length).toBe(1); - expect(wrapper.find(RadioGroup).length).toBe(1); + expect(wrapper.find(ListSubheader).length).toBe(1); const labels = wrapper.find(FormControlLabel); expect(labels.length).toBe(2); expect(labels.at(0).props().value).toBe('single'); expect(labels.at(1).props().value).toBe('book'); }); - it('should set the correct label active', () => { + it('should set the correct label active (by setting the secondary color)', () => { let wrapper = createWrapper({ windowViewType: 'single' }); - expect(wrapper.find(RadioGroup).props().value).toBe('single'); + expect(wrapper.find(FormControlLabel).at(0).props().control.props.color).toEqual('secondary'); + expect(wrapper.find(FormControlLabel).at(1).props().control.props.color).not.toEqual('secondary'); + wrapper = createWrapper({ windowViewType: 'book' }); - expect(wrapper.find(RadioGroup).props().value).toBe('book'); + expect(wrapper.find(FormControlLabel).at(1).props().control.props.color).toEqual('secondary'); }); it('updates state when the view config selection changes', () => { const setWindowViewType = jest.fn(); const wrapper = createWrapper({ setWindowViewType }); - wrapper.find(RadioGroup).first().simulate('change', { target: { value: 'book' } }); - expect(setWindowViewType).toHaveBeenCalledWith('xyz', 'book'); - wrapper.find(RadioGroup).first().simulate('change', { target: { value: 'single' } }); + wrapper.find(MenuItem).at(0).simulate('click'); expect(setWindowViewType).toHaveBeenCalledWith('xyz', 'single'); + wrapper.find(MenuItem).at(1).simulate('click'); + expect(setWindowViewType).toHaveBeenCalledWith('xyz', 'book'); + }); + + it('sets the selected ref to a MenuItem in the component (when mounting)', () => { + const wrapper = mount( + <WindowViewSettings + classes={{}} + windowId="xyz" + setWindowViewType={() => {}} + windowViewType="single" + />, + ); + + expect( + wrapper // eslint-disable-line no-underscore-dangle + .instance() + .selectedRef + ._reactInternalFiber + .type + .displayName, + ).toEqual('WithStyles(MenuItem)'); + + // The document's ActiveElement is an li + expect( + document + .activeElement[Object.keys(document.activeElement)[0]] + .elementType, + ).toEqual('li'); }); }); diff --git a/src/components/WindowThumbnailSettings.js b/src/components/WindowThumbnailSettings.js index 3ea45d60bfc1f941e6dbf0de3dc77911ccd8ff87..9492546ebd2459b4926feb830ccead831dddcd30 100644 --- a/src/components/WindowThumbnailSettings.js +++ b/src/components/WindowThumbnailSettings.js @@ -1,8 +1,7 @@ import React, { Component } from 'react'; import FormControlLabel from '@material-ui/core/FormControlLabel'; -import Radio from '@material-ui/core/Radio'; -import RadioGroup from '@material-ui/core/RadioGroup'; -import Typography from '@material-ui/core/Typography'; +import ListSubheader from '@material-ui/core/ListSubheader'; +import MenuItem from '@material-ui/core/MenuItem'; import ThumbnailsOffIcon from '@material-ui/icons/CropDinSharp'; import PropTypes from 'prop-types'; import ThumbnailNavigationBottomIcon from './icons/ThumbnailNavigationBottomIcon'; @@ -22,10 +21,10 @@ export class WindowThumbnailSettings extends Component { /** * @private */ - handleChange(event) { + handleChange(value) { const { windowId, setWindowThumbnailPosition } = this.props; - setWindowThumbnailPosition(windowId, event.target.value); + setWindowThumbnailPosition(windowId, value); } /** @@ -34,42 +33,61 @@ export class WindowThumbnailSettings extends Component { * @return {type} description */ render() { - const { thumbnailNavigationPosition, t } = this.props; + const { + classes, handleClose, t, thumbnailNavigationPosition, + } = this.props; return ( <> - <Typography>{t('thumbnails')}</Typography> - <RadioGroup aria-label={t('position')} name="position" value={thumbnailNavigationPosition} onChange={this.handleChange} row> + <ListSubheader role="presentation" tabIndex="-1">{t('thumbnails')}</ListSubheader> + + <MenuItem className={classes.MenuItem} onClick={() => { this.handleChange('off'); handleClose(); }}> <FormControlLabel value="off" - control={<Radio color="secondary" icon={<ThumbnailsOffIcon />} checkedIcon={<ThumbnailsOffIcon />} />} + classes={{ label: thumbnailNavigationPosition === 'off' ? classes.selectedLabel : undefined }} + control={ + <ThumbnailsOffIcon color={thumbnailNavigationPosition === 'off' ? 'secondary' : undefined} /> + } label={t('off')} labelPlacement="bottom" /> + </MenuItem> + <MenuItem className={classes.MenuItem} onClick={() => { this.handleChange('far-bottom'); handleClose(); }}> <FormControlLabel value="far-bottom" - control={<Radio color="secondary" icon={<ThumbnailNavigationBottomIcon />} checkedIcon={<ThumbnailNavigationBottomIcon />} />} + classes={{ label: thumbnailNavigationPosition === 'far-bottom' ? classes.selectedLabel : undefined }} + control={ + <ThumbnailNavigationBottomIcon color={thumbnailNavigationPosition === 'far-bottom' ? 'secondary' : undefined} /> + } label={t('bottom')} labelPlacement="bottom" /> + </MenuItem> + <MenuItem className={classes.MenuItem} onClick={() => { this.handleChange('far-right'); handleClose(); }}> <FormControlLabel value="far-right" - control={<Radio color="secondary" icon={<ThumbnailNavigationRightIcon />} checkedIcon={<ThumbnailNavigationRightIcon />} />} + classes={{ label: thumbnailNavigationPosition === 'far-right' ? classes.selectedLabel : undefined }} + control={ + <ThumbnailNavigationRightIcon color={thumbnailNavigationPosition === 'far-right' ? 'secondary' : undefined} /> + } label={t('right')} labelPlacement="bottom" /> - </RadioGroup> + </MenuItem> </> ); } } WindowThumbnailSettings.propTypes = { + classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types + handleClose: PropTypes.func, windowId: PropTypes.string.isRequired, setWindowThumbnailPosition: PropTypes.func.isRequired, thumbnailNavigationPosition: PropTypes.string.isRequired, t: PropTypes.func, }; WindowThumbnailSettings.defaultProps = { + handleClose: () => {}, t: key => key, }; diff --git a/src/components/WindowTopMenu.js b/src/components/WindowTopMenu.js index 011c4f68da5eba7ca58e4190ffa4573554290fa9..d8622f15013a8bc5670ab141a28c96b139b2dcef 100644 --- a/src/components/WindowTopMenu.js +++ b/src/components/WindowTopMenu.js @@ -1,5 +1,4 @@ import React, { Component } from 'react'; -import ListItem from '@material-ui/core/ListItem'; import Menu from '@material-ui/core/Menu'; import PropTypes from 'prop-types'; import WindowThumbnailSettings from '../containers/WindowThumbnailSettings'; @@ -36,13 +35,10 @@ export class WindowTopMenu extends Component { getContentAnchorEl={null} open={Boolean(anchorEl)} onClose={handleClose} + disableAutoFocusItem > - <ListItem divider> - <WindowViewSettings windowId={windowId} /> - </ListItem> - <ListItem divider> - <WindowThumbnailSettings windowId={windowId} /> - </ListItem> + <WindowViewSettings windowId={windowId} handleClose={handleClose} /> + <WindowThumbnailSettings windowId={windowId} handleClose={handleClose} /> </Menu> </> ); diff --git a/src/components/WindowViewSettings.js b/src/components/WindowViewSettings.js index 729b4ee0a232de4774e27430800f4c8028cf01af..59f2b085bb9c681a4a7de4175edb4a890e8a40e5 100644 --- a/src/components/WindowViewSettings.js +++ b/src/components/WindowViewSettings.js @@ -1,8 +1,8 @@ import React, { Component } from 'react'; +import ReactDOM from 'react-dom'; import FormControlLabel from '@material-ui/core/FormControlLabel'; -import Radio from '@material-ui/core/Radio'; -import RadioGroup from '@material-ui/core/RadioGroup'; -import Typography from '@material-ui/core/Typography'; +import MenuItem from '@material-ui/core/MenuItem'; +import ListSubheader from '@material-ui/core/ListSubheader'; import SingleIcon from '@material-ui/icons/CropOriginalSharp'; import PropTypes from 'prop-types'; import BookViewIcon from './icons/BookViewIcon'; @@ -19,13 +19,32 @@ export class WindowViewSettings extends Component { this.handleChange = this.handleChange.bind(this); } + /** + * Take action when the component mounts for the first time + */ + componentDidMount() { + if (this.selectedRef) { + // MUI uses ReactDOM.findDOMNode and refs for handling focus + ReactDOM.findDOMNode(this.selectedRef).focus(); // eslint-disable-line react/no-find-dom-node + } + } + /** * @private */ - handleChange(event) { + handleSelectedRef(ref) { + if (this.selectedRef) return; + + this.selectedRef = ref; + } + + /** + * @private + */ + handleChange(value) { const { windowId, setWindowViewType } = this.props; - setWindowViewType(windowId, event.target.value); + setWindowViewType(windowId, value); } /** @@ -34,36 +53,50 @@ export class WindowViewSettings extends Component { * @return {type} description */ render() { - const { windowViewType, t } = this.props; + const { + classes, handleClose, t, windowViewType, + } = this.props; return ( <> - <Typography>{t('view')}</Typography> - <RadioGroup aria-label={t('position')} name="position" value={windowViewType} onChange={this.handleChange} row> + <ListSubheader role="presentation" tabIndex="-1">{t('view')}</ListSubheader> + + <MenuItem + className={classes.MenuItem} + ref={ref => this.handleSelectedRef(ref)} + onClick={() => { this.handleChange('single'); handleClose(); }} + > <FormControlLabel value="single" - control={<Radio color="secondary" icon={<SingleIcon />} checkedIcon={<SingleIcon />} />} + classes={{ label: windowViewType === 'single' ? classes.selectedLabel : undefined }} + control={<SingleIcon color={windowViewType === 'single' ? 'secondary' : undefined} />} label={t('single')} labelPlacement="bottom" /> + </MenuItem> + <MenuItem className={classes.MenuItem} onClick={() => { this.handleChange('book'); handleClose(); }}> <FormControlLabel value="book" - control={<Radio color="secondary" icon={<BookViewIcon />} checkedIcon={<BookViewIcon />} />} + classes={{ label: windowViewType === 'book' ? classes.selectedLabel : undefined }} + control={<BookViewIcon color={windowViewType === 'book' ? 'secondary' : undefined} />} label={t('book')} labelPlacement="bottom" /> - </RadioGroup> + </MenuItem> </> ); } } WindowViewSettings.propTypes = { + classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types + handleClose: PropTypes.func, windowId: PropTypes.string.isRequired, setWindowViewType: PropTypes.func.isRequired, windowViewType: PropTypes.string.isRequired, t: PropTypes.func, }; WindowViewSettings.defaultProps = { + handleClose: () => {}, t: key => key, }; diff --git a/src/containers/WindowThumbnailSettings.js b/src/containers/WindowThumbnailSettings.js index c796e5868525d7e46d688d25d979a5bc74a0d097..04d4788226c4730b501e4c992da91745a6d16c53 100644 --- a/src/containers/WindowThumbnailSettings.js +++ b/src/containers/WindowThumbnailSettings.js @@ -1,6 +1,7 @@ import { compose } from 'redux'; import { connect } from 'react-redux'; import { withTranslation } from 'react-i18next'; +import { withStyles } from '@material-ui/core/styles'; import * as actions from '../state/actions'; import { getThumbnailNavigationPosition } from '../state/selectors'; import { WindowThumbnailSettings } from '../components/WindowThumbnailSettings'; @@ -23,7 +24,18 @@ const mapStateToProps = (state, props) => ( } ); +/** */ +const styles = theme => ({ + selectedLabel: { + color: theme.palette.secondary.main, + }, + MenuItem: { + display: 'inline', + }, +}); + const enhance = compose( + withStyles(styles), withTranslation(), connect(mapStateToProps, mapDispatchToProps), // further HOC go here diff --git a/src/containers/WindowViewSettings.js b/src/containers/WindowViewSettings.js index e2c12f72f3d3513bf0d2ea66b7cc103e81f3b013..002b0d04af532040889ad388ef43f536a2bd2f90 100644 --- a/src/containers/WindowViewSettings.js +++ b/src/containers/WindowViewSettings.js @@ -1,6 +1,7 @@ import { compose } from 'redux'; import { connect } from 'react-redux'; import { withTranslation } from 'react-i18next'; +import { withStyles } from '@material-ui/core/styles'; import * as actions from '../state/actions'; import { getWindowViewType } from '../state/selectors'; import { WindowViewSettings } from '../components/WindowViewSettings'; @@ -23,7 +24,18 @@ const mapStateToProps = (state, props) => ( } ); +/** */ +const styles = theme => ({ + selectedLabel: { + color: theme.palette.secondary.main, + }, + MenuItem: { + display: 'inline', + }, +}); + const enhance = compose( + withStyles(styles), withTranslation(), connect(mapStateToProps, mapDispatchToProps), );