Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[geo] add graticule #111

Merged
merged 1 commit into from
Aug 4, 2017
Merged

[geo] add graticule #111

merged 1 commit into from
Aug 4, 2017

Conversation

nschnierer
Copy link
Contributor

@nschnierer nschnierer commented Aug 1, 2017

Implemented graticule for projection's.

bildschirmfoto 2017-08-02 um 01 13 28

Draw graticule:

<Mercator
  data={world.features}
  scale={width / 630 * 100}
  translate={[width / 2, height / 2 + 50]}
  fill={() => '#8be4c5'}
  stroke={() => '#5fcfa7'}
  graticule={{
    stroke: 'rgba(85, 189, 213, 0.3)'
  }}
/>

Graticule lines in foreground of the map and outline:

<Mercator
  //...
  graticuleLines={{
    foreGround: true,
    stroke: (data) => 'rgba(85, 189, 213, 0.3)'
  }}
  graticuleOutline={{
    stroke: 'rgba(85, 189, 213, 0.3)'
  }}
/>

Any other ideas to implement graticule. I think graticule as prop is fine because a graticule without a projection doesn't make sense.

<Graticule lines={g => path(g)} {...graticuleLines} />}
{graticuleOutline &&
graticuleOutline.foreGround &&
<Graticule outline={g => path(g)} {...graticuleOutline} />}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any idea to resolve this repeating code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to change "foreGround" to "foreground"...

@hshoff
Copy link
Member

hshoff commented Aug 2, 2017

awesome, thanks @nschnierer! I'll put aside some time to review this more throughly in the next couple of days.

@hshoff hshoff self-requested a review August 2, 2017 18:15
@hshoff hshoff added this to the v0.0.135 milestone Aug 4, 2017
Copy link
Member

@hshoff hshoff left a comment

Choose a reason for hiding this comment

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

Looks good! I'm going to play around with a bit before publishing and see if there's a happy way for resolving the repeating code. I'll also do the foreGround => foreground change.

👍

@hshoff
Copy link
Member

hshoff commented Aug 4, 2017

looks like a test failed cause of typo. I'll update that after the merge.

    Expected value to be (using ===):
      "vx-mercator vx-new"
    Received:
      "vx-geo-mercator"

@hshoff hshoff merged commit 00658f9 into airbnb:master Aug 4, 2017
@nschnierer
Copy link
Contributor Author

Great. Need to add tests for the next pull request.

@nschnierer nschnierer deleted the geo-graticule branch August 5, 2017 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants