From fcdb7767bf20591004e76f81bf8df57038843c6f Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Wed, 4 Apr 2018 16:28:23 +0300 Subject: [PATCH 01/10] Refactor helper functions --- src/containers/SearchPage/SearchPage.js | 57 +++++++++++++------------ 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 6d1bacf1..02a0b6bf 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -47,21 +47,15 @@ const CATEGORY_URL_PARAM = 'pub_category'; const AMENITIES_URL_PARAM = 'pub_amenities'; const USE_SEARCH_FILTER_PANEL = false; -// Find correct extended data key from config.custom -// e.g. 'pub_category' -> 'categories'. -const customConfigKey = paramKey => { - switch (paramKey) { - case CATEGORY_URL_PARAM: - return 'categories'; - case AMENITIES_URL_PARAM: - return 'amenities'; - default: - return null; - } +const customURLParamToConfig = { + [CATEGORY_URL_PARAM]: 'categories', + [AMENITIES_URL_PARAM]: 'amenities', }; -const validURLParamForExtendedData = (paramKey, urlParams) => { - const configKey = customConfigKey(paramKey); +// customURLParams +const validURLParamForExtendedData = (paramKey, urlParams, customConfigKeys) => { + //const configKey = customConfigKey(paramKey); + const configKey = customConfigKeys[paramKey]; const value = urlParams[paramKey]; const valueArray = value ? value.split(',') : []; @@ -74,25 +68,32 @@ const validURLParamForExtendedData = (paramKey, urlParams) => { }; // validate filter params -const validURLParamsForExtendedData = params => { - const { [CATEGORY_URL_PARAM]: category, [AMENITIES_URL_PARAM]: amenities, ...rest } = params; - return { - ...rest, - ...validURLParamForExtendedData(CATEGORY_URL_PARAM, params), - ...validURLParamForExtendedData(AMENITIES_URL_PARAM, params), - }; +const validURLParamsForExtendedData = (params, customURLParams, customURLParamToConfig) => { + const paramKeys = Object.keys(params); + return paramKeys.reduce((validParams, paramKey) => { + return customURLParams.includes(paramKey) + ? { ...validParams, ...validURLParamForExtendedData(paramKey, params, customURLParamToConfig) } + : { ...validParams, [paramKey] : params[paramKey] }; + }, {}); }; // extract search parameters, including a custom attribute named category -const pickSearchParamsOnly = params => { +const pickSearchParamsOnly = (params, customURLParams, customURLParamToConfig) => { const { address, origin, bounds, country, ...rest } = params || {}; const boundsMaybe = bounds ? { bounds } : {}; const originMaybe = config.sortSearchByDistance && origin ? { origin } : {}; + + const customSearchParamKeys = Object.keys(rest); + const customSearchParams = customSearchParamKeys.reduce((validParams, paramKey) => { + return customURLParams.includes(paramKey) + ? { ...validParams, ...validURLParamForExtendedData(paramKey, rest, customURLParamToConfig) } + : { ...validParams }; + }, {}); + return { ...boundsMaybe, ...originMaybe, - ...validURLParamForExtendedData(CATEGORY_URL_PARAM, rest), - ...validURLParamForExtendedData(AMENITIES_URL_PARAM, rest), + ...customSearchParams, }; }; @@ -175,8 +176,8 @@ export class SearchPageComponent extends Component { bounds: viewportBounds, country, mapSearch: true, - ...validURLParamForExtendedData(CATEGORY_URL_PARAM, rest), - ...validURLParamForExtendedData(AMENITIES_URL_PARAM, rest), + ...validURLParamForExtendedData(CATEGORY_URL_PARAM, rest, customURLParamToConfig), + ...validURLParamForExtendedData(AMENITIES_URL_PARAM, rest, customURLParamToConfig), }; this.viewportBounds = viewportBounds; history.push( @@ -271,11 +272,11 @@ export class SearchPageComponent extends Component { // urlQueryParams doesn't contain page specific url params // like mapSearch, page or origin (origin depends on config.sortSearchByDistance) - const urlQueryParams = pickSearchParamsOnly(searchInURL); + const urlQueryParams = pickSearchParamsOnly(searchInURL, [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], customURLParamToConfig); // Page transition might initially use values from previous search const urlQueryString = stringify(urlQueryParams); - const paramsQueryString = stringify(pickSearchParamsOnly(searchParams)); + const paramsQueryString = stringify(pickSearchParamsOnly(searchParams, [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], customURLParamToConfig)); const searchParamsMatch = urlQueryString === paramsQueryString; const { address, bounds, origin } = searchInURL || {}; @@ -284,7 +285,7 @@ export class SearchPageComponent extends Component { const totalItems = searchParamsMatch && hasPaginationInfo ? pagination.totalItems : 0; const listingsAreLoaded = !searchInProgress && searchParamsMatch && hasPaginationInfo; - const validQueryParams = validURLParamsForExtendedData(searchInURL); + const validQueryParams = validURLParamsForExtendedData(searchInURL, [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], customURLParamToConfig); const searchError = (

From 207728fbde4b0fa36d00916478878b23719e0d36 Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Wed, 4 Apr 2018 16:31:30 +0300 Subject: [PATCH 02/10] Move helper functions away from SearchPage main file. --- .../SearchPage/SearchPage.helpers.js | 50 +++++++++++++ src/containers/SearchPage/SearchPage.js | 74 ++++++------------- 2 files changed, 74 insertions(+), 50 deletions(-) create mode 100644 src/containers/SearchPage/SearchPage.helpers.js diff --git a/src/containers/SearchPage/SearchPage.helpers.js b/src/containers/SearchPage/SearchPage.helpers.js new file mode 100644 index 00000000..6009d20b --- /dev/null +++ b/src/containers/SearchPage/SearchPage.helpers.js @@ -0,0 +1,50 @@ +import { intersection } from 'lodash'; +import config from '../../config'; + +// customURLParams +export const validURLParamForExtendedData = (paramKey, urlParams, customConfigKeys) => { + //const configKey = customConfigKey(paramKey); + const configKey = customConfigKeys[paramKey]; + const value = urlParams[paramKey]; + const valueArray = value ? value.split(',') : []; + + if (configKey && valueArray.length > 0) { + const allowedValues = config.custom[configKey].map(a => a.key); + const validValues = intersection(valueArray, allowedValues).join(','); + return validValues.length > 0 ? { [paramKey]: validValues } : {}; + } + return {}; +}; + +// validate filter params +export const validURLParamsForExtendedData = (params, customURLParams, customURLParamToConfig) => { + const paramKeys = Object.keys(params); + return paramKeys.reduce((validParams, paramKey) => { + return customURLParams.includes(paramKey) + ? { + ...validParams, + ...validURLParamForExtendedData(paramKey, params, customURLParamToConfig), + } + : { ...validParams, [paramKey]: params[paramKey] }; + }, {}); +}; + +// extract search parameters, including a custom attribute named category +export const pickSearchParamsOnly = (params, customURLParams, customURLParamToConfig) => { + const { address, origin, bounds, country, ...rest } = params || {}; + const boundsMaybe = bounds ? { bounds } : {}; + const originMaybe = config.sortSearchByDistance && origin ? { origin } : {}; + + const customSearchParamKeys = Object.keys(rest); + const customSearchParams = customSearchParamKeys.reduce((validParams, paramKey) => { + return customURLParams.includes(paramKey) + ? { ...validParams, ...validURLParamForExtendedData(paramKey, rest, customURLParamToConfig) } + : { ...validParams }; + }, {}); + + return { + ...boundsMaybe, + ...originMaybe, + ...customSearchParams, + }; +}; diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 02a0b6bf..b6bd7784 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -4,7 +4,7 @@ import { FormattedMessage, injectIntl, intlShape } from 'react-intl'; import { connect } from 'react-redux'; import { compose } from 'redux'; import { withRouter } from 'react-router-dom'; -import { debounce, intersection, isEqual, unionWith } from 'lodash'; +import { debounce, isEqual, unionWith } from 'lodash'; import classNames from 'classnames'; import config from '../../config'; import routeConfiguration from '../../routeConfiguration'; @@ -29,8 +29,13 @@ import { SearchFiltersPanel, } from '../../components'; import { TopbarContainer } from '../../containers'; -import { searchListings, searchMapListings, setActiveListing } from './SearchPage.duck'; +import { searchListings, searchMapListings, setActiveListing } from './SearchPage.duck'; +import { + pickSearchParamsOnly, + validURLParamForExtendedData, + validURLParamsForExtendedData, +} from './SearchPage.helpers'; import css from './SearchPage.css'; // Pagination page size might need to be dynamic on responsive page layouts @@ -52,51 +57,6 @@ const customURLParamToConfig = { [AMENITIES_URL_PARAM]: 'amenities', }; -// customURLParams -const validURLParamForExtendedData = (paramKey, urlParams, customConfigKeys) => { - //const configKey = customConfigKey(paramKey); - const configKey = customConfigKeys[paramKey]; - const value = urlParams[paramKey]; - const valueArray = value ? value.split(',') : []; - - if (configKey && valueArray.length > 0) { - const allowedValues = config.custom[configKey].map(a => a.key); - const validValues = intersection(valueArray, allowedValues).join(','); - return validValues.length > 0 ? { [paramKey]: validValues } : {}; - } - return {}; -}; - -// validate filter params -const validURLParamsForExtendedData = (params, customURLParams, customURLParamToConfig) => { - const paramKeys = Object.keys(params); - return paramKeys.reduce((validParams, paramKey) => { - return customURLParams.includes(paramKey) - ? { ...validParams, ...validURLParamForExtendedData(paramKey, params, customURLParamToConfig) } - : { ...validParams, [paramKey] : params[paramKey] }; - }, {}); -}; - -// extract search parameters, including a custom attribute named category -const pickSearchParamsOnly = (params, customURLParams, customURLParamToConfig) => { - const { address, origin, bounds, country, ...rest } = params || {}; - const boundsMaybe = bounds ? { bounds } : {}; - const originMaybe = config.sortSearchByDistance && origin ? { origin } : {}; - - const customSearchParamKeys = Object.keys(rest); - const customSearchParams = customSearchParamKeys.reduce((validParams, paramKey) => { - return customURLParams.includes(paramKey) - ? { ...validParams, ...validURLParamForExtendedData(paramKey, rest, customURLParamToConfig) } - : { ...validParams }; - }, {}); - - return { - ...boundsMaybe, - ...originMaybe, - ...customSearchParams, - }; -}; - export class SearchPageComponent extends Component { constructor(props) { super(props); @@ -272,11 +232,21 @@ export class SearchPageComponent extends Component { // urlQueryParams doesn't contain page specific url params // like mapSearch, page or origin (origin depends on config.sortSearchByDistance) - const urlQueryParams = pickSearchParamsOnly(searchInURL, [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], customURLParamToConfig); + const urlQueryParams = pickSearchParamsOnly( + searchInURL, + [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], + customURLParamToConfig + ); // Page transition might initially use values from previous search const urlQueryString = stringify(urlQueryParams); - const paramsQueryString = stringify(pickSearchParamsOnly(searchParams, [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], customURLParamToConfig)); + const paramsQueryString = stringify( + pickSearchParamsOnly( + searchParams, + [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], + customURLParamToConfig + ) + ); const searchParamsMatch = urlQueryString === paramsQueryString; const { address, bounds, origin } = searchInURL || {}; @@ -285,7 +255,11 @@ export class SearchPageComponent extends Component { const totalItems = searchParamsMatch && hasPaginationInfo ? pagination.totalItems : 0; const listingsAreLoaded = !searchInProgress && searchParamsMatch && hasPaginationInfo; - const validQueryParams = validURLParamsForExtendedData(searchInURL, [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], customURLParamToConfig); + const validQueryParams = validURLParamsForExtendedData( + searchInURL, + [AMENITIES_URL_PARAM, CATEGORY_URL_PARAM], + customURLParamToConfig + ); const searchError = (

From cd01d694e7c48466daa646370fd07a7f29d9ba49 Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Wed, 4 Apr 2018 16:36:56 +0300 Subject: [PATCH 03/10] Remove unused fetchMoreListingsToMap --- src/containers/SearchPage/SearchPage.js | 54 ------------------------- 1 file changed, 54 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index b6bd7784..4857ca9b 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -42,8 +42,6 @@ import css from './SearchPage.css'; // Current design has max 3 columns 12 is divisible by 2 and 3 // So, there's enough cards to fill all columns on full pagination pages const RESULT_PAGE_SIZE = 24; -const MAX_SEARCH_RESULT_PAGE_SIZE_ON_MAP = 80; // max page size is 100 in API -const MAX_SEARCH_RESULT_PAGES_ON_MAP = 1; // page size * n pages = number of listings shown on a map. const MODAL_BREAKPOINT = 768; // Search is in modal on mobile layout const SEARCH_WITH_MAP_DEBOUNCE = 300; // Little bit of debounce before search is initiated. const BOUNDS_FIXED_PRECISION = 8; @@ -77,18 +75,12 @@ export class SearchPageComponent extends Component { this.searchMapListingsInProgress = false; this.onIdle = debounce(this.onIdle.bind(this), SEARCH_WITH_MAP_DEBOUNCE); - this.fetchMoreListingsToMap = this.fetchMoreListingsToMap.bind(this); this.onOpenMobileModal = this.onOpenMobileModal.bind(this); this.onCloseMobileModal = this.onCloseMobileModal.bind(this); } - componentDidMount() { - this.fetchMoreListingsToMap(this.props.location); - } - componentWillReceiveProps(nextProps) { if (!isEqual(this.props.location, nextProps.location)) { - this.fetchMoreListingsToMap(nextProps.location); // If no mapSearch url parameter is given, this is original location search const { mapSearch } = parse(nextProps.location.search, { @@ -149,52 +141,6 @@ export class SearchPageComponent extends Component { } } - fetchMoreListingsToMap(location) { - // TODO Remove this function. - // Temporarily just return immediately to avoid merge conflicts. - return; - - // eslint-disable-next-line no-unreachable - const { onSearchMapListings } = this.props; - const searchInURL = parse(location.search, { - latlng: ['origin'], - latlngBounds: ['bounds'], - }); - - const perPage = MAX_SEARCH_RESULT_PAGE_SIZE_ON_MAP; - const page = 1; - const { address, country, origin, ...rest } = searchInURL; - const originMaybe = config.sortSearchByDistance ? { origin } : {}; - const searchParamsForMapResults = { - ...rest, - ...originMaybe, - include: ['images'], - page, - perPage, - 'fields.image': ['variants.landscape-crop', 'variants.landscape-crop2x'], - 'limit.images': 1, - }; - this.searchMapListingsInProgress = true; - - // Search more listings for map - onSearchMapListings(searchParamsForMapResults) - .then(response => { - const hasNextPage = - page < response.data.meta.totalPages && page < MAX_SEARCH_RESULT_PAGES_ON_MAP; - if (hasNextPage) { - onSearchMapListings({ ...searchParamsForMapResults, page: page + 1 }); - } else { - this.searchMapListingsInProgress = false; - } - }) - .catch(error => { - // In case of error, stop recursive loop and report error. - // TODO: Show and error in the listings column - // eslint-disable-next-line no-console - console.error(`An error (${error} occured while trying to retrieve map listings`); - }); - } - // Invoked when a modal is opened from a child component, // for example when a filter modal is opened in mobile view onOpenMobileModal() { From 3a65a56d32105b4d04ad9f1e274213f07b4a385d Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Wed, 4 Apr 2018 16:44:51 +0300 Subject: [PATCH 04/10] Gather JSX usage inside left panel/container --- src/containers/SearchPage/SearchPage.js | 55 ++++++++++++------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 4857ca9b..0556ef18 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -270,33 +270,7 @@ export class SearchPageComponent extends Component { this.setState({ isSearchMapOpenOnMobile: true }); }; - const extraSearchFiltersPanelOrListings = - USE_SEARCH_FILTER_PANEL && this.state.isSearchFiltersPanelOpen ? ( -
- this.setState({ isSearchFiltersPanelOpen: false })} - /> -
- ) : ( -
- {searchListingsError ? searchError : null} - -
- ); + const isSearchFiltersPanelOpen = USE_SEARCH_FILTER_PANEL && this.state.isSearchFiltersPanelOpen; // An example how to check how many filters are selected on SearchFilterPanel // if it is in use. @@ -372,7 +346,32 @@ export class SearchPageComponent extends Component { categories={categories} amenities={amenities} /> - {extraSearchFiltersPanelOrListings} + { isSearchFiltersPanelOpen ? ( +
+ this.setState({ isSearchFiltersPanelOpen: false })} + /> +
+ ) : ( +
+ {searchListingsError ? searchError : null} + +
+ )} Date: Wed, 4 Apr 2018 18:14:50 +0300 Subject: [PATCH 05/10] Main column stuff to separate MainPanel component --- .../SearchFiltersPanel/SearchFiltersPanel.js | 9 +- src/containers/SearchPage/MainPanel.js | 157 ++++++++++++++++++ src/containers/SearchPage/SearchPage.js | 121 +++----------- 3 files changed, 180 insertions(+), 107 deletions(-) create mode 100644 src/containers/SearchPage/MainPanel.js diff --git a/src/components/SearchFiltersPanel/SearchFiltersPanel.js b/src/components/SearchFiltersPanel/SearchFiltersPanel.js index 103df60c..2d6f019c 100644 --- a/src/components/SearchFiltersPanel/SearchFiltersPanel.js +++ b/src/components/SearchFiltersPanel/SearchFiltersPanel.js @@ -41,10 +41,6 @@ import { createResourceLocatorString } from '../../util/routes'; import { InlineTextButton } from '../../components'; import css from './SearchFiltersPanel.css'; -// Create constants from url params and uset them in FILTERS array and while adding actual filters -// e.g. const MULTI_SELECT_URL_PARAM = 'pub_filterX'; -const FILTERS = []; - class SearchFiltersPanelComponent extends Component { constructor(props) { super(props); @@ -82,9 +78,9 @@ class SearchFiltersPanelComponent extends Component { // Reset all filter query parameters resetAll(e) { - const { urlQueryParams, history, onClosePanel } = this.props; + const { urlQueryParams, customURLParamToConfig, history, onClosePanel } = this.props; - const queryParams = omit(urlQueryParams, FILTERS); + const queryParams = omit(urlQueryParams, Object.keys(customURLParamToConfig)); history.push(createResourceLocatorString('SearchPage', routeConfiguration(), {}, queryParams)); // Ensure that panel closes (if now changes have been made) @@ -161,6 +157,7 @@ SearchFiltersPanelComponent.propTypes = { rootClassName: string, className: string, urlQueryParams: object.isRequired, + customURLParamToConfig: object.isRequired, onClosePanel: func.isRequired, // from injectIntl diff --git a/src/containers/SearchPage/MainPanel.js b/src/containers/SearchPage/MainPanel.js new file mode 100644 index 00000000..c13fc815 --- /dev/null +++ b/src/containers/SearchPage/MainPanel.js @@ -0,0 +1,157 @@ +import React, { Component } from 'react'; +import { array, bool, func, object, number, string } from 'prop-types'; +import { FormattedMessage } from 'react-intl'; +import classNames from 'classnames'; +import { propTypes } from '../../util/types'; +import { + SearchResultsPanel, + SearchFilters, + SearchFiltersMobile, + SearchFiltersPanel, +} from '../../components'; + +import css from './SearchPage.css'; + +class MainPanel extends Component { + constructor(props) { + super(props); + this.state = { isSearchFiltersPanelOpen: false }; + } + + render() { + const { + className, + rootClassName, + urlQueryParams, + listings, + searchInProgress, + searchListingsError, + searchParamsAreInSync, + onActivateListing, + onManageDisableScrolling, + onMapIconClick, + pagination, + searchParamsForPagination, + showAsModalMaxWidth, + customURLParamToConfig, + primaryFilters, + secondaryFilters, + } = this.props; + + const isSearchFiltersPanelOpen = !!secondaryFilters && this.state.isSearchFiltersPanelOpen; + const searchFiltersPanelSelectedCount = !secondaryFilters + ? 0 + : Object.keys(customURLParamToConfig) + .map(key => urlQueryParams[key]) + .filter(param => !!param).length; + + const searchFiltersPanelProps = !!secondaryFilters + ? { + isSearchFiltersPanelOpen: this.state.isSearchFiltersPanelOpen, + toggleSearchFiltersPanel: isOpen => { + this.setState({ isSearchFiltersPanelOpen: isOpen }); + }, + searchFiltersPanelSelectedCount, + } + : {}; + + const hasPaginationInfo = !!pagination && pagination.totalItems != null; + const totalItems = searchParamsAreInSync && hasPaginationInfo ? pagination.totalItems : 0; + const listingsAreLoaded = !searchInProgress && searchParamsAreInSync && hasPaginationInfo; + + const classes = classNames(rootClassName || css.searchResultContainer, className); + + return ( +
+ + + {isSearchFiltersPanelOpen ? ( +
+ this.setState({ isSearchFiltersPanelOpen: false })} + {...secondaryFilters} + /> +
+ ) : ( +
+ {searchListingsError ? ( +

+ +

+ ) : null} + +
+ )} +
+ ); + } +} + +MainPanel.defaultProps = { + className: null, + rootClassName: null, + listings: [], + resultsCount: 0, + pagination: null, + searchParamsForPagination: {}, + primaryFilters: null, + secondaryFilters: null, +}; + +MainPanel.propTypes = { + className: string, + rootClassName: string, + + urlQueryParams: object.isRequired, + listings: array, + searchInProgress: bool.isRequired, + searchListingsError: propTypes.error, + searchParamsAreInSync: bool.isRequired, + onActivateListing: func.isRequired, + onManageDisableScrolling: func.isRequired, + onMapIconClick: func.isRequired, + pagination: propTypes.pagination, + searchParamsForPagination: object, + showAsModalMaxWidth: number.isRequired, + customURLParamToConfig: object.isRequired, + primaryFilters: object, + secondaryFilters: object, +}; + +export default MainPanel; diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 0556ef18..57f490e0 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -1,6 +1,6 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; -import { FormattedMessage, injectIntl, intlShape } from 'react-intl'; +import { injectIntl, intlShape } from 'react-intl'; import { connect } from 'react-redux'; import { compose } from 'redux'; import { withRouter } from 'react-router-dom'; @@ -19,15 +19,7 @@ import { createSlug, parse, stringify } from '../../util/urlHelpers'; import { propTypes } from '../../util/types'; import { getListingsById } from '../../ducks/marketplaceData.duck'; import { manageDisableScrolling, isScrollingDisabled } from '../../ducks/UI.duck'; -import { - SearchMap, - ModalInMobile, - Page, - SearchResultsPanel, - SearchFilters, - SearchFiltersMobile, - SearchFiltersPanel, -} from '../../components'; +import { SearchMap, ModalInMobile, Page } from '../../components'; import { TopbarContainer } from '../../containers'; import { searchListings, searchMapListings, setActiveListing } from './SearchPage.duck'; @@ -36,6 +28,7 @@ import { validURLParamForExtendedData, validURLParamsForExtendedData, } from './SearchPage.helpers'; +import MainPanel from './MainPanel'; import css from './SearchPage.css'; // Pagination page size might need to be dynamic on responsive page layouts @@ -48,7 +41,6 @@ const BOUNDS_FIXED_PRECISION = 8; const CATEGORY_URL_PARAM = 'pub_category'; const AMENITIES_URL_PARAM = 'pub_amenities'; -const USE_SEARCH_FILTER_PANEL = false; const customURLParamToConfig = { [CATEGORY_URL_PARAM]: 'categories', @@ -62,7 +54,6 @@ export class SearchPageComponent extends Component { this.state = { isSearchMapOpenOnMobile: props.tab === 'map', isMobileModalOpen: false, - isSearchFiltersPanelOpen: false, }; // Initiating map creates 'bounds_changes' event @@ -81,7 +72,6 @@ export class SearchPageComponent extends Component { componentWillReceiveProps(nextProps) { if (!isEqual(this.props.location, nextProps.location)) { - // If no mapSearch url parameter is given, this is original location search const { mapSearch } = parse(nextProps.location.search, { latlng: ['origin'], @@ -193,13 +183,9 @@ export class SearchPageComponent extends Component { customURLParamToConfig ) ); - const searchParamsMatch = urlQueryString === paramsQueryString; - const { address, bounds, origin } = searchInURL || {}; - const hasPaginationInfo = !!pagination && pagination.totalItems != null; - const totalItems = searchParamsMatch && hasPaginationInfo ? pagination.totalItems : 0; - const listingsAreLoaded = !searchInProgress && searchParamsMatch && hasPaginationInfo; + const searchParamsAreInSync = urlQueryString === paramsQueryString; const validQueryParams = validURLParamsForExtendedData( searchInURL, @@ -207,12 +193,6 @@ export class SearchPageComponent extends Component { customURLParamToConfig ); - const searchError = ( -

- -

- ); - const searchMap = ( !!param).length - const searchFiltersPanelSelectedCount = 0; - const searchFiltersPanelProps = USE_SEARCH_FILTER_PANEL - ? { - isSearchFiltersPanelOpen: this.state.isSearchFiltersPanelOpen, - toggleSearchFiltersPanel: isOpen => { - this.setState({ isSearchFiltersPanelOpen: isOpen }); - }, - searchFiltersPanelSelectedCount, - } - : {}; - // Set topbar class based on if a modal is open in // a child component const topbarClasses = this.state.isMobileModalOpen @@ -318,61 +276,22 @@ export class SearchPageComponent extends Component { currentSearchParams={urlQueryParams} />
-
- - - { isSearchFiltersPanelOpen ? ( -
- this.setState({ isSearchFiltersPanelOpen: false })} - /> -
- ) : ( -
- {searchListingsError ? searchError : null} - -
- )} -
+ Date: Thu, 5 Apr 2018 13:21:17 +0300 Subject: [PATCH 06/10] Gather Map related JSX inside render return --- src/containers/SearchPage/SearchPage.js | 41 ++++++++++++------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 57f490e0..bc0a6345 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -183,8 +183,6 @@ export class SearchPageComponent extends Component { customURLParamToConfig ) ); - const { address, bounds, origin } = searchInURL || {}; - const searchParamsAreInSync = urlQueryString === paramsQueryString; const validQueryParams = validURLParamsForExtendedData( @@ -193,25 +191,7 @@ export class SearchPageComponent extends Component { customURLParamToConfig ); - const searchMap = ( - { - onManageDisableScrolling('SearchPage.map', false); - }} - useLocationSearchBounds={!this.viewportBounds} - /> - ); - const showSearchMapInMobile = this.state.isSearchMapOpenOnMobile ? searchMap : null; const isWindowDefined = typeof window !== 'undefined'; - const searchMapMaybe = - isWindowDefined && window.innerWidth < MODAL_BREAKPOINT ? showSearchMapInMobile : searchMap; - // Schema for search engines (helps them to understand what this page is about) // http://schema.org // We are using JSON-LD format @@ -241,6 +221,9 @@ export class SearchPageComponent extends Component { itemListOrder: 'http://schema.org/ItemListOrderAscending', itemListElement: schemaListings, }); + const isMobileLayout = isWindowDefined && window.innerWidth < MODAL_BREAKPOINT; + const shouldShowSearchMap = + !isMobileLayout || (isMobileLayout && this.state.isSearchMapOpenOnMobile); const onMapIconClick = () => { this.useLocationSearchBounds = true; @@ -248,6 +231,7 @@ export class SearchPageComponent extends Component { this.setState({ isSearchMapOpenOnMobile: true }); }; + const { address, bounds, origin } = searchInURL || {}; // Set topbar class based on if a modal is open in // a child component const topbarClasses = this.state.isMobileModalOpen @@ -300,7 +284,22 @@ export class SearchPageComponent extends Component { showAsModalMaxWidth={MODAL_BREAKPOINT} onManageDisableScrolling={onManageDisableScrolling} > -
{searchMapMaybe}
+
+ {shouldShowSearchMap ? ( + { + onManageDisableScrolling('SearchPage.map', false); + }} + useLocationSearchBounds={!this.viewportBounds} + /> + ) : null} +
From 922b38a37dda429c47cf53bd75eb4e71d7dba56f Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Thu, 5 Apr 2018 13:24:30 +0300 Subject: [PATCH 07/10] Move schema building to helper function. --- .../SearchPage/SearchPage.helpers.js | 48 +++++++++++++++++++ src/containers/SearchPage/SearchPage.js | 46 +++--------------- 2 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.helpers.js b/src/containers/SearchPage/SearchPage.helpers.js index 6009d20b..e0514233 100644 --- a/src/containers/SearchPage/SearchPage.helpers.js +++ b/src/containers/SearchPage/SearchPage.helpers.js @@ -1,5 +1,8 @@ import { intersection } from 'lodash'; import config from '../../config'; +import { createResourceLocatorString } from '../../util/routes'; +import { createSlug } from '../../util/urlHelpers'; +import routeConfiguration from '../../routeConfiguration'; // customURLParams export const validURLParamForExtendedData = (paramKey, urlParams, customConfigKeys) => { @@ -48,3 +51,48 @@ export const pickSearchParamsOnly = (params, customURLParams, customURLParamToCo ...customSearchParams, }; }; + +export const createSearchResultSchema = (listings, address, intl) => { + // Schema for search engines (helps them to understand what this page is about) + // http://schema.org + // We are using JSON-LD format + const siteTitle = config.siteTitle; + const searchAddress = address || intl.formatMessage({ id: 'SearchPage.schemaMapSearch' }); + const schemaDescription = intl.formatMessage({ id: 'SearchPage.schemaDescription' }); + const schemaTitle = intl.formatMessage( + { id: 'SearchPage.schemaTitle' }, + { searchAddress, siteTitle } + ); + + const schemaListings = listings.map((l, i) => { + const title = l.attributes.title; + const pathToItem = createResourceLocatorString('ListingPage', routeConfiguration(), { + id: l.id.uuid, + slug: createSlug(title), + }); + return { + '@type': 'ListItem', + position: i, + url: `${config.canonicalRootURL}${pathToItem}`, + name: title, + }; + }); + + const schemaMainEntity = JSON.stringify({ + '@type': 'ItemList', + name: searchAddress, + itemListOrder: 'http://schema.org/ItemListOrderAscending', + itemListElement: schemaListings, + }); + return { + title: schemaTitle, + description: schemaDescription, + schema: { + '@context': 'http://schema.org', + '@type': 'SearchResultsPage', + description: schemaDescription, + name: schemaTitle, + mainEntity: [schemaMainEntity], + }, + }; +}; diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index bc0a6345..e960ddd1 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -15,7 +15,7 @@ import { hasSameSDKBounds, } from '../../util/googleMaps'; import { createResourceLocatorString } from '../../util/routes'; -import { createSlug, parse, stringify } from '../../util/urlHelpers'; +import { parse, stringify } from '../../util/urlHelpers'; import { propTypes } from '../../util/types'; import { getListingsById } from '../../ducks/marketplaceData.duck'; import { manageDisableScrolling, isScrollingDisabled } from '../../ducks/UI.duck'; @@ -27,6 +27,7 @@ import { pickSearchParamsOnly, validURLParamForExtendedData, validURLParamsForExtendedData, + createSearchResultSchema, } from './SearchPage.helpers'; import MainPanel from './MainPanel'; import css from './SearchPage.css'; @@ -192,35 +193,6 @@ export class SearchPageComponent extends Component { ); const isWindowDefined = typeof window !== 'undefined'; - // Schema for search engines (helps them to understand what this page is about) - // http://schema.org - // We are using JSON-LD format - const siteTitle = config.siteTitle; - const searchAddress = address || intl.formatMessage({ id: 'SearchPage.schemaMapSearch' }); - const schemaTitle = intl.formatMessage( - { id: 'SearchPage.schemaTitle' }, - { searchAddress, siteTitle } - ); - const schemaDescription = intl.formatMessage({ id: 'SearchPage.schemaDescription' }); - const schemaListings = listings.map((l, i) => { - const title = l.attributes.title; - const pathToItem = createResourceLocatorString('ListingPage', routeConfiguration(), { - id: l.id.uuid, - slug: createSlug(title), - }); - return { - '@type': 'ListItem', - position: i, - url: `${config.canonicalRootURL}${pathToItem}`, - name: title, - }; - }); - const schemaMainEntity = JSON.stringify({ - '@type': 'ItemList', - name: searchAddress, - itemListOrder: 'http://schema.org/ItemListOrderAscending', - itemListElement: schemaListings, - }); const isMobileLayout = isWindowDefined && window.innerWidth < MODAL_BREAKPOINT; const shouldShowSearchMap = !isMobileLayout || (isMobileLayout && this.state.isSearchMapOpenOnMobile); @@ -232,6 +204,8 @@ export class SearchPageComponent extends Component { }; const { address, bounds, origin } = searchInURL || {}; + const { title, description, schema } = createSearchResultSchema(listings, address, intl); + // Set topbar class based on if a modal is open in // a child component const topbarClasses = this.state.isMobileModalOpen @@ -244,15 +218,9 @@ export class SearchPageComponent extends Component { return ( Date: Thu, 5 Apr 2018 15:01:43 +0300 Subject: [PATCH 08/10] Add README.md to SearchPage --- src/containers/SearchPage/README.md | 71 +++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 src/containers/SearchPage/README.md diff --git a/src/containers/SearchPage/README.md b/src/containers/SearchPage/README.md new file mode 100644 index 00000000..1c358224 --- /dev/null +++ b/src/containers/SearchPage/README.md @@ -0,0 +1,71 @@ +# SearchPage + +SearchPage component has roughly 3 sections inside its layout: +Topbar, MainPanel (for results and filters), and Map + +So, rough JSX presentation is something like: +```jsx + + + + + +``` + +Searches can be made by each of these components. + +* Topbar contains location search (LocationAutocompleteInput) +* MainPanel has Filters that can fine-tune current location search +* SearchMap can create new location searches when the map's bounding box changes + (i.e. moving, zooming, etc.) + +## Topbar + +In contrast to other pages, Topbar gets `currentSearchParams` among other props. This makes it +possible for Topbar to take current filters into account. + +## MainPanel + +MainPanel has two functions: showing searchResults and showing filters. There are two sets of +filters that can be passed to it: `primaryFilters`, `secondaryFilters`. + +Primary filters are filters that are shown always on top of SearchResults as dropdown-selections. +We recommend that only 1 - 3 primary filters are passed in since they start to take too much space +on narrow screens. + +Secondary filters create one more button to the space containing primary filters: *More filters*. +This more-filters button opens up a SearchFiltersPanel component that can be changed to show those +extra filters passed to it. +**Note:** Currently, it doesn't contain any filter components by default, even if you pass in some +filter data. Creating those filter components is part of the customization process. + +On the mobile layout, all the filters are shown in separate mobile filters panel. + +## SearchMap + +SearchMap listens to 'idle' event and SearchPage function `onIndle` can create a new location search if +SearchMap's bounds have changed enough. + +## Other things to consider + +### URL Params vs marketplace-custom-config.js + +Filter handling needs some mapping between configured extended data. For example, +*marketplace-custom-config.js* defines all the categories used by the app, but that data needs to be in +format `pub_category=selectedCategory` when we talk with the API. (We also use this format in URL.) + +This mapping is done at the beginning of the file: +``` +const CATEGORY_URL_PARAM = 'pub_category'; +const AMENITIES_URL_PARAM = 'pub_amenities'; + +const customURLParamToConfig = { + [CATEGORY_URL_PARAM]: 'categories', + [AMENITIES_URL_PARAM]: 'amenities', +}; +``` + +### SeachPage schema / SEO + +Schema is created inside `createSearchResultSchema` in *SearchPage.helpers.js*. It needs listings +and address to make meaningful JSON-LD presentation for search engines. From 465c8eb857b4fbc7e7120e273cfbf3abfc58f09e Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Thu, 5 Apr 2018 15:35:01 +0300 Subject: [PATCH 09/10] Fix tests --- src/containers/SearchPage/SearchPage.js | 1 + src/containers/SearchPage/SearchPage.test.js | 1 + .../__snapshots__/SearchPage.test.js.snap | 97 ++++++++----------- 3 files changed, 45 insertions(+), 54 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index e960ddd1..ccd92167 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -293,6 +293,7 @@ const { array, bool, func, oneOf, object, shape, string } = PropTypes; SearchPageComponent.propTypes = { listings: array, mapListings: array, + onActivateListing: func.isRequired, onManageDisableScrolling: func.isRequired, onSearchMapListings: func.isRequired, pagination: propTypes.pagination, diff --git a/src/containers/SearchPage/SearchPage.test.js b/src/containers/SearchPage/SearchPage.test.js index aa38bfc9..9b7a35e4 100644 --- a/src/containers/SearchPage/SearchPage.test.js +++ b/src/containers/SearchPage/SearchPage.test.js @@ -36,6 +36,7 @@ describe('SearchPageComponent', () => { currentUserHasListings: false, intl: fakeIntl, isAuthenticated: false, + onActivateListing: noop, onLogout: noop, onManageDisableScrolling: noop, onSearchMapListings: noop, diff --git a/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap b/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap index 2c6d9c8f..df9d474c 100644 --- a/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap +++ b/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap @@ -22,10 +22,29 @@ exports[`SearchPageComponent matches snapshot 1`] = ` currentSearchParams={Object {}} />
-
- - -
- -
-
+ } + showAsModalMaxWidth={768} + urlQueryParams={Object {}} + /> Date: Fri, 6 Apr 2018 11:54:15 +0300 Subject: [PATCH 10/10] Review changes --- src/containers/SearchPage/SearchPage.helpers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/containers/SearchPage/SearchPage.helpers.js b/src/containers/SearchPage/SearchPage.helpers.js index e0514233..d42d2487 100644 --- a/src/containers/SearchPage/SearchPage.helpers.js +++ b/src/containers/SearchPage/SearchPage.helpers.js @@ -6,7 +6,6 @@ import routeConfiguration from '../../routeConfiguration'; // customURLParams export const validURLParamForExtendedData = (paramKey, urlParams, customConfigKeys) => { - //const configKey = customConfigKey(paramKey); const configKey = customConfigKeys[paramKey]; const value = urlParams[paramKey]; const valueArray = value ? value.split(',') : []; @@ -32,7 +31,8 @@ export const validURLParamsForExtendedData = (params, customURLParams, customURL }, {}); }; -// extract search parameters, including a custom attribute named category +// extract search parameters, including a custom URL params +// which are validated by mapping the values to marketplace custom config. export const pickSearchParamsOnly = (params, customURLParams, customURLParamToConfig) => { const { address, origin, bounds, country, ...rest } = params || {}; const boundsMaybe = bounds ? { bounds } : {};