Merge pull request #882 from sharetribe/reusable-searchmap-vol2

Reusable SearchMap

.nsprc

@@ -4,6 +4,7 @@
     "https://nodesecurity.io/advisories/157",
     "https://nodesecurity.io/advisories/577",
     "https://nodesecurity.io/advisories/654",
-    "https://nodesecurity.io/advisories/664"
+    "https://nodesecurity.io/advisories/664",
+    "https://nodesecurity.io/advisories/678"
   ]
 }

CHANGELOG.md

@@ -11,6 +11,10 @@ way to update this template, but currently, we follow a pattern:
 * Patch (v0.0.**X**): Bug fixes and small changes to components.
 
 ---
+## Upcoming version 2018-08-XX
+* [change] Reusable SearchMap. Fixed the original reverted version. (Includes audit exception 678)
+  [#882](https://github.com/sharetribe/flex-template-web/pull/882)
+
 ## v1.3.1
 * [fix] Hotfix: reverting the usage of ReusableMapContainer due to
   production build error.

src/components/SearchMap/ReusableMapContainer.js

@@ -1,8 +1,20 @@
 import React from 'react';
 import ReactDOM from 'react-dom';
 import { node, string } from 'prop-types';
+import mapValues from 'lodash/mapValues';
+import { IntlProvider } from 'react-intl';
+import messages from '../../translations/en.json';
+import config from '../../config';
+
 import css from './SearchMap.css';
 
+/**
+ * ReusableMapContainer makes Google Map usage more effective. This improves:
+ * - Performance: no need to load dynamic map every time user enters the search page view on SPA.
+ * - Efficient Google Maps usage: less unnecessary calls to instantiate a dynamic map.
+ * - Reaction to a bug when removing Google Map instance
+ *   https://issuetracker.google.com/issues/35821412
+ */
 class ReusableMapContainer extends React.Component {
   constructor(props) {
     super(props);
@@ -41,6 +53,29 @@ class ReusableMapContainer extends React.Component {
   }
 
   renderSearchMap() {
+    // Prepare rendering child (MapWithGoogleMap component) to new location
+    // We need to add translations (IntlProvider) for map overlay components
+    //
+    // NOTICE: Children rendered with ReactDOM.render doesn't have Router access
+    // You need to provide onClick functions and URLs as props.
+    const renderChildren = () => {
+      const isTestEnv = process.env.NODE_ENV === 'test';
+
+      // Locale should not affect the tests. We ensure this by providing
+      // messages with the key as the value of each message.
+      const testMessages = mapValues(messages, (val, key) => key);
+      const localeMessages = isTestEnv ? testMessages : messages;
+
+      const children = (
+        <IntlProvider locale={config.locale} messages={localeMessages}>
+          {this.props.children}
+        </IntlProvider>
+      );
+
+      // Render children to created element
+      ReactDOM.render(children, this.el);
+    };
+
     const targetDomNode = document.getElementById(this.el.id);
 
     // Check if we have already added map somewhere on the DOM
@@ -52,6 +87,7 @@ class ReusableMapContainer extends React.Component {
         // if no mountNode is found, append this outside SPA rendering tree (to document body)
         document.body.appendChild(this.el);
       }
+      renderChildren();
     } else {
       this.el.classList.remove(css.reusableMapHidden);
 
@@ -60,10 +96,14 @@ class ReusableMapContainer extends React.Component {
         // (but it's not yet moved to correct location of rendering tree).
         document.body.removeChild(this.el);
         this.mountNode.appendChild(this.el);
+
+        // render children and call reattach
+        renderChildren();
+        this.props.onReattach();
+      } else {
+        renderChildren();
       }
     }
-
-    ReactDOM.render(this.props.children, this.el);
   }
 
   render() {

src/components/SearchMap/SearchMap.js

@@ -1,10 +1,14 @@
 import React, { Component } from 'react';
 import { arrayOf, bool, func, number, string, shape } from 'prop-types';
+import { withRouter } from 'react-router-dom';
 import { withGoogleMap, GoogleMap } from 'react-google-maps';
 import classNames from 'classnames';
 import groupBy from 'lodash/groupBy';
 import isEqual from 'lodash/isEqual';
 import reduce from 'lodash/reduce';
+import routeConfiguration from '../../routeConfiguration';
+import { createResourceLocatorString } from '../../util/routes';
+import { createSlug } from '../../util/urlHelpers';
 import { types as sdkTypes } from '../../util/sdkLoader';
 import { propTypes } from '../../util/types';
 import { obfuscatedCoordinates } from '../../util/maps';
@@ -12,6 +16,7 @@ import { googleBoundsToSDKBounds } from '../../util/googleMaps';
 import { SearchMapInfoCard, SearchMapPriceLabel, SearchMapGroupLabel } from '../../components';
 import config from '../../config';
 
+import ReusableMapContainer from './ReusableMapContainer';
 import css from './SearchMap.css';
 
 const LABEL_HANDLE = 'SearchMapLabel';
@@ -97,11 +102,13 @@ const MapWithGoogleMap = withGoogleMap(props => {
     infoCardOpen,
     isOpenOnModal,
     listings,
-    onCloseAsModal,
     onIdle,
     onListingClicked,
+    onListingInfoCardClicked,
+    createURLToListing,
     onMapLoad,
     zoom,
+    mapComponentRefreshToken,
   } = props;
 
   const listingArraysInLocations = reducedToArray(groupedByCoordinates(listings));
@@ -126,6 +133,7 @@ const MapWithGoogleMap = withGoogleMap(props => {
           className={LABEL_HANDLE}
           listing={listing}
           onListingClicked={onListingClicked}
+          mapComponentRefreshToken={mapComponentRefreshToken}
         />
       );
     }
@@ -136,6 +144,7 @@ const MapWithGoogleMap = withGoogleMap(props => {
         className={LABEL_HANDLE}
         listings={listingArr}
         onListingClicked={onListingClicked}
+        mapComponentRefreshToken={mapComponentRefreshToken}
       />
     );
   });
@@ -144,9 +153,11 @@ const MapWithGoogleMap = withGoogleMap(props => {
   const openedCard = infoCardOpen ? (
     <SearchMapInfoCard
       key={listingsArray[0].id.uuid}
+      mapComponentRefreshToken={mapComponentRefreshToken}
       className={INFO_CARD_HANDLE}
       listings={listingsArray}
-      onClickCallback={onCloseAsModal}
+      onListingInfoCardClicked={onListingInfoCardClicked}
+      createURLToListing={createURLToListing}
     />
   ) : null;
 
@@ -191,7 +202,11 @@ export class SearchMapComponent extends Component {
 
     this.listings = [];
     this.googleMap = null;
+    this.mapReattachmentCount = 0;
     this.state = { infoCardOpen: null };
+
+    this.createURLToListing = this.createURLToListing.bind(this);
+    this.onListingInfoCardClicked = this.onListingInfoCardClicked.bind(this);
     this.onListingClicked = this.onListingClicked.bind(this);
     this.onMapClicked = this.onMapClicked.bind(this);
     this.onMapLoadHandler = this.onMapLoadHandler.bind(this);
@@ -214,10 +229,30 @@ export class SearchMapComponent extends Component {
     this.listings = [];
   }
 
+  createURLToListing(listing) {
+    const routes = routeConfiguration();
+
+    const id = listing.id.uuid;
+    const slug = createSlug(listing.attributes.title);
+    const pathParams = { id, slug };
+
+    return createResourceLocatorString('ListingPage', routes, pathParams, {});
+  }
+
   onListingClicked(listings) {
     this.setState({ infoCardOpen: listings });
   }
 
+  onListingInfoCardClicked(listing) {
+    if (this.props.onCloseAsModal) {
+      this.props.onCloseAsModal();
+    }
+
+    // To avoid full page refresh we need to use internal router
+    const history = this.props.history;
+    history.push(this.createURLToListing(listing));
+  }
+
   onMapClicked(e) {
     // Close open listing popup / infobox, unless the click is attached to a price label
     const labelClicked = hasParentWithClassName(e.nativeEvent.target, LABEL_HANDLE);
@@ -240,6 +275,7 @@ export class SearchMapComponent extends Component {
     const {
       className,
       rootClassName,
+      reusableContainerClassName,
       center,
       isOpenOnModal,
       listings: originalListings,
@@ -257,34 +293,44 @@ export class SearchMapComponent extends Component {
     const listings = coordinatesConfig.fuzzy
       ? withCoordinatesObfuscated(listingsWithLocation)
       : listingsWithLocation;
+    const infoCardOpen = this.state.infoCardOpen;
 
     const isMapsLibLoaded = typeof window !== 'undefined' && window.google && window.google.maps;
 
+    const forceUpdateHandler = () => {
+      this.mapReattachmentCount += 1;
+    };
+
     // container element listens clicks so that opened SearchMapInfoCard can be closed
     /* eslint-disable jsx-a11y/no-static-element-interactions */
     return isMapsLibLoaded ? (
-      <MapWithGoogleMap
-        containerElement={<div className={classes} onClick={this.onMapClicked} />}
-        mapElement={<div className={mapClasses} />}
-        center={center}
-        isOpenOnModal={isOpenOnModal}
-        listings={listings}
-        activeListingId={activeListingId}
-        infoCardOpen={this.state.infoCardOpen}
-        onListingClicked={this.onListingClicked}
-        onMapLoad={this.onMapLoadHandler}
-        onIdle={() => {
-          if (this.googleMap) {
-            onIdle(this.googleMap);
-          }
-        }}
-        onCloseAsModal={() => {
-          if (onCloseAsModal) {
-            onCloseAsModal();
-          }
-        }}
-        zoom={zoom}
-      />
+      <ReusableMapContainer className={reusableContainerClassName} onReattach={forceUpdateHandler}>
+        <MapWithGoogleMap
+          containerElement={<div className={classes} onClick={this.onMapClicked} />}
+          mapElement={<div className={mapClasses} />}
+          center={center}
+          isOpenOnModal={isOpenOnModal}
+          listings={listings}
+          activeListingId={activeListingId}
+          infoCardOpen={infoCardOpen}
+          onListingClicked={this.onListingClicked}
+          onListingInfoCardClicked={this.onListingInfoCardClicked}
+          createURLToListing={this.createURLToListing}
+          onMapLoad={this.onMapLoadHandler}
+          onIdle={() => {
+            if (this.googleMap) {
+              onIdle(this.googleMap);
+            }
+          }}
+          onCloseAsModal={() => {
+            if (onCloseAsModal) {
+              onCloseAsModal();
+            }
+          }}
+          zoom={zoom}
+          mapComponentRefreshToken={this.mapReattachmentCount}
+        />
+      </ReusableMapContainer>
     ) : (
       <div className={classes} />
     );
@@ -296,6 +342,7 @@ SearchMapComponent.defaultProps = {
   className: null,
   rootClassName: null,
   mapRootClassName: null,
+  reusableContainerClassName: null,
   bounds: null,
   center: new sdkTypes.LatLng(0, 0),
   activeListingId: null,
@@ -311,6 +358,7 @@ SearchMapComponent.propTypes = {
   className: string,
   rootClassName: string,
   mapRootClassName: string,
+  reusableContainerClassName: string,
   bounds: propTypes.latlngBounds,
   center: propTypes.latlng,
   activeListingId: propTypes.uuid,
@@ -323,8 +371,13 @@ SearchMapComponent.propTypes = {
   coordinatesConfig: shape({
     fuzzy: bool.isRequired,
   }),
+
+  // from withRouter
+  history: shape({
+    push: func.isRequired,
+  }).isRequired,
 };
 
-const SearchMap = SearchMapComponent;
+const SearchMap = withRouter(SearchMapComponent);
 
 export default SearchMap;

src/components/SearchMapInfoCard/SearchMapInfoCard.js

@@ -1,18 +1,14 @@
 import React, { Component } from 'react';
-import PropTypes from 'prop-types';
+import { arrayOf, bool, func, string } from 'prop-types';
 import { OverlayView } from 'react-google-maps';
 import { OVERLAY_VIEW } from 'react-google-maps/lib/constants';
 import { compose } from 'redux';
-import { withRouter } from 'react-router-dom';
 import { injectIntl, intlShape } from 'react-intl';
 import classNames from 'classnames';
 import config from '../../config';
-import routeConfiguration from '../../routeConfiguration';
 import { propTypes } from '../../util/types';
 import { formatMoney } from '../../util/currency';
 import { ensureListing } from '../../util/data';
-import { createResourceLocatorString } from '../../util/routes';
-import { createSlug } from '../../util/urlHelpers';
 import { ResponsiveImage } from '../../components';
 
 import css from './SearchMapInfoCard.css';
@@ -37,22 +33,14 @@ const getPixelPositionOffset = (width, height) => {
   return { x: -1 * (width / 2), y: -1 * (height + 3) };
 };
 
-const createURL = (routes, history, listing) => {
-  const id = listing.id.uuid;
-  const slug = createSlug(listing.attributes.title);
-  const pathParams = { id, slug };
-  return createResourceLocatorString('ListingPage', routes, pathParams, {});
-};
-
 // ListingCard is the listing info without overlayview or carousel controls
 const ListingCard = props => {
-  const { className, clickHandler, history, intl, isInCarousel, listing } = props;
+  const { className, clickHandler, intl, isInCarousel, listing, urlToListing } = props;
 
   const { title, price } = listing.attributes;
   const formattedPrice =
     price && price.currency === config.currency ? formatMoney(intl, price) : price.currency;
   const firstImage = listing.images && listing.images.length > 0 ? listing.images[0] : null;
-  const urlToListing = createURL(routeConfiguration(), history, listing);
 
   // listing card anchor needs sometimes inherited border radius.
   const classes = classNames(
@@ -69,8 +57,8 @@ const ListingCard = props => {
       href={urlToListing}
       onClick={e => {
         e.preventDefault();
-        // Handle click callbacks and use internal router
-        clickHandler(urlToListing);
+        // Use clickHandler from props to call internal router
+        clickHandler(listing);
       }}
     >
       <div
@@ -103,15 +91,10 @@ ListingCard.defaultProps = {
   className: null,
 };
 
-const { arrayOf, bool, func, shape, string } = PropTypes;
-
 ListingCard.propTypes = {
   className: string,
   listing: propTypes.listing.isRequired,
   clickHandler: func.isRequired,
-  history: shape({
-    push: func.isRequired,
-  }).isRequired,
   intl: intlShape.isRequired,
   isInCarousel: bool.isRequired,
 };
@@ -121,21 +104,17 @@ class SearchMapInfoCard extends Component {
     super(props);
 
     this.state = { currentListingIndex: 0 };
-    this.clickHandler = this.clickHandler.bind(this);
-  }
-
-  clickHandler(urlToListing) {
-    if (this.props.onClickCallback) {
-      this.props.onClickCallback();
-    }
-
-    // To avoid full page refresh we need to use internal router
-    const history = this.props.history;
-    history.push(urlToListing);
   }
 
   render() {
-    const { className, rootClassName, intl, history, listings } = this.props;
+    const {
+      className,
+      rootClassName,
+      intl,
+      listings,
+      createURLToListing,
+      onListingInfoCardClicked,
+    } = this.props;
     const currentListing = ensureListing(listings[this.state.currentListingIndex]);
     const geolocation = currentListing.attributes.geolocation;
 
@@ -185,9 +164,9 @@ class SearchMapInfoCard extends Component {
         <div className={classes}>
           <div className={css.caretShadow} />
           <ListingCard
-            clickHandler={this.clickHandler}
+            clickHandler={onListingInfoCardClicked}
+            urlToListing={createURLToListing(currentListing)}
             listing={currentListing}
-            history={history}
             intl={intl}
             isInCarousel={hasCarousel}
           />
@@ -202,22 +181,17 @@ class SearchMapInfoCard extends Component {
 SearchMapInfoCard.defaultProps = {
   className: null,
   rootClassName: null,
-  onClickCallback: null,
 };
 
 SearchMapInfoCard.propTypes = {
   className: string,
   rootClassName: string,
   listings: arrayOf(propTypes.listing).isRequired,
-  onClickCallback: func,
-
-  // from withRouter
-  history: shape({
-    push: func.isRequired,
-  }).isRequired,
+  onListingInfoCardClicked: func.isRequired,
+  createURLToListing: func.isRequired,
 
   // from injectIntl
   intl: intlShape.isRequired,
 };
 
-export default compose(withRouter, injectIntl)(SearchMapInfoCard);
+export default compose(injectIntl)(SearchMapInfoCard);

src/containers/SearchPage/SearchPage.js

@@ -251,9 +251,10 @@ export class SearchPageComponent extends Component {
             showAsModalMaxWidth={MODAL_BREAKPOINT}
             onManageDisableScrolling={onManageDisableScrolling}
           >
-            <div className={css.map}>
+            <div className={css.mapWrapper}>
               {shouldShowSearchMap ? (
                 <SearchMap
+                  reusableContainerClassName={css.map}
                   activeListingId={activeListingId}
                   bounds={bounds}
                   center={origin}

src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap

@@ -86,39 +86,13 @@ exports[`SearchPageComponent matches snapshot 1`] = `
       showAsModalMaxWidth={768}
     >
       <div>
-        <SearchMapComponent
+        <withRouter(SearchMapComponent)
           activeListingId={null}
-          bounds={null}
-          center={
-            LatLng {
-              "lat": 0,
-              "lng": 0,
-            }
-          }
-          className={null}
-          coordinatesConfig={
-            Object {
-              "circleOptions": Object {
-                "clickable": false,
-                "fillColor": "#c0392b",
-                "fillOpacity": 0.2,
-                "strokeColor": "#c0392b",
-                "strokeOpacity": 0.5,
-                "strokeWeight": 1,
-              },
-              "coordinateOffset": 500,
-              "fuzzy": false,
-              "fuzzyDefaultZoomLevel": 14,
-            }
-          }
           isOpenOnModal={false}
           listings={Array []}
-          mapRootClassName={null}
           onCloseAsModal={[Function]}
           onIdle={[Function]}
-          rootClassName={null}
           useLocationSearchBounds={true}
-          zoom={11}
         />
       </div>
     </withViewport(ModalInMobileComponent)>