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: biomass loss widget + woody biomass analysis #3664

Merged
merged 13 commits into from
Dec 4, 2018

Conversation

dfrico
Copy link
Contributor

@dfrico dfrico commented Nov 27, 2018

Overview

Updates the old Climate emissions widget to add the following:

  • new colours to match the associated layer
  • show widget in map when layer active
  • allow widget to be viewable for wdpas, geostore, and use cases

Additional features:

  • Reduce analysis fetches by filtering endpoints by active widgets
  • updates widgets to have default labels for wdpa, geostore, use

Copy link
Contributor

@benlaken benlaken left a comment

Choose a reason for hiding this comment

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

Widget is working well and looking good. Only exception is that we seem to have a bug on the view on map button.

screenshot 2018-11-27 at 15 48 34

view on map error in console:
screenshot 2018-11-27 at 15 54 23

Side note: I am also seeing a console bug on the climate-watch widget (historical emissions). This is an important one for the climate tab, and it seems like it is the same error on production and locally - we should fix that if we can, as it will also be under scrutiny on the Dec 5th event too.

:
http://localhost:5000/dashboards/country/BRA?category=climate

screenshot 2018-11-27 at 15 51 25

@edbrett
Copy link
Contributor

edbrett commented Nov 28, 2018

@benlaken you can now see the widget inside the map when analysing the area, so I would say this PR is complete. Can you review?

@dfrico
Copy link
Contributor Author

dfrico commented Nov 28, 2018

@benlaken you can now see the widget inside the map when analysing the area, so I would say this PR is complete. Can you review?

We're having some bugs when trying to analyze a geometry or polygon for this layer (in the map). Seems like the widget never renders - just infinite spinner.

screenshot 2018-11-28 at 16 02 17

screenshot 2018-11-28 at 15 59 07

@edbrett
Copy link
Contributor

edbrett commented Nov 29, 2018

@benlaken @blayhem updated! But I think the endpoint is failing to regularly for this to be usable. You have all the analysis types working now. Also the sentences no longer make sense as WDPA, custom areas etc dont really have nice names.

@benlaken
Copy link
Contributor

@edbrett this lgtm - true the analysis fails often, but that is mostly out of our hands - I can possibly tweak the params of the micro-service to make it less accurate but more reliable, and try and find a balance. Seems to work well apart from that.
We could also improve the dynamic sentences in the analysis, but I think that is something that is a more general issue.
I suggest we go ahead and merge this.
screenshot 2018-11-30 at 13 08 20

Copy link
Contributor

@benlaken benlaken left a comment

Choose a reason for hiding this comment

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

Working well 👍

@edbrett edbrett changed the title Tree loss widget working with tooltip, missing map layer integration Feature: biomass loss widget + woody biomass analysis Nov 30, 2018
@edbrett edbrett added WIP Any PR that should not be merged to develop staging Any PR expected to be kept on staging labels Nov 30, 2018
@edbrett edbrett removed the WIP Any PR that should not be merged to develop label Dec 4, 2018
@edbrett edbrett merged commit 00b55db into develop Dec 4, 2018
@edbrett edbrett deleted the feature/tree-loss-widget branch December 4, 2018 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging Any PR expected to be kept on staging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants