Skip to content
This repository was archived by the owner on Mar 3, 2020. It is now read-only.

Feature: Map#19

Open
dleitee wants to merge 26 commits intomasterfrom
bia/master-with-map
Open

Feature: Map#19
dleitee wants to merge 26 commits intomasterfrom
bia/master-with-map

Conversation

@dleitee
Copy link
Copy Markdown
Contributor

@dleitee dleitee commented Jul 25, 2018

...

@beatriz1304 beatriz1304 self-assigned this Jul 30, 2018
@beatriz1304 beatriz1304 force-pushed the bia/master-with-map branch from adfcd48 to 79f4e65 Compare July 30, 2018 19:08
Copy link
Copy Markdown
Member

@douglasgimli douglasgimli left a comment

Choose a reason for hiding this comment

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

Awesome work, small inline comments :) 👏

import Mapbox from '_components/organisms/mapbox-map'
import MarkerList from '_components/molecules/marker-list'

const road = 'ROADMAP'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't you create an enum of the options and receive it as prop?

)}
</Fragment>
) : (
<div>Loading map</div>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe you should receive this message as a prop too, because the app could be in Portuguese :)


class Mapbox extends Component {
state = {
viewport: {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this hardcoded as state and not as prop values?

@@ -0,0 +1,9 @@
export const isBrowser = typeof window !== 'undefined'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants