Skip to content

Commit

Permalink
#2287: replace selection list by simple list for change theme dialog (#…
Browse files Browse the repository at this point in the history
…2338)

* #2287: replace selection list by simple list for change theme dialog

* #2287: rework workspace type selection dialog

* closes dialog after selecting a theme, changes cursor

* #2287: beautify ChangeThemeDialog

* #2287: adds hoc for material-ui list, that provides keyboard navigation

* #2287: removes obsolete attributes

* #2287: removes test, that is already tested in ListKeyboardNavigation
  • Loading branch information
glefi authored Apr 8, 2019
1 parent a85ee17 commit e91995b
Show file tree
Hide file tree
Showing 14 changed files with 465 additions and 64 deletions.
37 changes: 37 additions & 0 deletions __tests__/src/components/ChangeThemeDialog.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import React from 'react';
import { shallow } from 'enzyme';
import { ChangeThemeDialog } from '../../../src/components/ChangeThemeDialog';

/**
* Helper function to create a shallow wrapper around ErrorDialog
*/
function createWrapper(props) {
return shallow(
<ChangeThemeDialog
classes={{ darkColor: '#000000', lightColor: '#ffffff' }}
handleClose={() => {}}
updateConfig={() => {}}
t={t => (t)}
theme="light"
{...props}
/>,
);
}

describe('ChangeThemeDialog', () => {
let wrapper;

it('renders propertly', () => {
wrapper = createWrapper();

expect(wrapper.find('WithStyles(Dialog)').length).toBe(1);
});

it('shows up theme selection properly', () => {
wrapper = createWrapper();

expect(wrapper.find('WithStyles(ListItemText)').length).toBe(2);
expect(wrapper.find('WithStyles(ListItemText)').first().render().text()).toBe('light');
expect(wrapper.find('WithStyles(ListItemText)').last().render().text()).toBe('dark');
});
});
14 changes: 5 additions & 9 deletions __tests__/src/components/WorkspaceSelectionDialog.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import React from 'react';
import { shallow } from 'enzyme';
import Select from '@material-ui/core/Select';
import { WorkspaceSelectionDialog } from '../../../src/components/WorkspaceSelectionDialog';
import settings from '../../../src/config/settings';

describe('WorkspaceSettings', () => {
let wrapper;
Expand All @@ -15,23 +13,21 @@ describe('WorkspaceSettings', () => {

wrapper = shallow(
<WorkspaceSelectionDialog
classes={{ listItem: {} }}
open
handleClose={handleClose}
updateConfig={updateConfig}
workspaceType={settings.workspace.type}
/>,
);
});

it('renders without an error', () => {
expect(wrapper.find('WithStyles(Dialog)').length).toBe(1);
expect(wrapper.find('WithStyles(FormControl)').length).toBe(1);
expect(wrapper.find('WithStyles(List)').length).toBe(1);
});

it('calls updateConfig updating the workspace type when selected', () => {
wrapper.find(Select).props().onChange({ target: { value: 'foo' } });
expect(updateConfig).toHaveBeenCalledWith({ workspace: { type: 'foo' } });
});
it('passes the current workspace type as the value prop to the Select', () => {
expect(wrapper.find(Select).props().value).toEqual('mosaic');
wrapper.find('WithStyles(ListItem)').first().simulate('click');
expect(updateConfig).toHaveBeenCalledWith({ workspace: { type: 'elastic' } });
});
});
81 changes: 81 additions & 0 deletions __tests__/src/lib/ListKeyboardNavigation.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import React from 'react';
import { shallow } from 'enzyme';
import { List, ListItem } from '@material-ui/core';
import { ListKeyboardNavigation } from '../../../src/lib/ListKeyboardNavigation';

const onChangeMock = jest.fn();
const mockEvent = { preventDefault: () => {} };
const onClickMock = jest.fn();
/**
* Helper function to create a shallow wrapper around LanguageSettings
*/
function createWrapper(props, childProps) {
return shallow(
<ListKeyboardNavigation
onChange={onChangeMock}
selected="listItem01"
{...props}
>
<ListItem value="listItem01" {...childProps} />
<ListItem value="listItem02" {...childProps} />
<ListItem value="listItem03" {...childProps} />
</ListKeyboardNavigation>,
);
}

describe('ListKeyboardNavigation', () => {
it('renders properly', () => {
const wrapper = createWrapper();
expect(wrapper.matchesElement(
<List />,
));
expect(wrapper.find('WithStyles(ListItem)').length).toBe(3);
});

it('the second item is selected, when passing its value as `selected`', () => {
const wrapper = createWrapper({ selected: 'listItem02' });
expect(wrapper.find('WithStyles(ListItem)[value="listItem02"]').first().prop('selected')).toBe(true);
});

it('pressing ArrowDown selects the next list item', () => {
const wrapper = createWrapper({ selected: 'listItem02' });
wrapper.simulate('keyDown', { key: 'ArrowDown', ...mockEvent });
expect(wrapper.find('WithStyles(ListItem)').last().prop('selected')).toBe(true);
});

it('pressing ArrowDown selects the first list item, when the last one is selected', () => {
const wrapper = createWrapper({ selected: 'listItem03' });
wrapper.simulate('keyDown', { key: 'ArrowDown', ...mockEvent });
expect(wrapper.find('WithStyles(ListItem)').first().prop('selected')).toBe(true);
});

it('pressing ArrowUp selects the previous list item', () => {
const wrapper = createWrapper({ selected: 'listItem02' });
wrapper.simulate('keyDown', { key: 'ArrowUp', ...mockEvent });
expect(wrapper.find('WithStyles(ListItem)').first().prop('selected')).toBe(true);
});

it('pressing ArrowUp selects the last list item, when the first one is selected', () => {
const wrapper = createWrapper({ selected: 'listItem01' });
wrapper.simulate('keyDown', { key: 'ArrowUp', ...mockEvent });
expect(wrapper.find('WithStyles(ListItem)').last().prop('selected')).toBe(true);
});

it('onChange handler is called when enter key is pressed', () => {
const wrapper = createWrapper();
wrapper.find('WithStyles(List)').first().simulate('keyDown', { key: 'Enter', ...mockEvent });
expect(onChangeMock).toBeCalledWith('listItem01');
});

it('onChange handler is called when clicking a list item', () => {
const wrapper = createWrapper();
wrapper.find('WithStyles(ListItem)').first().simulate('click');
expect(onChangeMock).toBeCalledWith('listItem01');
});

it('onClickHandler of list items is called besides onChange', () => {
const wrapper = createWrapper(undefined, { onClick: onClickMock });
wrapper.find('WithStyles(ListItem)').first().simulate('click');
expect(onClickMock).toBeCalledTimes(1);
});
});
93 changes: 93 additions & 0 deletions src/components/ChangeThemeDialog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import React, { Component } from 'react';
import {
Dialog,
DialogTitle,
ListItem,
ListItemIcon,
ListItemText,
Typography,
DialogContent,
} from '@material-ui/core';
import PaletteIcon from '@material-ui/icons/PaletteSharp';
import PropTypes from 'prop-types';
import { ListKeyboardNavigation } from '../lib/ListKeyboardNavigation';

/**
* a simple dialog providing the possibility to switch the theme
*/
export class ChangeThemeDialog extends Component {
/**
*/
constructor(props) {
super(props);
this.handleThemeChange = this.handleThemeChange.bind(this);
}

/**
* Propagate theme palette type selection into the global state
*/
handleThemeChange(theme) {
const { updateConfig, handleClose } = this.props;

updateConfig({
theme: {
palette: {
type: theme,
},
},
});
handleClose();
}

/** */
render() {
const {
classes,
handleClose,
open,
t,
theme,
} = this.props;
return (
<Dialog
onClose={handleClose}
open={open}
>
<DialogTitle id="change-the-dialog-title" disableTypography>
<Typography variant="h2">
{t('changeTheme')}
</Typography>
</DialogTitle>
<DialogContent className={classes.dialogContent}>
<ListKeyboardNavigation selected={theme} onChange={this.handleThemeChange}>
<ListItem className={classes.listitem} value="light">
<ListItemIcon>
<PaletteIcon className={classes.lightColor} />
</ListItemIcon>
<ListItemText>{t('light')}</ListItemText>
</ListItem>
<ListItem className={classes.listitem} value="dark">
<ListItemIcon>
<PaletteIcon className={classes.darkColor} />
</ListItemIcon>
<ListItemText>{t('dark')}</ListItemText>
</ListItem>
</ListKeyboardNavigation>
</DialogContent>
</Dialog>
);
}
}

ChangeThemeDialog.propTypes = {
classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
handleClose: PropTypes.func.isRequired,
open: PropTypes.bool,
t: PropTypes.func.isRequired,
theme: PropTypes.string.isRequired,
updateConfig: PropTypes.func.isRequired,
};

ChangeThemeDialog.defaultProps = {
open: false,
};
31 changes: 10 additions & 21 deletions src/components/WorkspaceMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ import ListItemIcon from '@material-ui/core/ListItemIcon';
import MenuItem from '@material-ui/core/MenuItem';
import Typography from '@material-ui/core/Typography';
import SaveAltIcon from '@material-ui/icons/SaveAltSharp';
import SettingsIcon from '@material-ui/icons/SettingsSharp';
import PropTypes from 'prop-types';
import LanguageSettings from '../containers/LanguageSettings';
import { NestedMenu } from './NestedMenu';
import WindowList from '../containers/WindowList';
import WorkspaceSettings from '../containers/WorkspaceSettings';
import WorkspaceSelectionDialog from '../containers/WorkspaceSelectionDialog';
import WorkspaceExport from '../containers/WorkspaceExport';
import WorkspaceImport from '../containers/WorkspaceImport';
import ns from '../config/css-ns';
import ChangeThemeDialog from '../containers/ChangeThemeDialog';

/**
*/
Expand All @@ -25,9 +24,9 @@ export class WorkspaceMenu extends Component {
constructor(props) {
super(props);
this.state = {
changeTheme: {},
exportWorkspace: {},
importWorkspace: {},
settings: {},
toggleZoom: {},
windowList: {},
workspaceSelection: {},
Expand Down Expand Up @@ -83,9 +82,9 @@ export class WorkspaceMenu extends Component {

const {
importWorkspace,
changeTheme,
windowList,
toggleZoom,
settings,
exportWorkspace,
workspaceSelection,
} = this.state;
Expand Down Expand Up @@ -137,17 +136,13 @@ export class WorkspaceMenu extends Component {
<NestedMenu label={t('language')}>
<LanguageSettings afterSelect={handleClose} />
</NestedMenu>

<MenuItem
aria-haspopup="true"
onClick={(e) => { this.handleMenuItemClick('settings', e); handleClose(e); }}
aria-owns={settings.AnchorEl ? 'workspace-settings' : undefined}
onClick={(e) => { this.handleMenuItemClick('changeTheme', e); handleClose(e); }}
aria-owns={changeTheme.anchorEl ? 'change-theme' : undefined}
divider
>
<ListItemIcon>
<SettingsIcon />
</ListItemIcon>
<Typography variant="body1">{t('settings')}</Typography>
<Typography variant="body1">{t('changeTheme')}</Typography>
</MenuItem>
<MenuItem
aria-haspopup="true"
Expand Down Expand Up @@ -178,17 +173,11 @@ export class WorkspaceMenu extends Component {
handleClose={this.handleMenuItemClose('windowList')}
/>
)}
{Boolean(toggleZoom.open) && (
<WorkspaceSettings
open={Boolean(toggleZoom.open)}
handleClose={this.handleMenuItemClose('toggleZoom')}
/>
)}
{Boolean(settings.open) && (
<WorkspaceSettings
open={Boolean(settings.open)}
{Boolean(changeTheme.open) && (
<ChangeThemeDialog
container={container}
handleClose={this.handleMenuItemClose('settings')}
handleClose={this.handleMenuItemClose('changeTheme')}
open={Boolean(changeTheme.open)}
/>
)}
{Boolean(workspaceSelection.open) && (
Expand Down
Loading

0 comments on commit e91995b

Please sign in to comment.