From c78e51cf07421405467efd8666d69f1eb6e92168 Mon Sep 17 00:00:00 2001 From: Chris Beer <cabeer@stanford.edu> Date: Mon, 11 Feb 2019 14:40:41 -0800 Subject: [PATCH] Move adding manifests to the workspace to a new component --- __tests__/src/actions/workspace.test.js | 9 ++ __tests__/src/components/App.test.js | 8 ++ __tests__/src/components/WorkspaceAdd.test.js | 32 +++++++ .../src/components/WorkspaceAddButton.test.js | 48 +++++----- __tests__/src/reducers/workspace.test.js | 8 ++ src/components/App.js | 11 ++- src/components/ManifestForm.js | 5 +- src/components/WorkspaceAdd.js | 43 +++++++++ src/components/WorkspaceAddButton.js | 91 +++---------------- src/containers/App.js | 1 + src/containers/WorkspaceAdd.js | 25 +++++ src/containers/WorkspaceAddButton.js | 11 ++- src/state/actions/action-types.js | 1 + src/state/actions/workspace.js | 10 ++ src/state/reducers/workspace.js | 2 + src/styles/index.scss | 4 + 16 files changed, 204 insertions(+), 105 deletions(-) create mode 100644 __tests__/src/components/WorkspaceAdd.test.js create mode 100644 src/components/WorkspaceAdd.js create mode 100644 src/containers/WorkspaceAdd.js diff --git a/__tests__/src/actions/workspace.test.js b/__tests__/src/actions/workspace.test.js index 5dcc6d681..587f6709b 100644 --- a/__tests__/src/actions/workspace.test.js +++ b/__tests__/src/actions/workspace.test.js @@ -40,4 +40,13 @@ describe('workspace actions', () => { expect(actions.toggleZoomControls(true)).toEqual(expectedAction); }); }); + describe('setWorkspaceAddVisibility', () => { + it('should set the workspace add visibility', () => { + const expectedAction = { + type: ActionTypes.SET_WORKSPACE_ADD_VISIBILITY, + isWorkspaceAddVisible: true, + }; + expect(actions.setWorkspaceAddVisibility(true)).toEqual(expectedAction); + }); + }); }); diff --git a/__tests__/src/components/App.test.js b/__tests__/src/components/App.test.js index 258bc47b4..74bfad6e4 100644 --- a/__tests__/src/components/App.test.js +++ b/__tests__/src/components/App.test.js @@ -4,6 +4,7 @@ import { MuiThemeProvider } from '@material-ui/core/styles'; import Fullscreen from 'react-fullscreen-crossbrowser'; import WorkspaceControlPanel from '../../../src/components/WorkspaceControlPanel'; import Workspace from '../../../src/containers/Workspace'; +import WorkspaceAdd from '../../../src/containers/WorkspaceAdd'; import App from '../../../src/components/App'; /** */ @@ -49,4 +50,11 @@ describe('App', () => { expect(wrapper.find(Fullscreen).first().prop('enabled')) .toEqual(true); }); + + describe('with isWorkspaceAddVisible', () => { + const wrapper = createWrapper({ isWorkspaceAddVisible: true }); + + expect(wrapper.find(Workspace).length).toBe(0); + expect(wrapper.find(WorkspaceAdd).length).toBe(1); + }); }); diff --git a/__tests__/src/components/WorkspaceAdd.test.js b/__tests__/src/components/WorkspaceAdd.test.js new file mode 100644 index 000000000..643bfffc1 --- /dev/null +++ b/__tests__/src/components/WorkspaceAdd.test.js @@ -0,0 +1,32 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import WorkspaceAdd from '../../../src/components/WorkspaceAdd'; +import fixture from '../../fixtures/version-2/002.json'; + +/** create wrapper */ +function createWrapper(props) { + return shallow( + <WorkspaceAdd + setWorkspaceAddVisibility={() => {}} + manifests={{ foo: fixture, bar: fixture }} + classes={{}} + t={str => str} + {...props} + />, + ); +} + +describe('WorkspaceAddButton', () => { + it('renders a list item for each manifest in the state', () => { + const wrapper = createWrapper(); + expect(wrapper.find('ul Connect(ManifestListItem)').length).toBe(2); + }); + + it('toggles the workspace visibility', () => { + const setWorkspaceAddVisibility = jest.fn(); + const wrapper = createWrapper({ setWorkspaceAddVisibility }); + + wrapper.find('ul Connect(ManifestListItem)').first().props().handleClose(); + expect(setWorkspaceAddVisibility).toHaveBeenCalledWith(false); + }); +}); diff --git a/__tests__/src/components/WorkspaceAddButton.test.js b/__tests__/src/components/WorkspaceAddButton.test.js index 3f17405eb..7ab45d043 100644 --- a/__tests__/src/components/WorkspaceAddButton.test.js +++ b/__tests__/src/components/WorkspaceAddButton.test.js @@ -1,33 +1,39 @@ import React from 'react'; import { shallow } from 'enzyme'; +import Fab from '@material-ui/core/Fab'; +import AddIcon from '@material-ui/icons/Add'; +import ClearIcon from '@material-ui/icons/Clear'; import WorkspaceAddButton from '../../../src/components/WorkspaceAddButton'; -import fixture from '../../fixtures/version-2/002.json'; -describe('WorkspaceAddButton', () => { - let wrapper; - beforeEach(() => { - wrapper = shallow( - <WorkspaceAddButton manifests={{ foo: fixture, bar: fixture }} classes={{}} />, - ).dive(); - }); +/** create wrapper */ +function createWrapper(props) { + return shallow( + <WorkspaceAddButton + classes={{}} + t={str => str} + {...props} + />, + ).dive(); // unwrap HOC created by withStyles() +} - it('renders a list item for each manifest in the state', () => { - expect(wrapper.find('ul Connect(ManifestListItem)').length).toBe(2); - }); +describe('WorkspaceAddButton', () => { + it('renders a button to open the load window area', () => { + const setWorkspaceAddVisibility = jest.fn(); + const wrapper = createWrapper({ isWorkspaceAddVisible: false, setWorkspaceAddVisibility }); - describe('handleAddManifestClick', () => { - it('sets the anchor state', () => { - wrapper.instance().handleAddManifestClick({ currentTarget: true }); + expect(wrapper.find(AddIcon).length).toBe(1); - expect(wrapper.dive().find('WithStyles(Menu)').props().open).toBe(true); - }); + wrapper.find(Fab).simulate('click'); + expect(setWorkspaceAddVisibility).toHaveBeenCalledWith(true); }); - describe('handleAddManifestClose', () => { - it('resets the anchor state', () => { - wrapper.instance().handleAddManifestClose(); + it('renders a button to close the load window area', () => { + const setWorkspaceAddVisibility = jest.fn(); + const wrapper = createWrapper({ isWorkspaceAddVisible: true, setWorkspaceAddVisibility }); + + expect(wrapper.find(ClearIcon).length).toBe(1); - expect(wrapper.dive().find('WithStyles(Menu)').props().open).toBe(false); - }); + wrapper.find(Fab).simulate('click'); + expect(setWorkspaceAddVisibility).toHaveBeenCalledWith(false); }); }); diff --git a/__tests__/src/reducers/workspace.test.js b/__tests__/src/reducers/workspace.test.js index 8b5732259..62c1c67df 100644 --- a/__tests__/src/reducers/workspace.test.js +++ b/__tests__/src/reducers/workspace.test.js @@ -34,4 +34,12 @@ describe('workspace reducer', () => { layout: { foo: 'bar' }, }); }); + it('should handle SET_WORKSPACE_ADD_VISIBILITY', () => { + expect(reducer([], { + type: ActionTypes.SET_WORKSPACE_ADD_VISIBILITY, + isWorkspaceAddVisible: true, + })).toEqual({ + isWorkspaceAddVisible: true, + }); + }); }); diff --git a/src/components/App.js b/src/components/App.js index 7b653149c..0bb2c1c48 100644 --- a/src/components/App.js +++ b/src/components/App.js @@ -5,6 +5,7 @@ import { MuiThemeProvider, createMuiTheme, withStyles } from '@material-ui/core/ import Fullscreen from 'react-fullscreen-crossbrowser'; import WorkspaceControlPanel from './WorkspaceControlPanel'; import Workspace from '../containers/Workspace'; +import WorkspaceAdd from '../containers/WorkspaceAdd'; import ns from '../config/css-ns'; /** @@ -32,7 +33,7 @@ class App extends Component { */ render() { const { - isFullscreenEnabled, setWorkspaceFullscreen, classes, + isFullscreenEnabled, setWorkspaceFullscreen, classes, isWorkspaceAddVisible, } = this.props; return ( @@ -42,7 +43,11 @@ class App extends Component { enabled={isFullscreenEnabled} onChange={setWorkspaceFullscreen} > - <Workspace /> + { + isWorkspaceAddVisible + ? <WorkspaceAdd /> + : <Workspace /> + } </Fullscreen> <WorkspaceControlPanel /> </MuiThemeProvider> @@ -56,10 +61,12 @@ App.propTypes = { isFullscreenEnabled: PropTypes.bool, // eslint-disable-line react/forbid-prop-types classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types, setWorkspaceFullscreen: PropTypes.func.isRequired, + isWorkspaceAddVisible: PropTypes.bool, }; App.defaultProps = { isFullscreenEnabled: false, + isWorkspaceAddVisible: false, }; /** Material UI style overrides diff --git a/src/components/ManifestForm.js b/src/components/ManifestForm.js index bc9e32513..f69f88a33 100644 --- a/src/components/ManifestForm.js +++ b/src/components/ManifestForm.js @@ -4,7 +4,6 @@ import PropTypes from 'prop-types'; /** * Provides a form for user input of a manifest url * @prop {Function} fetchManifest - * @prop {Function} setLastRequested */ class ManifestForm extends Component { /** @@ -26,11 +25,10 @@ class ManifestForm extends Component { * @private */ formSubmit(event) { - const { fetchManifest, setLastRequested } = this.props; + const { fetchManifest } = this.props; const { formValue } = this.state; event.preventDefault(); fetchManifest(formValue); - setLastRequested(formValue); } /** @@ -69,7 +67,6 @@ class ManifestForm extends Component { ManifestForm.propTypes = { fetchManifest: PropTypes.func.isRequired, - setLastRequested: PropTypes.func.isRequired, t: PropTypes.func, }; diff --git a/src/components/WorkspaceAdd.js b/src/components/WorkspaceAdd.js new file mode 100644 index 000000000..25461d64e --- /dev/null +++ b/src/components/WorkspaceAdd.js @@ -0,0 +1,43 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import ns from '../config/css-ns'; +import ManifestForm from '../containers/ManifestForm'; +import ManifestListItem from '../containers/ManifestListItem'; + +/** + * An area for managing manifests and adding them to workspace + * @memberof Workspace + * @private + */ +class WorkspaceAdd extends React.Component { + /** + * render + */ + render() { + const { manifests, setWorkspaceAddVisibility } = this.props; + + const manifestList = Object.keys(manifests).map(manifest => ( + <ManifestListItem + key={manifest} + manifest={manifest} + handleClose={() => setWorkspaceAddVisibility(false)} + /> + )); + + return ( + <div className={ns('workspace-add')}> + <ManifestForm + id="add-form" + /> + <ul>{manifestList}</ul> + </div> + ); + } +} + +WorkspaceAdd.propTypes = { + manifests: PropTypes.instanceOf(Object).isRequired, + setWorkspaceAddVisibility: PropTypes.func.isRequired, +}; + +export default WorkspaceAdd; diff --git a/src/components/WorkspaceAddButton.js b/src/components/WorkspaceAddButton.js index 7e37ef166..00de5550d 100644 --- a/src/components/WorkspaceAddButton.js +++ b/src/components/WorkspaceAddButton.js @@ -3,113 +3,50 @@ import PropTypes from 'prop-types'; import ListItem from '@material-ui/core/ListItem'; import Fab from '@material-ui/core/Fab'; import AddIcon from '@material-ui/icons/Add'; +import ClearIcon from '@material-ui/icons/Clear'; import { withStyles } from '@material-ui/core/styles'; -import Menu from '@material-ui/core/Menu'; -import ManifestForm from '../containers/ManifestForm'; -import ManifestListItem from '../containers/ManifestListItem'; /** */ class WorkspaceAddButton extends Component { - /** - * constructor - - */ - constructor(props) { - super(props); - this.state = { - lastRequested: '', - anchorEl: null, - }; - - this.handleAddManifestClick = this.handleAddManifestClick.bind(this); - this.handleAddManifestClose = this.handleAddManifestClose.bind(this); - this.setLastRequested = this.setLastRequested.bind(this); - } - - /** - * setLastRequested - Sets the state lastRequested - * - * @private - */ - setLastRequested(requested) { - this.setState({ - lastRequested: requested, - }); - } - - /** - * @private - */ - handleAddManifestClick(event) { - this.setState({ - anchorEl: event.currentTarget, - }); - } - - /** - * @private - */ - handleAddManifestClose() { - this.setState({ - anchorEl: null, - }); - } - /** * render * @return */ render() { - const { classes, t, manifests } = this.props; - const { lastRequested, anchorEl } = this.state; - - const manifestList = Object.keys(manifests).map(manifest => ( - <ManifestListItem - key={manifest} - manifest={manifest} - handleClose={this.handleAddManifestClose} - /> - )); - + const { + classes, t, setWorkspaceAddVisibility, isWorkspaceAddVisible, + } = this.props; return ( <ListItem> <Fab color="primary" id="addBtn" - aria-label={t('add')} + aria-label={isWorkspaceAddVisible ? t('closeWindow') : t('add')} className={classes.fab} - aria-owns={anchorEl ? 'add-form' : undefined} - aria-haspopup="true" - onClick={this.handleAddManifestClick} + onClick={() => { setWorkspaceAddVisibility(!isWorkspaceAddVisible); }} > - <AddIcon /> + { + isWorkspaceAddVisible + ? <ClearIcon /> + : <AddIcon /> + } </Fab> - <Menu - id="ws-ctrl-pnl-mn" - anchorEl={anchorEl} - open={Boolean(anchorEl)} - onClose={this.handleAddManifestClose} - > - <ManifestForm - id="add-form" - setLastRequested={this.setLastRequested} - /> - <ul>{manifestList}</ul> - {lastRequested} - </Menu> </ListItem> ); } } WorkspaceAddButton.propTypes = { - manifests: PropTypes.instanceOf(Object).isRequired, classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types t: PropTypes.func, + setWorkspaceAddVisibility: PropTypes.func.isRequired, + isWorkspaceAddVisible: PropTypes.bool, }; WorkspaceAddButton.defaultProps = { t: key => key, + isWorkspaceAddVisible: false, }; /** diff --git a/src/containers/App.js b/src/containers/App.js index 1806662ae..556413809 100644 --- a/src/containers/App.js +++ b/src/containers/App.js @@ -12,6 +12,7 @@ const mapStateToProps = state => ( { theme: state.config.theme, isFullscreenEnabled: state.workspace.isFullscreenEnabled, + isWorkspaceAddVisible: state.workspace.isWorkspaceAddVisible, } ); diff --git a/src/containers/WorkspaceAdd.js b/src/containers/WorkspaceAdd.js new file mode 100644 index 000000000..5fff5df95 --- /dev/null +++ b/src/containers/WorkspaceAdd.js @@ -0,0 +1,25 @@ +import { compose } from 'redux'; +import { connect } from 'react-redux'; +import * as actions from '../state/actions'; +import WorkspaceAdd from '../components/WorkspaceAdd'; + +/** + * mapStateToProps - to hook up connect + * @memberof Workspace + * @private + */ +const mapStateToProps = state => ({ manifests: state.manifests }); + +/** + * mapDispatchToProps - used to hook up connect to action creators + * @memberof Workspace + * @private + */ +const mapDispatchToProps = { setWorkspaceAddVisibility: actions.setWorkspaceAddVisibility }; + +const enhance = compose( + connect(mapStateToProps, mapDispatchToProps), + // further HOC go here +); + +export default enhance(WorkspaceAdd); diff --git a/src/containers/WorkspaceAddButton.js b/src/containers/WorkspaceAddButton.js index d7faf8218..b13dbd15f 100644 --- a/src/containers/WorkspaceAddButton.js +++ b/src/containers/WorkspaceAddButton.js @@ -1,6 +1,7 @@ import { connect } from 'react-redux'; import { compose } from 'redux'; import { withNamespaces } from 'react-i18next'; +import * as actions from '../state/actions'; import miradorWithPlugins from '../lib/miradorWithPlugins'; import WorkspaceAddButton from '../components/WorkspaceAddButton'; @@ -12,11 +13,19 @@ import WorkspaceAddButton from '../components/WorkspaceAddButton'; const mapStateToProps = state => ( { manifests: state.manifests, + isWorkspaceAddVisible: state.workspace.isWorkspaceAddVisible, } ); +/** + * mapDispatchToProps - used to hook up connect to action creators + * @memberof Workspace + * @private + */ +const mapDispatchToProps = { setWorkspaceAddVisibility: actions.setWorkspaceAddVisibility }; + const enhance = compose( - connect(mapStateToProps), + connect(mapStateToProps, mapDispatchToProps), withNamespaces(), miradorWithPlugins, ); diff --git a/src/state/actions/action-types.js b/src/state/actions/action-types.js index fb78df8cd..5620f3bea 100644 --- a/src/state/actions/action-types.js +++ b/src/state/actions/action-types.js @@ -14,6 +14,7 @@ const ActionTypes = { SET_CONFIG: 'SET_CONFIG', SET_WINDOW_THUMBNAIL_POSITION: 'SET_WINDOW_THUMBNAIL_POSITION', SET_WINDOW_VIEW_TYPE: 'SET_WINDOW_VIEW_TYPE', + SET_WORKSPACE_ADD_VISIBILITY: 'SET_WORKSPACE_ADD_VISIBILITY', TOGGLE_WINDOW_SIDE_BAR: 'TOGGLE_WINDOW_SIDE_BAR', TOGGLE_WINDOW_SIDE_BAR_PANEL: 'TOGGLE_WINDOW_SIDE_BAR_PANEL', TOGGLE_ZOOM_CONTROLS: 'TOGGLE_ZOOM_CONTROLS', diff --git a/src/state/actions/workspace.js b/src/state/actions/workspace.js index a5aeb61a6..cec07c965 100644 --- a/src/state/actions/workspace.js +++ b/src/state/actions/workspace.js @@ -29,3 +29,13 @@ export function toggleZoomControls(showZoomControls) { export function updateWorkspaceMosaicLayout(layout) { return { type: ActionTypes.UPDATE_WORKSPACE_MOSAIC_LAYOUT, layout }; } + +/** + * updateWorkspaceMosaicLayout - action creator + * + * @param {Object} layout + * @memberof ActionCreators + */ +export function setWorkspaceAddVisibility(isWorkspaceAddVisible) { + return { type: ActionTypes.SET_WORKSPACE_ADD_VISIBILITY, isWorkspaceAddVisible }; +} diff --git a/src/state/reducers/workspace.js b/src/state/reducers/workspace.js index 47ccdf7de..13a295f58 100644 --- a/src/state/reducers/workspace.js +++ b/src/state/reducers/workspace.js @@ -13,6 +13,8 @@ const workspaceReducer = (state = {}, action) => { return { ...state, showZoomControls: action.showZoomControls }; case ActionTypes.UPDATE_WORKSPACE_MOSAIC_LAYOUT: return { ...state, layout: action.layout }; + case ActionTypes.SET_WORKSPACE_ADD_VISIBILITY: + return { ...state, isWorkspaceAddVisible: action.isWorkspaceAddVisible }; default: return state; } diff --git a/src/styles/index.scss b/src/styles/index.scss index cd70bf40f..0de619730 100644 --- a/src/styles/index.scss +++ b/src/styles/index.scss @@ -19,6 +19,10 @@ top: 0; } + &-workspace-add { + padding-left: 100px; + } + &-window-middle-content { display: flex; flex: 1; -- GitLab