[geo] add package geo #105

nschnierer · 2017-07-23T00:17:08Z

I want to create a new package vx/geo with the ability to draw geoJSON/TopoJSON with different projections. Under the hood the d3-geo library.

This pull request isn't ready to merge now. The documentation isn't complete and a example is missing. I want to finish it in few days. However I think it's good time to get feedback and to review my code.

First example:

The example for vx-demo will be more beautiful ;)

render() {
  const width = 800;
  const height = 600;

  const world = topoJsonClient.feature(topology, topology.objects.units).features;

  return (
    <svg width={width} height={height}>
      <Mercator
        data={world}
        scale={110}
        translate={[width / 2, height / 2]}
        fill="#bbbbbb"
        projectionFunc={projection => {
          console.log('Get projection to use invert', projection);
        }}
        onClick={data => event => {
          console.log(`Clicked country: ${data.id}`);
        }}
      />
    </svg>
  );
}

I really like the vx concept. It's so much easier without d3. Good work ;)

Flaque · 2017-07-23T01:26:12Z

packages/vx-geo/src/components/Projection.js

+  );
+}
+
+Projection.propTypes = {


<3 <3 <3 Proptypes! :D

Flaque · 2017-07-23T01:28:50Z

packages/vx-geo/src/components/Projection.js

+  fitSize: PropTypes.array,
+  centroid: PropTypes.func,
+  className: PropTypes.string
+};


I might be wrong about this, but does Javascript let you add stuff to something "after" you export it? If so => COOL! :D

yup! should be fine https://github.com/hshoff/vx/blob/master/packages/vx-pattern/src/patterns/Pattern.js

Looks like I haven't been consistent with my .propTypes placement. Some packages have them above the export others below... at some point I should pick one.

I like the pattern to place it after it (like https://github.com/facebook/prop-types#usage). Maybe it's better to export the function after that, but I'm not sure.

Flaque · 2017-07-23T01:29:11Z

This is super super cool! Let me know if you need any help with anything <3

Also: Your code is clean and nice <3

hshoff · 2017-07-23T01:33:05Z

packages/vx-geo/src/components/Projection.js

+          <path
+            className={classnames(`vx-${projection}`, className)}
+            d={path(feature)}
+            {...additionalProps(restProps, feature)}


we will likely want to pass the centroid and maybe the index i in the data part here.

something like we did with <Arc /> in @vx/shape: https://github.com/hshoff/vx/blob/master/packages/vx-shape/src/shapes/Arc.js#L44

Ok, good idea.

hshoff · 2017-07-23T01:38:09Z

packages/vx-geo/src/util/additionalProps.js

+    ret[cur] = callOrValue(restProps[cur], data);
+    return ret;
+  }, {});
+}


this comment is just a reminder for myself to follow up on #69

hshoff · 2017-07-23T01:40:01Z

packages/vx-geo/src/index.js

@@ -0,0 +1,3 @@
+export { default as Albers } from './components/Albers';
+export { default as Mercator } from './components/Mercator';
+export { default as Orthographic } from './components/Orthographic';


minor thing, could we rename the ./components/dir to ./projections/? this will help match convention with the other packages.

@hshoff Yeah, sure. It think projections will be great. Is it a good idea to place the Projection.js into this folder. There is no export for Projection.

hshoff · 2017-07-23T01:41:42Z

packages/vx-geo/src/components/Projection.js

+          {centroid && centroid(path.centroid(feature), feature)}
+        </g>
+      )}
+      {projectionFunc && projectionFunc(currProjection)}


What's projectionFunc for? just an outlet for rendering out something based on the currProjection?

@hshoff Might be helpful to use invert (https://github.com/d3/d3-geo#projection_invert) x and y coordinates back to longitude and latitude. Not a good solution because you can set projection settings with props like scale, translate. Any ideas?

hshoff · 2017-07-23T01:47:04Z

Fantastic work @nschnierer! A really great addition to vx thanks for making this happen.

Looking forward to the gallery tile. Let me know if things get confusing in vx-demo, I've been putting off cleaning it up...

nschnierer · 2017-07-23T08:20:24Z

@hshoff @Flaque Thanks a lot for the feedback ;) I will finish it soon.

nschnierer · 2017-07-23T23:16:56Z

`vx/geo`

Finished the work. I think that is a good result for the first release ;)

`<Mercator />`

The demo uses a topology with low resolution (110 metre, ~213kb).

`<Orthographic />`

`<Albers />`

I hope the color of the demo is fine ✌️

Next steps:

Add graticule
Add more projection's
Add pictures to readme ✅
Rethinking about the projectionFunc property 🤔

williaster · 2017-07-23T23:38:41Z

packages/vx-geo/src/projections/Projection.js

+
+// TODO: Implement all projections of d3-geo
+const projectionMapping = {
+  orthographic: geoOrthographic(),


if there are multiple instances on a page of one type of projection, would singleton instances result in mixed or lingering config / props being set on them (e.g., if one set clipAngle and another didn't, would they both have the specified clipAngle)?

you could get around it by making these factories as opposed to singletons?

@williaster Oh, good point. I think factories are better. So, maybe like this:

const projectionMapping = { orthographic: () => geoOrthographic(), // ... };

Or better to use function instead of arrow function?

arrow looks good to me. good catch @williaster!

It's done 👍

williaster · 2017-07-23T23:41:07Z

packages/vx-geo/src/projections/Projection.js

+          </g>
+        );
+      })}
+      {/* TODO: Maybe find a different way to pass projection function to use for example invert */}


yeah it's a bummer there is no projection.copy() utility analogous to scales 🤔

hshoff · 2017-07-24T00:32:55Z

packages/vx-geo/src/projections/Mercator.js

+import React from 'react';
+import Projection from './Projection';
+
+export default function Mercator({ ...restProps }) {


should have caught this in the first review but this is an unnecessary rest here.

let's go with:

export default function Mercator(props) { return <Projection projection="mercator" {...props} />; }

and we'll need to update the other projections. this isn't a blocker. happy to change this after a merge.

Ugh... Solved it. @hshoff

hshoff · 2017-07-24T00:42:36Z

LGTM! Thanks again @nschnierer! 🏆

Flaque · 2017-07-24T01:32:47Z

Open Source is great <3

williaster · 2017-07-24T05:41:08Z

This is a great add, nice work @nschnierer 💯!

[geo] add package geo

027e4bb

Flaque reviewed Jul 23, 2017

View reviewed changes

packages/vx-geo/src/components/Projection.js Outdated

);

}

Projection.propTypes = {

Copy link

Contributor

Flaque Jul 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3 <3 <3 Proptypes! :D

Flaque reviewed Jul 23, 2017

View reviewed changes

hshoff reviewed Jul 23, 2017

View reviewed changes

hshoff added this to the v0.0.132 milestone Jul 23, 2017

nschnierer added 4 commits July 23, 2017 12:12

[geo] Rename components folder to projections.

b0e8368

[geo] Add geo mercator example to vx-demo

a60ab11

[geo] Pass centroid and index to parent component

b02925f

[geo] Update readme and clean code.

6708e7c

[geo] Add pictures to readme.

0dfe7fd

williaster reviewed Jul 23, 2017

View reviewed changes

[geo] Using factories instead of singletons to map projections.

e9b8627

hshoff reviewed Jul 24, 2017

View reviewed changes

[geo] Remove unnecessary rests.

ac12eb8

hshoff merged commit 9a8b923 into airbnb:master Jul 24, 2017

nschnierer deleted the nschnierer-geo branch July 25, 2017 06:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[geo] add package geo #105

[geo] add package geo #105

nschnierer commented Jul 23, 2017 •

edited

Loading

Flaque Jul 23, 2017

Flaque Jul 23, 2017

hshoff Jul 23, 2017 •

edited

Loading

nschnierer Jul 23, 2017

Flaque commented Jul 23, 2017 •

edited

Loading

hshoff Jul 23, 2017

nschnierer Jul 23, 2017

hshoff Jul 23, 2017

hshoff Jul 23, 2017

nschnierer Jul 23, 2017

hshoff Jul 23, 2017

nschnierer Jul 23, 2017 •

edited

Loading

hshoff commented Jul 23, 2017 •

edited

Loading

nschnierer commented Jul 23, 2017

nschnierer commented Jul 23, 2017 •

edited

Loading

williaster Jul 23, 2017

nschnierer Jul 23, 2017 •

edited

Loading

hshoff Jul 24, 2017

nschnierer Jul 24, 2017

williaster Jul 23, 2017

hshoff Jul 24, 2017 •

edited

Loading

nschnierer Jul 24, 2017

hshoff commented Jul 24, 2017 •

edited

Loading

Flaque commented Jul 24, 2017

williaster commented Jul 24, 2017

[geo] add package geo #105

[geo] add package geo #105

Conversation

nschnierer commented Jul 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hshoff Jul 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Flaque commented Jul 23, 2017 • 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

nschnierer Jul 23, 2017 • edited Loading

Choose a reason for hiding this comment

hshoff commented Jul 23, 2017 • edited Loading

nschnierer commented Jul 23, 2017

nschnierer commented Jul 23, 2017 • edited Loading

vx/geo

<Mercator />

<Orthographic />

<Albers />

Choose a reason for hiding this comment

nschnierer Jul 23, 2017 • 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

hshoff Jul 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hshoff commented Jul 24, 2017 • edited Loading

Flaque commented Jul 24, 2017

williaster commented Jul 24, 2017

nschnierer commented Jul 23, 2017 •

edited

Loading

hshoff Jul 23, 2017 •

edited

Loading

Flaque commented Jul 23, 2017 •

edited

Loading

nschnierer Jul 23, 2017 •

edited

Loading

hshoff commented Jul 23, 2017 •

edited

Loading

nschnierer commented Jul 23, 2017 •

edited

Loading

`vx/geo`

`<Mercator />`

`<Orthographic />`

`<Albers />`

nschnierer Jul 23, 2017 •

edited

Loading

hshoff Jul 24, 2017 •

edited

Loading

hshoff commented Jul 24, 2017 •

edited

Loading