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

feature/analysis-location-selects #3513

Merged

Conversation

pjosh
Copy link
Contributor

@pjosh pjosh commented Jul 17, 2018

Overview

kapture 2018-07-17 at 16 41 29

@pjosh pjosh requested a review from edbrett July 17, 2018 14:43
Copy link
Contributor

@edbrett edbrett left a comment

Choose a reason for hiding this comment

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

Awesome. Not sure about putting country data inside the analysis as we might need it else where but I am happy with it for now.

@@ -67,6 +69,8 @@ class DataAnalysisMenu extends PureComponent {
analysis.showResults && <PolygonAnalysis />}
</div>
)}
<CountryDataProvider location={analysis.location} />
Copy link
Contributor

Choose a reason for hiding this comment

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

We are likely going to need country data outside of the analysis on the map in many cases. Think embed forms etc. We could leave it here, but maybe it is clearer at the page level?

@edbrett edbrett merged commit 8b35d54 into feature/remove-widget-dependencies Jul 20, 2018
@edbrett edbrett deleted the feature/analysis-location-selects branch July 20, 2018 09:36
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.

None yet

2 participants