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

Added y axis label to Line- and Areagraph #1995

Merged
merged 8 commits into from Oct 9, 2019

Conversation

DrFaust
Copy link
Contributor

@DrFaust DrFaust commented Oct 1, 2019

This is a PR for issue #968

I placed the unit labels parallel to the axis and not at the top because it provided more space this way.
The second chart is now in GW.

Happy for feedpack!

@alixunderplatz
Copy link
Collaborator

@DrFaust cool! can you post a screenshot so we can see what it looks like?

@DrFaust
Copy link
Contributor Author

DrFaust commented Oct 1, 2019

@DrFaust cool! can you post a screenshot so we can see what it looks like?

screenshot

@alixunderplatz
Copy link
Collaborator

@DrFaust Thank you, well done! 👍

Two minor things that I noticed:

  1. The specific emissions unit is "g/kWh".

  2. A more general thing: In the related issue Units missing from left panel charts #968 it says to change the units of the middle chart to "GW" instead of "MW", which you have done. So for smaller zones with less production (islands like Aland, Bornholm, El Hierro,...), will we get decimal numbers now like "0.04 GW"? Sorry for asking, I can't run the code at the moment to check this. ;)
    I'm just thinking if it is better to dynamically change to MW if the maximum value on the chart doesn't exceed 1 GW. Will that be easy to implement?
    Any thoughts on this, @corradio & @martincollignon?

@corradio
Copy link
Member

corradio commented Oct 2, 2019

Agreed with @alixunderplatz for the two points.
If memory serves well, I think the tooltip already has this functionality of writing "GW" or "MW" depending on the scale:
image

Maybe you can reproduce that behaviour? Thanks for looking into this!

Max Zimmermann and others added 2 commits October 2, 2019 13:03
- corrected y axis label class name
- corrected y axis label for emissions
@DrFaust
Copy link
Contributor Author

DrFaust commented Oct 2, 2019

Good point @alixunderplatz I updated the first label text and will have a look at the tooltip like @corradio proposed.

@DrFaust
Copy link
Contributor Author

DrFaust commented Oct 7, 2019

Hi @alixunderplatz & @corradio. I changed it so that the unit of the second graph is dynamically set depending on the data. For a maximum with more than 1 GW its GW units otherwise it will be MW.

Would be glad if you would have time to take a look..

var formattingFactor = 1;
if (!that._displayByEmissions) {
// Display in GW or MW
formattingFactor = maxTotalValue > 1e3 ? 1e3 : 1;
Copy link
Member

Choose a reason for hiding this comment

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

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 tried to use d3.format before, but failed. Output of that function is a formatted string, which would not work here since we need to align the total dataset dynamically accordingly. Thats why this threshold is implemented.

Copy link
Member

@corradio corradio Oct 8, 2019

Choose a reason for hiding this comment

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

What do you mean exactly by "align the total dataset dynamically accordingly"? I'm trying to figure out if we can find a more elegant solution because some areas have kW scale data.

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 see. Sure 👍 . Added a new scalePower formatter.

Copy link
Contributor Author

@DrFaust DrFaust Oct 8, 2019

Choose a reason for hiding this comment

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

I meant that data and domain need to be scaled.

Copy link
Member

@corradio corradio left a comment

Choose a reason for hiding this comment

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

Last small challenge and we should be good to go! Thank you for your patience.

return (d.totalCo2Production + d.totalCo2Import + d.totalCo2Discharge) / 1e6 / 60.0;
}
})
maxTotalValue
Copy link
Member

Choose a reason for hiding this comment

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

can you multiply here by 1.1? It gives a bit of space between the graph and the axis (and then we keep the existing behavior)

@corradio corradio merged commit d4be668 into electricitymaps:master Oct 9, 2019
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

3 participants