From ebb14ffc54670c8e2cbeba18eac238965eee4e81 Mon Sep 17 00:00:00 2001 From: Noah Loomans Date: Fri, 6 Jul 2018 16:14:30 +0200 Subject: client: Simplify setUser --- src/client/react/AppRouter.js | 33 +------------------ src/client/react/components/container/HelpBox.js | 2 +- src/client/react/components/container/Results.js | 20 +++--------- src/client/react/components/container/Search.js | 23 ++++++++++---- .../react/components/presentational/Results.js | 13 ++++---- .../react/components/presentational/Search.js | 11 ++++--- src/client/react/lib/url.js | 6 +++- src/client/react/store/actions.js | 14 ++------ src/client/react/store/reducers.js | 26 +++------------ src/client/react/store/reducers.test.js | 37 ++++++++++++---------- 10 files changed, 70 insertions(+), 115 deletions(-) diff --git a/src/client/react/AppRouter.js b/src/client/react/AppRouter.js index ae637f0..0384555 100644 --- a/src/client/react/AppRouter.js +++ b/src/client/react/AppRouter.js @@ -1,35 +1,15 @@ import React from 'react'; import { - withRouter, Route, Switch, Redirect, } from 'react-router-dom'; -import { connect } from 'react-redux'; -import { PropTypes } from 'prop-types'; import Index from './components/page/Index'; import User from './components/page/User'; -import { setUser } from './store/actions'; -import { userFromLocation } from './lib/url'; class AppRouter extends React.Component { - static propTypes = { - user: PropTypes.string, - resetUserState: PropTypes.func.isRequired, - } - - static defaultProps = { - user: null, - } - - componentDidMount() { - const { user, resetUserState } = this.props; - - resetUserState(user); - } - render() { return ( @@ -41,15 +21,4 @@ class AppRouter extends React.Component { } } -const mapStateToProps = (state, { location }) => { - return { - key: location.pathname, - user: userFromLocation(location), - }; -}; - -const mapDispatchToProps = dispatch => ({ - resetUserState: user => dispatch(setUser(user)), -}); - -export default withRouter(connect(mapStateToProps, mapDispatchToProps)(AppRouter)); +export default AppRouter; diff --git a/src/client/react/components/container/HelpBox.js b/src/client/react/components/container/HelpBox.js index 30b2942..d10c26b 100644 --- a/src/client/react/components/container/HelpBox.js +++ b/src/client/react/components/container/HelpBox.js @@ -22,7 +22,7 @@ import { connect } from 'react-redux'; import HelpBox from '../presentational/HelpBox'; const mapStateToProps = state => ({ - isVisible: state.search.results.length === 0 && state.search.text === '', + isVisible: !state.search || (state.search.results.length === 0 && state.search.text === ''), }); export default connect(mapStateToProps)(HelpBox); diff --git a/src/client/react/components/container/Results.js b/src/client/react/components/container/Results.js index 0d761d1..398adaa 100644 --- a/src/client/react/components/container/Results.js +++ b/src/client/react/components/container/Results.js @@ -19,27 +19,17 @@ */ import { connect } from 'react-redux'; -import { withRouter } from 'react-router-dom'; - -import users from '../../users'; -import { userFromMatch } from '../../lib/url'; import { setUser } from '../../store/actions'; import Results from '../presentational/Results'; -const mapStateToProps = (state, { match }) => { - const user = userFromMatch(match); - const searchText = state.search.text; - - return { - isExactMatch: user != null && searchText === users.byId[user].value, - results: state.search.results, - selectedResult: state.search.selected, - }; -}; +const mapStateToProps = state => ({ + results: state.search ? state.search.results : undefined, + selectedResult: state.search && state.search.selected, +}); const mapDispatchToProps = dispatch => ({ setUser: user => dispatch(setUser(user)), }); -export default withRouter(connect(mapStateToProps, mapDispatchToProps)(Results)); +export default connect(mapStateToProps, mapDispatchToProps)(Results); diff --git a/src/client/react/components/container/Search.js b/src/client/react/components/container/Search.js index b680fc6..73919d3 100644 --- a/src/client/react/components/container/Search.js +++ b/src/client/react/components/container/Search.js @@ -28,21 +28,32 @@ import users from '../../users'; const mapStateToProps = (state, { location }) => { const currentUser = userFromLocation(location); - const searchText = state.search.text; - const isExactMatch = currentUser != null && searchText === users.byId[currentUser].value; + const selectedUser = state.search && state.search.selected; + let searchText; + let isExactMatch; + + if (state.search) { + searchText = state.search.text; + isExactMatch = false; + } else if (currentUser) { + searchText = users.byId[currentUser].value; + isExactMatch = true; + } else { + searchText = ''; + isExactMatch = false; + } return { currentUser, + selectedUser, + searchText, isExactMatch, - selectedUser: state.search.selected, - results: state.search.results, - searchText: state.search.text, }; }; const mapDispatchToProps = dispatch => ({ setUser: user => dispatch(setUserAction(user)), - onInputChange: searchText => dispatch({ + changeInput: searchText => dispatch({ type: 'SEARCH/INPUT_CHANGE', searchText, }), diff --git a/src/client/react/components/presentational/Results.js b/src/client/react/components/presentational/Results.js index 173c644..4abd507 100644 --- a/src/client/react/components/presentational/Results.js +++ b/src/client/react/components/presentational/Results.js @@ -27,34 +27,35 @@ import './Results.scss'; class Results extends React.Component { static propTypes = { - results: PropTypes.arrayOf(PropTypes.string).isRequired, + results: PropTypes.arrayOf(PropTypes.string), selectedResult: PropTypes.string, - isExactMatch: PropTypes.bool.isRequired, setUser: PropTypes.func.isRequired, }; static defaultProps = { selectedResult: null, + results: [], }; render() { const { results, selectedResult, - isExactMatch, setUser, } = this.props; + const hasResults = results.length > 0; + return (
0, + hasResults, })} style={{ - minHeight: isExactMatch ? 0 : results.length * 54, + minHeight: hasResults ? results.length * 54 : 0, }} > - {!isExactMatch && results.map(userId => ( + {results.map(userId => (
} />
onInputChange(event.target.value)} + onChange={event => changeInput(event.target.value)} onKeyDown={event => this.handleKeyDown(event)} value={searchText} placeholder="Zoeken" diff --git a/src/client/react/lib/url.js b/src/client/react/lib/url.js index 752fec2..442e5eb 100644 --- a/src/client/react/lib/url.js +++ b/src/client/react/lib/url.js @@ -74,7 +74,11 @@ export function makeSetWeek(history) { export function makeUpdatePathname(history) { return function updatePathname(pathname) { const query = history.location.search; - history.push(`/${pathname}${query}`); + if (pathname) { + history.push(`/${pathname}${query}`); + } else { + history.push(`/${query}`); + } }; } diff --git a/src/client/react/store/actions.js b/src/client/react/store/actions.js index 36dc748..2d2cd30 100644 --- a/src/client/react/store/actions.js +++ b/src/client/react/store/actions.js @@ -4,18 +4,10 @@ import withinRange from '../lib/withinRange'; export function setUser(newUser) { return (dispatch, getState, { getHistory }) => { - const { user, updatePathname } = getHistory(); + const { updatePathname } = getHistory(); - if (newUser === user) { - // EDGE CASE: If the user that is being selected is equal to the user - // that is already selected, then updatePathname will not change - // anything. This results in state.search not properly being resetted. - // Example: If you search for 5H2 while already viewing 6-5H2, pressing - // enter won't do anything unless this check is present. - dispatch({ type: 'SEARCH/SET_USER', user }); - } else { - updatePathname(newUser || ''); - } + dispatch({ type: 'SEARCH/RESET' }); + updatePathname(newUser); }; } diff --git a/src/client/react/store/reducers.js b/src/client/react/store/reducers.js index 34c6d99..4a3576d 100644 --- a/src/client/react/store/reducers.js +++ b/src/client/react/store/reducers.js @@ -19,15 +19,10 @@ */ import getSearchResults from '../lib/getSearchResults'; -import users from '../users'; import withinRange from '../lib/withinRange'; const DEFAULT_STATE = { - search: { - results: [], - text: '', - selected: null, - }, + search: null, isRoomFinderVisible: false, schedules: {}, }; @@ -52,23 +47,10 @@ const schedule = (state = {}, action) => { function reducer(state = DEFAULT_STATE, action) { switch (action.type) { - case 'SEARCH/SET_USER': { - const { user } = action; - - if (user == null) { - return { - ...state, - search: DEFAULT_STATE.search, - }; - } - + case 'SEARCH/RESET': { return { ...state, - search: { - results: [], - text: users.byId[user].value, - selected: user, - }, + search: null, }; } @@ -86,7 +68,7 @@ function reducer(state = DEFAULT_STATE, action) { } case 'SEARCH/CHANGE_SELECTED_RESULT': { - if (state.search.results.length === 0) { + if (!state.search || state.search.results.length === 0) { return state; } diff --git a/src/client/react/store/reducers.test.js b/src/client/react/store/reducers.test.js index 3f6aff1..89cd438 100644 --- a/src/client/react/store/reducers.test.js +++ b/src/client/react/store/reducers.test.js @@ -39,26 +39,15 @@ describe('reducers', () => { deepFreeze(DEFAULT_STATE); }); - describe('SEARCH/SET_USER', () => { - it('Resets the search state if the user is null', () => { + describe('SEARCH/RESET', () => { + it('Resets the search state', () => { const prevState = { search: { foo: 'bar' } }; - const action = { type: 'SEARCH/SET_USER', user: null }; + const action = { type: 'SEARCH/RESET', user: null }; deepFreeze([prevState, action]); expect(reducer(prevState, action)).toEqual({ - search: DEFAULT_STATE.search, - }); - }); - - it('Sets all the values of that user properly', () => { - expect(reducer(undefined, { type: 'SEARCH/SET_USER', user: 's/18561' })).toEqual({ - ...DEFAULT_STATE, - search: { - results: [], - text: '18561', - selected: 's/18561', - }, + search: null, }); }); }); @@ -108,7 +97,7 @@ describe('reducers', () => { }); describe('SEARCH/CHANGE_SELECTED_RESULT', () => { - describe('State has no results', () => { + describe('State is empty', () => { it('Does nothing', () => { const actionPlus = { type: 'SEARCH/CHANGE_SELECTED_RESULT', relativeChange: +1 }; const actionMin = { type: 'SEARCH/CHANGE_SELECTED_RESULT', relativeChange: -1 }; @@ -122,6 +111,22 @@ describe('reducers', () => { }); }); + describe('State has no results', () => { + it('Does nothing', () => { + const state = { ...DEFAULT_STATE, search: { results: [] } }; + + const actionPlus = { type: 'SEARCH/CHANGE_SELECTED_RESULT', relativeChange: +1 }; + const actionMin = { type: 'SEARCH/CHANGE_SELECTED_RESULT', relativeChange: -1 }; + + deepFreeze([actionPlus, actionMin]); + + const nextStatePlus = reducer(state, actionPlus); + const nextStateMin = reducer(state, actionMin); + expect(nextStatePlus).toEqual(state); + expect(nextStateMin).toEqual(state); + }); + }); + describe('State has many results', () => { it('Switches to the correct selectedResult', () => { const prevState = { -- cgit v1.1