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

feat: allow full circle in piechart #237

Merged
merged 3 commits into from
Jul 30, 2019

Conversation

scmmishra
Copy link
Contributor

@scmmishra scmmishra commented Jul 29, 2019

Solves #186

Explanation About What Code Achieves:

The PR adds a full circle check to pie chart and donut chart, this is required when a single data point is given to these charts, or when all but one data points have the value zero.

Using the SVGs arc path, one can draw 99.999% of the circle but not a complete circle, this doesn't work because the starting point and ending point of the arc is the same for a circle, so I guess the browser does not bother to draw the SVG, it's similar to drawing a line from (10, 10) to (10, 10) you get no line. Normally it would be wise to use the <circle> tag, but that may require bigger changes and can be a part of a different release, so right now we use a clever hack to solve this problem.

The d attribute will accept and draw all valid path command strings, The trick is to have two arcs, the second one picking up where the first left off and get back to the original arc start point. [ref]

Screenshots/GIFs:

Previously as mentioned here #186 the chart would not render at all. After the fix, the rendering in either cases works fine:

Pie Chart Donut Chart
With Single Data Screenshot 2019-07-30 at 9 56 07 AM Screenshot 2019-07-30 at 9 55 52 AM
With Multiple Data Screenshot 2019-07-30 at 9 56 42 AM Screenshot 2019-07-30 at 9 56 59 AM

@coveralls
Copy link

coveralls commented Jul 29, 2019

Pull Request Test Coverage Report for Build 112

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 38.158%

Totals Coverage Status
Change from base Build 107: 0.0%
Covered Lines: 29
Relevant Lines: 62

💛 - Coveralls

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