From c341e97ef04c5070e5c2baae2de612b2f15475d5 Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Tue, 28 Mar 2017 22:09:33 +0300 Subject: [PATCH] SearchPage handles next page and prev page clicks --- server/dataLoader.js | 4 +- server/importer.js | 1 + src/Routes.js | 29 ++++- src/containers/SearchPage/SearchPage.duck.js | 12 +- src/containers/SearchPage/SearchPage.js | 93 ++++++++++++--- src/containers/SearchPage/SearchPage.test.js | 16 +-- .../__snapshots__/SearchPage.test.js.snap | 16 ++- src/index.js | 2 +- src/util/__snapshots__/routes.test.js.snap | 4 +- src/util/routes.js | 20 +++- src/util/routes.test.js | 107 ++++++++++++++---- 11 files changed, 242 insertions(+), 62 deletions(-) diff --git a/server/dataLoader.js b/server/dataLoader.js index 7f17051c..6b609812 100644 --- a/server/dataLoader.js +++ b/server/dataLoader.js @@ -1,9 +1,9 @@ const url = require('url'); -const { matchPathname, configureStore } = require('./importer'); +const { matchPathname, configureStore, routeConfiguration } = require('./importer'); exports.loadData = function(requestUrl, sdk) { const { pathname, query } = url.parse(requestUrl); - const matchedRoutes = matchPathname(pathname); + const matchedRoutes = matchPathname(pathname, routeConfiguration); const store = configureStore(sdk); diff --git a/server/importer.js b/server/importer.js index a09e6de0..83eb744f 100644 --- a/server/importer.js +++ b/server/importer.js @@ -16,4 +16,5 @@ module.exports = { renderApp: mainJs.default, matchPathname: mainJs.matchPathname, configureStore: mainJs.configureStore, + routeConfiguration: mainJs.routeConfiguration, }; diff --git a/src/Routes.js b/src/Routes.js index a97b92a3..d7b0e0bd 100644 --- a/src/Routes.js +++ b/src/Routes.js @@ -13,9 +13,29 @@ class RouteComponentRenderer extends Component { constructor(props) { super(props); this.canShowComponent = this.canShowComponent.bind(this); + this.callLoadData = this.callLoadData.bind(this); } + componentDidMount() { - const { match, location, route, dispatch } = this.props; + // Calling loadData on initial rendering (on client side). + this.callLoadData(this.props); + } + + componentWillReceiveProps(nextProps) { + // Calling loadData after initial rendering (on client side). + // This makes it possible to use loadData as default client side data loading technique. + // However it is better to fetch data before location change to avoid "Loading data" state. + this.callLoadData(nextProps); + } + + canShowComponent() { + const { isAuthenticated, route } = this.props; + const { auth } = route; + return !auth || isAuthenticated; + } + + callLoadData(props) { + const { match, location, route, dispatch } = props; const { loadData, name } = route; const shouldLoadData = typeof loadData === 'function' && this.canShowComponent(); @@ -31,11 +51,7 @@ class RouteComponentRenderer extends Component { }); } } - canShowComponent() { - const { isAuthenticated, route } = this.props; - const { auth } = route; - return !auth || isAuthenticated; - } + render() { const { route, match, location, staticContext, flattenedRoutes } = this.props; const { component: RouteComponent } = route; @@ -65,6 +81,7 @@ RouteComponentRenderer.propTypes = { search: string.isRequired, }).isRequired, staticContext: object.isRequired, + // eslint-disable-next-line react/no-unused-prop-types dispatch: func.isRequired, }; diff --git a/src/containers/SearchPage/SearchPage.duck.js b/src/containers/SearchPage/SearchPage.duck.js index f1e4df77..bb76fffe 100644 --- a/src/containers/SearchPage/SearchPage.duck.js +++ b/src/containers/SearchPage/SearchPage.duck.js @@ -9,6 +9,7 @@ export const SEARCH_LISTINGS_ERROR = 'app/SearchPage/SEARCH_LISTINGS_ERROR'; // ================ Reducer ================ // const initialState = { + pagination: null, searchParams: null, searchInProgress: false, searchListingsError: null, @@ -29,7 +30,12 @@ const listingPageReducer = (state = initialState, action = {}) => { searchListingsError: null, }; case SEARCH_LISTINGS_SUCCESS: - return { ...state, searchInProgress: false, currentPageResultIds: resultIds(payload.data) }; + return { + ...state, + currentPageResultIds: resultIds(payload.data), + pagination: payload.data.meta, + searchInProgress: false, + }; case SEARCH_LISTINGS_ERROR: // eslint-disable-next-line no-console console.error(payload); @@ -63,11 +69,11 @@ export const searchListings = searchParams => (dispatch, getState, sdk) => { dispatch(searchListingsRequest(searchParams)); - const { origin, include = [] } = searchParams; + const { origin, include = [], page, per_page } = searchParams; const searchOrQuery = origin ? sdk.listings.search(searchParams) - : sdk.listings.query({ include }); + : sdk.listings.query({ include, page, per_page }); return searchOrQuery .then(response => { diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 8ea0c9a5..6bed223a 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -2,23 +2,38 @@ import React, { PropTypes } from 'react'; import classNames from 'classnames'; import { FormattedMessage } from 'react-intl'; import { connect } from 'react-redux'; +import { withRouter } from 'react-router-dom'; import config from '../../config'; +import { createResourceLocatorString } from '../../util/routes'; import { parse } from '../../util/urlHelpers'; +import * as propTypes from '../../util/propTypes'; import { getListingsById } from '../../ducks/sdk.duck'; import { FilterPanel, - ListingCard, ListingCardSmall, MapPanel, PageLayout, SearchResultsPanel, } from '../../components'; import { searchListings } from './SearchPage.duck'; - import css from './SearchPage.css'; +// TODO Pagination page size might need to be dynamic on responsive page layouts +const RESULT_PAGE_SIZE = 12; + export const SearchPageComponent = props => { - const { tab, listings, searchParams, searchInProgress, searchListingsError } = props; + const { + flattenedRoutes, + listings, + location, + pagination, + push: historyPush, + searchInProgress, + searchListingsError, + searchParams, + tab, + } = props; + const totalItems = pagination ? pagination.total_items : 0; const filtersClassName = classNames(css.filters, { [css.open]: tab === 'filters' }); const listingsClassName = classNames(css.listings, { [css.open]: tab === 'listings' }); @@ -35,7 +50,7 @@ export const SearchPageComponent = props => { const resultsFound = (

- +

); @@ -51,22 +66,43 @@ export const SearchPageComponent = props => {

); + // Page is parsed from url + const { page = 1 } = parse(location.search); + const { address, origin, bounds } = searchParams || {}; + + const hasNext = pagination && !searchInProgress && page < pagination.totalPages; + const onNextPage = hasNext + ? () => { + const urlParams = { address, origin, bounds, page: page + 1 }; + historyPush(createResourceLocatorString('SearchPage', flattenedRoutes, {}, urlParams)); + } + : null; + + const hasPrev = pagination && !searchInProgress && page > 1; + const onPreviousPage = hasPrev + ? () => { + const urlParams = { address, origin, bounds, page: page - 1 }; + historyPush(createResourceLocatorString('SearchPage', flattenedRoutes, {}, urlParams)); + } + : null; + return ( {searchListingsError ? searchError : null} - {searchWasDone && listings.length > 0 ? resultsFound : null} - {searchWasDone && listings.length === 0 ? noResults : null} + {searchWasDone && totalItems > 0 ? resultsFound : null} + {searchWasDone && totalItems === 0 ? noResults : null} {searchInProgress ? loadingResults : null}
- - {listings.map(l => ( - - ))} - +
@@ -83,43 +119,64 @@ export const SearchPageComponent = props => { SearchPageComponent.defaultProps = { tab: 'listings', listings: [], + pagination: null, searchParams: {}, searchListingsError: null, }; -const { array, oneOf, object, instanceOf, bool } = PropTypes; +const { array, arrayOf, bool, func, instanceOf, number, oneOf, object, shape, string } = PropTypes; SearchPageComponent.propTypes = { - tab: oneOf(['filters', 'listings', 'map']).isRequired, + flattenedRoutes: arrayOf(propTypes.route).isRequired, listings: array, + location: shape({ + search: string.isRequired, + }).isRequired, + pagination: shape({ + page: number.isRequired, + per_page: number, // TODO handle snake_case + total_items: number, + total_pages: number.isRequired, + }), + // history.push from withRouter + push: func.isRequired, searchParams: object, searchInProgress: bool.isRequired, searchListingsError: instanceOf(Error), + tab: oneOf(['filters', 'listings', 'map']).isRequired, }; const mapStateToProps = state => { const { - searchParams, + currentPageResultIds, + pagination, searchInProgress, searchListingsError, - currentPageResultIds, + searchParams, } = state.SearchPage; return { listings: getListingsById(state.data, currentPageResultIds), - searchParams, + pagination, searchInProgress, searchListingsError, + searchParams, }; }; -const SearchPage = connect(mapStateToProps)(SearchPageComponent); +const SearchPage = connect(mapStateToProps)(withRouter(SearchPageComponent)); SearchPage.loadData = (params, search) => { const queryParams = parse(search, { latlng: ['origin'], latlngBounds: ['bounds'], }); - return searchListings({ ...queryParams, include: ['author', 'images'] }); + const page = queryParams.page || 1; + return searchListings({ + ...queryParams, + page, + per_page: RESULT_PAGE_SIZE, + include: ['author', 'images'], + }); }; export default SearchPage; diff --git a/src/containers/SearchPage/SearchPage.test.js b/src/containers/SearchPage/SearchPage.test.js index 6c0e2b04..efa32478 100644 --- a/src/containers/SearchPage/SearchPage.test.js +++ b/src/containers/SearchPage/SearchPage.test.js @@ -18,14 +18,14 @@ const { LatLng } = types; describe('SearchPageComponent', () => { it('matches snapshot', () => { - const tree = renderShallow( - v} - dispatch={() => null} - intl={fakeIntl} - searchInProgress={false} - /> - ); + const props = { + flattenedRoutes: [], + location: { search: '' }, + push: () => console.log('HistoryPush called'), + tab: 'listings', + searchInProgress: false, + }; + const tree = renderShallow(); expect(tree).toMatchSnapshot(); }); }); diff --git a/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap b/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap index 6c1c6972..6d062af7 100644 --- a/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap +++ b/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap @@ -8,7 +8,21 @@ exports[`SearchPageComponent matches snapshot 1`] = `
- +
diff --git a/src/index.js b/src/index.js index 5cf465fb..902aa2d4 100644 --- a/src/index.js +++ b/src/index.js @@ -87,4 +87,4 @@ export default renderApp; // exporting matchPathname and configureStore for server side rendering. // matchPathname helps to figure out which route is called and if it has preloading needs // configureStore is used for creating initial store state for Redux after preloading -export { matchPathname, configureStore }; +export { matchPathname, configureStore, routeConfiguration }; diff --git a/src/util/__snapshots__/routes.test.js.snap b/src/util/__snapshots__/routes.test.js.snap index 402a114d..0694a70a 100644 --- a/src/util/__snapshots__/routes.test.js.snap +++ b/src/util/__snapshots__/routes.test.js.snap @@ -1,6 +1,6 @@ -exports[`withFlattenedRoutes should inject the provided routes 1`] = ``; +exports[`util/routes.js withFlattenedRoutes should inject the provided routes 1`] = ``; -exports[`withFlattenedRoutes should inject the provided routes 2`] = ` +exports[`util/routes.js withFlattenedRoutes should inject the provided routes 2`] = `
SomePage
diff --git a/src/util/routes.js b/src/util/routes.js index f21b54e1..e6deccad 100644 --- a/src/util/routes.js +++ b/src/util/routes.js @@ -2,7 +2,7 @@ import React, { PropTypes } from 'react'; import { find } from 'lodash'; import { matchPath } from 'react-router-dom'; import pathToRegexp from 'path-to-regexp'; -import routesConfiguration from '../routesConfiguration'; +import { stringify } from './urlHelpers'; import * as propTypes from './propTypes'; // Flatten the routes config. @@ -50,7 +50,7 @@ export const pathByRouteName = (nameToFind, flattenedRoutes, params = {}) => * * @return {Array<{ route, params }>} - All matches as { route, params } objects */ -export const matchPathname = pathname => { +export const matchPathname = (pathname, routesConfiguration) => { // TODO: remove flattening when routesConfiguration is flat const flattenedRoutes = flattenRoutes(routesConfiguration); @@ -92,3 +92,19 @@ export const withFlattenedRoutes = Component => { return WithFlattenedRoutesComponent; }; + +/** + * ResourceLocatorString is used to direct webapp to correct page. + * In contrast to Universal Resource Locator (URL), this doesn't contain protocol, host, or port. + */ +export const createResourceLocatorString = ( + routeName, + flattenedRoutes, + pathParams = {}, + searchParams = {} +) => { + const searchQuery = stringify(searchParams); + const includeSearchQuery = searchQuery.length > 0 ? `?${searchQuery}` : ''; + const path = pathByRouteName(routeName, flattenedRoutes, pathParams); + return `${path}${includeSearchQuery}`; +}; diff --git a/src/util/routes.test.js b/src/util/routes.test.js index 50ae433f..2476c9b2 100644 --- a/src/util/routes.test.js +++ b/src/util/routes.test.js @@ -1,28 +1,97 @@ import React, { PropTypes } from 'react'; import { RoutesProvider } from '../components'; +import routesConfiguration from '../routesConfiguration'; import { renderDeep, renderShallow } from './test-helpers'; import * as propTypes from './propTypes'; -import { withFlattenedRoutes } from './routes'; +import { createResourceLocatorString, flattenRoutes, withFlattenedRoutes } from './routes'; const { arrayOf } = PropTypes; -describe('withFlattenedRoutes', () => { - it('should inject the provided routes', () => { - const CompComp = props =>
{props.flattenedRoutes[0].name}
; - CompComp.propTypes = { flattenedRoutes: arrayOf(propTypes.route).isRequired }; - const Comp = withFlattenedRoutes(CompComp); - const routes = [{ name: 'SomePage', path: 'some-page', component: () => null }]; - const shallowTree = renderShallow( - - - - ); - expect(shallowTree).toMatchSnapshot(); - const deepTree = renderDeep( - - - - ); - expect(deepTree).toMatchSnapshot(); +describe('util/routes.js', () => { + describe('withFlattenedRoutes', () => { + it('should inject the provided routes', () => { + const CompComp = props =>
{props.flattenedRoutes[0].name}
; + CompComp.propTypes = { flattenedRoutes: arrayOf(propTypes.route).isRequired }; + const Comp = withFlattenedRoutes(CompComp); + const routes = [{ name: 'SomePage', path: 'some-page', component: () => null }]; + const shallowTree = renderShallow( + + + + ); + expect(shallowTree).toMatchSnapshot(); + const deepTree = renderDeep( + + + + ); + expect(deepTree).toMatchSnapshot(); + }); + }); + + describe('createResourceLocatorString', () => { + const flattenedRoutes = flattenRoutes(routesConfiguration); + + it('should return meaningful strings if parameters are not needed', () => { + // default links without params in path or search query + expect( + createResourceLocatorString('SearchPage', flattenedRoutes, undefined, undefined) + ).toEqual('/s'); + expect(createResourceLocatorString('SearchPage', flattenedRoutes, {}, {})).toEqual('/s'); + }); + + it('should return meaningful strings with path parameters', () => { + expect( + createResourceLocatorString( + 'ListingPage', + flattenedRoutes, + { id: '1234', slug: 'nice-listing' }, + {} + ) + ).toEqual('/l/nice-listing/1234'); + expect(() => + createResourceLocatorString('ListingPage', flattenedRoutes, {}, {})).toThrowError( + TypeError('Expected "slug" to be defined') + ); + expect(() => + createResourceLocatorString( + 'ListingPage', + flattenedRoutes, + { id: '1234' }, + {} + )).toThrowError(TypeError('Expected "slug" to be defined')); + expect(() => + createResourceLocatorString( + 'ListingPage', + flattenedRoutes, + { slug: 'nice-listing' }, + {} + )).toThrowError(TypeError('Expected "id" to be defined')); + }); + + it('should return meaningful strings with search parameters', () => { + expect(createResourceLocatorString('SearchPage', flattenedRoutes, {}, { page: 2 })).toEqual( + '/s?page=2' + ); + expect( + createResourceLocatorString( + 'SearchPage', + flattenedRoutes, + {}, + { address: 'Helsinki', page: 2 } + ) + ).toEqual('/s?address=Helsinki&page=2'); + }); + + it('should return meaningful strings with path and search parameters', () => { + expect( + createResourceLocatorString( + 'ListingPage', + flattenedRoutes, + { id: '1234', slug: 'nice-listing' }, + { extrainfo: true } + ) + ).toEqual('/l/nice-listing/1234?extrainfo=true'); + }); }); });