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/cumulative emissions widget #3692

Merged
merged 26 commits into from
Feb 14, 2019

Conversation

dfrico
Copy link
Contributor

@dfrico dfrico commented Jan 24, 2019

Overview

Weekly forest disturbance alerts to track forest loss and emissions. This widget features a line chart showing cumulative emissions and area of loss. It replicates the behaviour of the glad alerts widget from climate GFW.

Demo

image

image

@dfrico dfrico requested a review from edbrett January 24, 2019 10:45
@dfrico dfrico self-assigned this Jan 24, 2019
@dfrico dfrico requested a review from benlaken January 24, 2019 10:45
@dfrico dfrico added staging Any PR expected to be kept on staging develop Any PR that is ready to be merged to develop labels Feb 6, 2019
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.

Great work! A complicated one! Only a small change needed to make sure we don't have duplicate keys.

@@ -0,0 +1,50 @@
export default {
widget: 'firesAlerts',
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to change otherwise we have two widgets with the same

analysis: true,
types: ['country'],
admins: ['adm0'],
layers: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

if no layers remove this key.

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.

Excellent work. Only some small changes to make this one unique 🌷

if (!data || isEmpty(data)) return null;

const { variable } = settings;
const target = variable === 'cumulative_emissions' ? 265000000 : 988880;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we clarify what these numbers are and place them as CONSTS at the top.

@edbrett edbrett merged commit c6b3d2d into develop Feb 14, 2019
@edbrett edbrett deleted the feature/cumulative-emissions-widget branch February 14, 2019 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
develop Any PR that is ready to be merged to develop 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