-
Notifications
You must be signed in to change notification settings - Fork 397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redux with stub data (fixes #8) #75
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
const initialState = { | ||
wat: {slug: 'wat', title: 'Wat is this?'}, | ||
foo: {slug: 'foo', title: 'The foo add-on'}, | ||
food: {slug: 'food', title: 'Find food'}, | ||
bar: {slug: 'bar', title: 'The bar add-on'}, | ||
}; | ||
|
||
export default function addon(state = initialState, action) { | ||
switch (action.type) { | ||
case 'ADDON_FETCHED': | ||
return Object.assign({}, state, {[action.addon.slug]: action.addon}); | ||
default: | ||
return state; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export function setQuery(query) { | ||
return { | ||
type: 'SET_QUERY', | ||
query, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,15 @@ | ||
import React from 'react'; | ||
|
||
import SearchForm from './SearchForm'; | ||
import SearchResults from './SearchResults'; | ||
import { gettext as _ } from 'core/utils'; | ||
|
||
export default class App extends React.Component { | ||
|
||
state = { | ||
query: null, | ||
results: [], | ||
} | ||
|
||
handleSearch = (query) => { | ||
const results = [{title: 'Foo'}, {title: 'Bar'}, {title: 'Baz'}]; | ||
this.setState({ query, results }); | ||
} | ||
|
||
render() { | ||
const { query, results } = this.state; | ||
return ( | ||
<div className="search-app"> | ||
<h1>{_('Add-on Search')}</h1> | ||
<SearchForm onSearch={this.handleSearch} /> | ||
<SearchResults results={results} query={query} /> | ||
</div> | ||
); | ||
} | ||
} | ||
import { Provider } from 'react-redux'; | ||
import { createStore, combineReducers } from 'redux'; | ||
import CurrentSearchPage from '../containers/CurrentSearchPage'; | ||
import search from '../reducers/search'; | ||
import addons from 'core/reducers/addons'; | ||
const store = createStore(combineReducers({addons, search})); | ||
|
||
const App = () => ( | ||
<Provider store={store}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This redux stuff normally seems to go in an index.js file but we want to split it out per app. I'm not sure if this is the best place for it but apparently it works. |
||
<CurrentSearchPage /> | ||
</Provider> | ||
); | ||
|
||
export default App; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import React, { PropTypes } from 'react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this file should be rolled into the container rather than be separate? Although there's going to be pros and cons for both approaches e.g. separation + testing being easer etc... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did feel like I was being forced to write it this way a little. The docs suggest that you don't have any redux in your
From Usage with React (doesn't support linking very well, because JavaScript) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That all seems like solid reasoning to keep it as is then 👍 |
||
|
||
import SearchForm from './SearchForm'; | ||
import SearchResults from './SearchResults'; | ||
import { gettext as _ } from 'core/utils'; | ||
|
||
const SearchPage = ({ handleSearch, results, query }) => ( | ||
<div className="search-page"> | ||
<h1>{_('Add-on Search')}</h1> | ||
<SearchForm onSearch={handleSearch} /> | ||
<SearchResults results={results} query={query} /> | ||
</div> | ||
); | ||
|
||
SearchPage.propTypes = { | ||
handleSearch: PropTypes.func.isRequired, | ||
results: PropTypes.arrayOf(PropTypes.shape({ | ||
slug: PropTypes.string.isRequired, | ||
title: PropTypes.string.isRequired, | ||
})), | ||
query: PropTypes.string, | ||
}; | ||
|
||
export default SearchPage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { connect } from 'react-redux'; | ||
import SearchPage from '../components/SearchPage'; | ||
import { setQuery } from '../actions'; | ||
import { getMatchingAddons } from 'search/reducers/search'; | ||
|
||
export function mapStateToProps(state) { | ||
return { | ||
results: getMatchingAddons(state.addons, state.search.query), | ||
query: state.search.query, | ||
}; | ||
} | ||
|
||
export function mapDispatchToProps(dispatch) { | ||
return { | ||
handleSearch: (query) => dispatch(setQuery(query)), | ||
}; | ||
} | ||
|
||
const CurrentSearchPage = connect( | ||
mapStateToProps, | ||
mapDispatchToProps, | ||
)(SearchPage); | ||
|
||
export default CurrentSearchPage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
const initialState = { | ||
query: null, | ||
}; | ||
|
||
export default function search(state = initialState, action) { | ||
switch (action.type) { | ||
case 'SET_QUERY': | ||
return Object.assign({}, state, {query: action.query}); | ||
default: | ||
return state; | ||
} | ||
} | ||
|
||
export function getMatchingAddons(addons, query) { | ||
const matches = []; | ||
for (const slug in addons) { | ||
if (addons[slug].title.indexOf(query) >= 0) { | ||
matches.push(addons[slug]); | ||
} | ||
} | ||
return matches; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
require('babel-polyfill'); | ||
|
||
const testsContext = require.context('./tests', true, /\.js$/); | ||
const componentsContext = require.context('./src/', true, /components\/.*\.js$/); | ||
const componentsContext = require.context( | ||
'./src/', true, /(actions|components|containers|reducers)\/.*\.js$/); | ||
|
||
testsContext.keys().forEach(testsContext); | ||
componentsContext.keys().forEach(componentsContext); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import addons from 'core/reducers/addons'; | ||
|
||
describe('addon reducer', () => { | ||
it('returns the old state', () => { | ||
const originalState = {foo: {slug: 'foo'}, bar: {slug: 'bar'}}; | ||
assert.strictEqual(originalState, addons(originalState, {type: 'BLAH'})); | ||
}); | ||
|
||
it('adds an addon', () => { | ||
const originalState = {foo: {slug: 'foo'}}; | ||
const baz = {slug: 'baz'}; | ||
const expectedState = {foo: {slug: 'foo'}, baz}; | ||
const newState = addons(originalState, {type: 'ADDON_FETCHED', addon: baz}); | ||
assert.notStrictEqual(originalState, newState); | ||
assert.deepEqual(expectedState, newState); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import React from 'react'; | ||
import { createRenderer } from 'react-addons-test-utils'; | ||
|
||
import SearchPage from 'search/components/SearchPage'; | ||
import SearchResults from 'search/components/SearchResults'; | ||
import SearchForm from 'search/components/SearchForm'; | ||
|
||
function findByTag(root, tag) { | ||
const matches = root.props.children.filter((child) => child.type === tag); | ||
assert.equal(matches.length, 1, 'expected one match'); | ||
return matches[0]; | ||
} | ||
|
||
describe('<SearchPage />', () => { | ||
let root; | ||
let state; | ||
|
||
beforeEach(() => { | ||
state = { | ||
handleSearch: sinon.spy(), | ||
results: [{title: 'Foo', slug: 'foo'}, {title: 'Bar', slug: 'bar'}], | ||
query: 'foo', | ||
}; | ||
// eslint-disable-next-line new-cap | ||
const renderer = createRenderer(); | ||
renderer.render(<SearchPage {...state} />); | ||
root = renderer.getRenderOutput(); | ||
}); | ||
|
||
it('has a nice heading', () => { | ||
const heading = findByTag(root, 'h1'); | ||
assert.equal(heading.props.children, 'Add-on Search'); | ||
}); | ||
|
||
it('renders the results', () => { | ||
const results = findByTag(root, SearchResults); | ||
assert.strictEqual(results.props.results, state.results); | ||
assert.strictEqual(results.props.query, state.query); | ||
assert.deepEqual( | ||
Object.keys(results.props).sort(), | ||
['results', 'query'].sort()); | ||
}); | ||
|
||
it('renders the query', () => { | ||
const form = findByTag(root, SearchForm); | ||
assert.strictEqual(form.props.onSearch, state.handleSearch); | ||
assert.deepEqual(Object.keys(form.props), ['onSearch']); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { mapStateToProps } from 'search/containers/CurrentSearchPage'; | ||
|
||
describe('CurrentSearchPage.mapStateToProps', () => { | ||
const props = mapStateToProps({ | ||
addons: {ab: {slug: 'ab', title: 'ad-block'}, | ||
cd: {slug: 'cd', title: 'cd-block'}}, | ||
search: {query: 'ad-block'}, | ||
}); | ||
|
||
it('passes the query through', () => { | ||
assert.equal(props.query, 'ad-block'); | ||
}); | ||
|
||
it('filters the add-ons', () => { | ||
assert.deepEqual(props.results, [{slug: 'ab', title: 'ad-block'}]); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import search, { getMatchingAddons } from 'search/reducers/search'; | ||
|
||
describe('search reducer', () => { | ||
it('defaults to a null query', () => { | ||
const { query } = search(undefined, {type: 'unrelated'}); | ||
assert.strictEqual(query, null); | ||
}); | ||
|
||
it('sets the query on SET_QUERY', () => { | ||
const state = search(undefined, {type: 'SET_QUERY', query: 'foo'}); | ||
assert.equal(state.query, 'foo'); | ||
const newState = search(state, {type: 'SET_QUERY', query: 'bar'}); | ||
assert.equal(newState.query, 'bar'); | ||
}); | ||
}); | ||
|
||
describe('getMatchingAddons', () => { | ||
it('matches on the title', () => { | ||
assert.deepEqual( | ||
getMatchingAddons( | ||
[{title: 'Foo'}, {title: 'Bar'}, {title: 'Bar Food'}], | ||
'Foo'), | ||
[{title: 'Foo'}, {title: 'Bar Food'}]); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to tweak the style settings so we don have spaces around default args e.g. so we have
state=initialState
instead ofstate = initialState
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I found this a little weird at first. I don't totally hate it but not what I went with first.