feat(geo): add simple GoogleMaps widget [PART-2] #1189

samouss · 2018-04-25T07:44:53Z

Summary

This is the second part of the GeoSearch widget. Those are the first building blocks for the widget. It only displays hits on the map. When you trigger a search with the SearchBox the map move accordingly. Right now this is it. The refine behaviour is not implemented it will comes after.

This PR includes:

<GoogleMapsLoader>: the usage of this component is not required. You can load the map with a different strategy it doesn't impact the widget.
<Provider>: it's a private component inside the widget to bind the connector to a render props. This component will hold all the logic related to the UI state in a next PR.
<GeoSearch>: it's the main component for the GeoSearch widget. It wraps the Provider & GoogleMaps components together.
<GoogleMaps>: is s wrapper around the GoogleMaps API. His responsibility is to manage the instance of the map and expose it through the context.
<Marker>: is a wrapper around the built-in GoogleMaps Marker. It retrieves the instance of map from the context exposed by GoogleMaps.

Usage

import { GoogleMapsLoader, GeoSearch, Marker } from 'react-instantsearch-dom-geo'; // name will change

<GoogleMapsLoader apiKey="GOOGLE_MAPS_API_KEY">
  {google => (
    <GeoSearch google={google}>
      {({ hits }) => (
        <Fragment>
          {hits.map(hit => <Marker key={hit.objectID} hit={hit} />)}
        </Fragment>
      )}
    </GeoSearch>
  )}
</GoogleMapsLoader>;

Result

You can use the widget in Storybook.

algobot · 2018-04-25T07:49:20Z

Deploy preview for react-instantsearch ready!

Built with commit 387cd23

https://deploy-preview-1189--react-instantsearch.netlify.com

Haroenv · 2018-04-25T08:41:19Z

packages/react-instantsearch-dom-geo/package.json

@@ -0,0 +1,67 @@
+{
+  "name": "react-instantsearch-dom-geo",


name is a bit odd, but descriptive

It's not the final name, more a placeholder. Any inputs / ideas are welcome!

react-instantsearch-maps-dom sounds better to me 😀

Yes but since the next package will be called react-instantsearch-dom it makes more sense to have react-instantsearch-dom-maps WDYT?

I'd put the specifier of the platform last still, since you'll think "I want the react instantsearch package for maps which works on web"

I would go for react-instantsearch-dom-geosearch (endname = widget name). About having dom before or after I have no preference, any examples of libraries named react-dom-xx vs react-xx-dom?

I followed the same name as React Router react-router-dom, react-router-native. Most of the time the libs used for React DOM are just named react-xxx. But for React Native it's common to have react-native-xxx. Since we support both, make sense to follow the convention react-{platform}-xxx.

Haroenv · 2018-04-25T08:41:53Z

packages/react-instantsearch-dom-geo/package.json

+    "jest": "22.4.3",
+    "react": "16.3.2",
+    "react-dom": "16.3.2",
+    "rollup": "0.58.0",


are those dependencies not top-level ones? or do you prefer the duplication?

In the next project structure every package is self-contained so I heavily rely on the deduplication of the Workspaces.

Haroenv · 2018-04-25T08:42:11Z

packages/react-instantsearch-dom-geo/package.json

+      "enzyme-to-json/serializer"
+    ]
+  },
+  "bundlesize": []


maybe add that here too?

I will add it when I have the final size of the package.

Haroenv · 2018-04-25T08:42:33Z

packages/react-instantsearch-dom-geo/src/GeoSearch.js

+import PropTypes from 'prop-types';
+// @TODO: Update this import when the package is correctly split:
+// import { createClassNames } from 'react-instantsearch-dom';
+import createClassNames from '../../react-instantsearch/src/components/createClassNames';


should this not be react-instantsearch/src/components/createClassNames?

With an absolute import the file will be the original one (non transpiled). Webpack will complain because we don't the node_modules files are not transpiled. In the next version this issue is fixed with a watch command that build the packages. For now, we can let the import like that.

Haroenv · 2018-04-25T08:46:20Z

packages/react-instantsearch-dom-geo/src/GeoSearch.js

+
+class GeoSearch extends Component {
+  static propTypes = {
+    google: PropTypes.object.isRequired,


I wonder if it would make sense to accept the google.maps object instead of the whole google object 🤔

At some point if we need an object that is not nested under the maps namespace it will be a breaking change. Not sure that will happen one day but anyway the overhead is inexistent.

Haroenv · 2018-04-25T08:49:24Z

packages/react-instantsearch-dom-geo/src/GoogleMaps.js

+    });
+
+    this.setState(() => ({
+      isMapAlreadyRender: true,


isMapAlreadyRendered

Replace with isMapReady, makes more sense at the end (especially with the future changes).

Haroenv · 2018-04-25T08:50:44Z

packages/react-instantsearch-dom-geo/src/GeoSearch.js

+    const { google } = this.props;
+
+    const latLngBounds = hits.reduce(
+      (acc, hit) => acc.extend(hit._geoloc),


just a question: where does extend come from?

ah, I see, google maps bounds

Haroenv · 2018-04-25T08:59:56Z

packages/react-instantsearch-dom-geo/src/GeoSearch.js

+
+    return (
+      <GoogleMaps
+        testID="GoogleMaps"


what's this used for? when you mock GoogleMaps?

Nope it avoid with Enzyme to target the component by reference .find(GoogleMaps) or by className or more generally everything that is subject to change in a test case .find('[testID="GoogleMaps"]') won't change at all.

https://blog.kentcdodds.com/making-your-ui-tests-resilient-to-change-d37a6ee37269

Haroenv · 2018-04-25T09:00:32Z

packages/react-instantsearch-dom-geo/src/GoogleMaps.js

+  };
+
+  static childContextTypes = {
+    [GOOGLE_MAPS_CONTEXT]: PropTypes.shape({


would be nice t here use the new context, and give it as a limitation to use new React, maybe?

The full version of InstantSearch is compatible with lower version than React 16.3. Plus I didn't find a official substitute for the new API (other than create-react-context). For now I prefer to stick with the old API. In the next version of the lib the question worth asking.

Haroenv · 2018-04-25T10:06:51Z

packages/react-instantsearch-dom-geo/src/GoogleMapsLoader.js

+import PropTypes from 'prop-types';
+import injectScript from 'scriptjs';
+
+class GoogleMapsLoader extends Component {


* feat(geo): add Redo component [PART-4] (#1201) * feat(geo): add Control component [PART-5] (#1205) * feat(geo): refactor & external update [PART-6] (#1207) * feat(geo): custom Marker [PART-7] (#1214) * feat(geo): add Marker & HTMLMarker interactions [PART-8] (#1227) * feat(geo): add stories [PART-9] (#1236) * feat(geo): add docs [PART-10] (#1289)

* feat(geo): add simple GoogleMaps widget [PART-2] (#1189) * feat(geo): add refine on interaction [PART-3] (#1192) * feat(geo): add Redo component [PART-4] (#1201) * feat(geo): add Control component [PART-5] (#1205) * feat(geo): refactor & external update [PART-6] (#1207) * feat(geo): custom Marker [PART-7] (#1214) * feat(geo): add Marker & HTMLMarker interactions [PART-8] (#1227) * feat(geo): add stories [PART-9] (#1236) * feat(geo): add docs [PART-10] (#1289)

<a name="5.2.0"></a> # [5.2.0](v5.2.0-beta.2...v5.2.0) (2018-07-04) ### Bug Fixes * **translatable:** avoid create a new function on every render ([#1383](#1383)) ([1285b3b](1285b3b)) ### Features * **core:** export translatable ([#1351](#1351)) ([6d5a89d](6d5a89d)) * **maps:** add connector & widget ([#1171](#1171)) ([16e288a](16e288a)), closes [#1189](#1189) [#1192](#1192) [#1201](#1201) [#1205](#1205) [#1207](#1207) [#1214](#1214) [#1227](#1227) [#1236](#1236) [#1289](#1289)

samouss requested review from vvo, Haroenv and bobylito and removed request for vvo April 25, 2018 07:45

samouss added the ⏳ In progress label Apr 25, 2018

Haroenv reviewed Apr 25, 2018

View reviewed changes

samouss force-pushed the feat/geo-part-2-simple-map branch 2 times, most recently from a3edc66 to 0af5667 Compare May 2, 2018 08:10

samouss force-pushed the feat/geo-part-2-simple-map branch from 0af5667 to 3dc3e71 Compare May 4, 2018 08:41

samouss force-pushed the feat/geo-part-2-simple-map branch 2 times, most recently from f0cec76 to 05f6e6d Compare May 14, 2018 13:08

samouss force-pushed the feat/geo-part-one branch 2 times, most recently from 6c0c4c5 to 1e1201d Compare June 19, 2018 13:40

samouss force-pushed the feat/geo-part-2-simple-map branch from 05f6e6d to 5d66ea9 Compare June 19, 2018 13:50

samouss force-pushed the feat/geo-part-one branch from 1e1201d to 23769fc Compare June 19, 2018 16:25

samouss force-pushed the feat/geo-part-2-simple-map branch from 5d66ea9 to 479cdc1 Compare June 19, 2018 16:26

samouss force-pushed the feat/geo-part-one branch from 23769fc to e6f3918 Compare June 20, 2018 07:51

samouss added 11 commits June 20, 2018 09:51

chore: setup package boilerplate

8b2bd46

chore: add scriptjs

6eaac28

feat(GoogleMapsLoader): add component

901a447

feat(stories): use GoogleMapsLoader

3b6fe97

feat(GeoSearch): add Provider

c28e741

chore: update IS.css to 7.1.0

c7c4c79

test(GoogleMaps): add utils for mock

6f2d767

feat(GoogleMaps): add basic component

0a8bbae

feat(GeoSearch): add basic component

dbc23fb

feat(GeoSearch): add default & initial stories

e95010c

fix(GoogleMaps): add missing defaultProps

a8f571d

samouss added 16 commits June 20, 2018 09:51

feat(GoogleMaps): expose google & instance through context

04599d2

fix(GeoSearch): move options from GoogleMaps to GeoSearch

2c603bd

feat(Marker): add basic component

f724859

feat(GeoSarch): add Marker in stories

6dd51f8

refactor(stories): only render image when defined

c80b3a5

refactor(GeoSearch): use propTypes for latLng & boundingBox

14b19ad

test(GeoSearch): update import for GoogleMaps mock

61e758a

fix(GoogleMaps): replace isMapAlreadyRender with isMapReady

cecc91c

fix(GoogleMaps): target the correct for the map

3ae6411

refactor(maps): use dom import

21f4d1a

fix(stories): use dom & maps import

857002c

chore(maps): setup package

8451cf6

fix: add react-instantsearch-dom-maps as top level devDepdendency

5b06f88

chore(maps): use private package to avoid publish

624ce22

chore(geo): move bundlesize on root package

79928b4

chore: remove useless bundlesize dependency on geo package

7dfc345

samouss force-pushed the feat/geo-part-2-simple-map branch from 125f026 to 7dfc345 Compare June 20, 2018 07:58

samouss merged commit 33e22ca into feat/geo-part-one Jun 20, 2018

samouss deleted the feat/geo-part-2-simple-map branch June 20, 2018 19:32

samouss removed the ⏳ In progress label Jun 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(geo): add simple GoogleMaps widget [PART-2] #1189

feat(geo): add simple GoogleMaps widget [PART-2] #1189

samouss commented Apr 25, 2018 •

edited

Loading

algobot commented Apr 25, 2018 •

edited

Loading

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 26, 2018

samouss Apr 26, 2018

Haroenv Apr 26, 2018

vvo May 15, 2018

samouss May 15, 2018

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 25, 2018

Haroenv Apr 25, 2018

Haroenv Apr 25, 2018

samouss Apr 26, 2018 •

edited

Loading

Haroenv Apr 25, 2018

samouss Apr 26, 2018

Haroenv Apr 25, 2018

feat(geo): add simple GoogleMaps widget [PART-2] #1189

feat(geo): add simple GoogleMaps widget [PART-2] #1189

Conversation

samouss commented Apr 25, 2018 • edited Loading

algobot commented Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouss Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samouss commented Apr 25, 2018 •

edited

Loading

algobot commented Apr 25, 2018 •

edited

Loading

samouss Apr 26, 2018 •

edited

Loading