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

Add Pie Charts! #28

Merged
merged 3 commits into from
Nov 4, 2017
Merged

Add Pie Charts! #28

merged 3 commits into from
Nov 4, 2017

Conversation

sheweichun
Copy link

Explanation About What Code Achieves:
  • I add pie chart , you can see it in the demo, I add a pie demo in the create chart section
Steps To Test:
  • you can see it in the create chart section
TODOs:
  • None
    support label showing in the pie chart
    support annular pie chart

@pratu16x7 pratu16x7 self-assigned this Nov 3, 2017
@pratu16x7
Copy link
Contributor

pratu16x7 commented Nov 3, 2017

Thanks so much for this! 🎉 (Meanwhile, have updated PR guidelines to include screenshots as well.) Will put one here for all to see:

pie chart contri

We were all for percentage charts, as they are easier to read and don't clump colors (probably the reason GitHub ignores them as well): http://www.storytellingwithdata.com/blog/2011/07/death-to-pie-charts

screen shot 2017-11-03 at 6 39 01 pm

Even so, what does everyone think? 😃

@pratu16x7 pratu16x7 changed the title add pie chart Add Pie Charts! Nov 3, 2017
@achillesrasquinha
Copy link
Contributor

Even I prefer percentages over pies, this does not much harm with the tooltip on. I'd pass.

@sheweichun
Copy link
Author

after you show the screenshot, I found some thing wrong with the hover effect,it has some deviation,I fix it in my local repository
kapture 2017-11-04 at 0 06

@pratu16x7
Copy link
Contributor

Cool! Be sure to rebase your branch as well :)

@pratu16x7 pratu16x7 merged commit 1f50544 into frappe:master Nov 4, 2017
@achillesrasquinha achillesrasquinha mentioned this pull request Nov 5, 2017
3 tasks
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