3
\$\begingroup\$

I've made a small js/css/html module which purpose is to fill a pie chart up to a given percentage. Basically it could be used for instance as a filling pie chart that keep tracks of the amount of donations for a non-profit organization.

enter image description here

Let's sum up briefly what it does. Basically I have two svg circles, the base color and the fill color. Javascript will get the fill circle and give it a stroke-dasharray that corresponds to the percentage value (based on the circle area). Basically the fill circle's stroke will animate up to a given value.

The main advantage is that my chart can be customized easily on those parameters : the base pie color, the filling color, the number color, the pie shadow (or its absence) and the dimension of the pie. Also, you can use as many pie charts as you want on a single page, each one only need a different id/class as you see fit.

The percentage to which the pie will fill is stocked in a html data-id. So it's very easy to use with something like PHP. Also, there is no need to write a single line in javascript, mostly everything is in the SCSS file (you still need to enter 2 values in the html for the svg circles dimensions).

The problem is I'm not that good at writing javascript so I'm pretty sure it could be more efficient. Also, the SCSS might be better, I'd like to know your opinion on that.

So if you want to review only the javascript or only the SCSS, that's very fine by me. Maybe even the whole concept of my module is flawed, if you want to comment on the big picture

Here is my codepen to see the pie chart in action and fiddle with the code

Here is the html

<figure id="pie" data-percentage="75" data-behavior="pie-chart">
  <svg>
    <!-- Radius has to be svg diameter divided by 4 -->
    <!-- Cx and cy have to be svg diameter divided by 2 -->
    <circle r="50" cx="100" cy="100"/>
    <circle r="50" cx="100" cy="100"/>
  </svg>
  <div class="receiver"></div>
</figure>

Here is the SCSS, as you can see I tried my best to stay modular. It's made so you can create any number of pie charts

// Base styles for every pie chart
%pie-base {
  position: relative;
  display: inline-block;
  margin: 0 auto;
  vertical-align: middle;
  border-radius: 50%;

  svg {
    display: block;
    margin: 0 auto;
    transform: rotate(-90deg);
    shape-rendering: geometricPrecision;
  }

  circle {
    shape-rendering: geometricPrecision;
  }

  // This is where the percentage is shown
  .receiver {
    position: absolute;
    top: 0;
    font-family: Helvetica;
    font-size: 2rem;
    text-align: center;

    &:after {
      content: '%';
    }
  }
}

// Custom styles for pie charts, 5 parameters are required
@mixin pie($base-color, $fill-color, $number-color, $shadow, $diameter) {
  $easing: 2000ms cubic-bezier(0.215, 0.61, 0.355, 1);
  $pi: 3.14159;
  @extend %pie-base;

  width: $diameter;
  box-shadow: $shadow;

  circle:nth-child(1) {
    fill: $base-color;
    stroke: $base-color;
    stroke-width: $diameter / 2 - 0.5;
    stroke-dasharray: ($pi * 2 * $diameter / 4) ($pi * 2 * $diameter / 4);
  }

  circle:nth-child(2) {
    transition: stroke-dasharray $easing;
    fill: $base-color;
    stroke: $fill-color;
    stroke-width: $diameter / 2;
    stroke-dasharray: 0 ($pi * 2 * $diameter / 4);
  }

  svg {
    width: $diameter + px;
    height: $diameter + px;
  }

  .receiver {
    width: $diameter + px;
    height: $diameter + px;
    line-height: $diameter + px;
    color: $number-color;
  }
}

// Declaring the pie parameters
$pie-param: (
  'base-color': hsl(210, 40%, 50%),
  'fill-color': hsl(90, 40%, 61%),
  'number-color': hsl(0, 0%, 100%),
  'shadow': 2px 2px 10px 0px rgba(0, 0, 0, 0.36),
  'diameter': 200
);

// Calling the pie
#pie {
  @include pie($pie-param...);
}

Now for the JS, as you can see it's made so any number of pie charts will be ok

'use strict';
/* global jQuery */

/**
 * Animated pie chart
 * @require jQuery v1.7+
 * @require jQuery-numerator v0.2+
 */

(function($) {

  // Custom easing
  $.easing.jswing = $.easing.swing;

  $.extend($.easing, {
    easeOutCubic: function(x, t, b, c, d) {
      return c * ((t = t / d - 1) * t * t + 1) + b;
    }
  });

  // Numbering animation
  function numerate(elem, value) {
    $(elem).numerator({
      easing: 'easeOutCubic',
      duration: 2000,
      delimiter: ' ',
      rounding: 0,
      toValue: value
    });
  };

  // Main function
  function makePieChart(elem) {

    // Local variables
    var percentage = $(elem).data('percentage');
    var $filling = $('circle:nth-child(2)', elem);
    var total = 2 * Math.PI * $(elem).width() / 4;

    // Converting the percentage
    function numberFixer(value) {
      var result = ((value * total) / 100);
      return result;
    };

    // Filling function
    function setPieChart(value) {
      var fixedNumber = numberFixer(value);
      var result = fixedNumber + ' ' + total;
      $filling.css('stroke-dasharray', result);
    };

    // Fill the chart to the given percentage
    setPieChart(percentage);

    // Animate the number to the given percentage
    numerate($('.receiver', elem), percentage)
  };

  // Call animation on dom element(s)
  $(document).ready(function() {

    $('*[data-behavior="pie-chart"]').each(function(i) {
      makePieChart($(this));
    });

  });

})(jQuery);

Here is my codepen again

\$\endgroup\$
1
\$\begingroup\$

Styling and readability

Your styling seems consistent and your indentation seems correct. I like the usage of variables prefixed with $ for jQuery objects. You consistently place variables at the top of functions, and have everything in an IIFE to prevent leaking into the global scope, and possible redefinition of $.

You are missing two semicolon's. I would recommend adding semicolon's behind every statement instead of letting ASI take care of it. This will prevent accidental mistakes where typo's cause multiple lines to belong to the same statement, instead of showing an error.

Naming of functions and parameters

The name of your function numberFixer does not really tell what that function does. The parameter name does not help either. The comment tells me that value should instead be named percentage. The function itself seems to convert that to a length of some kind?

The parameter of setPieChart does not name what is expected as input. It is called with a variable percentage. You probably should rename the parameter to that.

In addition to @Dannnno's answer, the function for easeOutCubic has very unclear parameters. You tell you took the function from jQuery-UI. Do yourself a favour, and add a comment to that function telling yourself where you got it from. This will save you a headache when debugging makes you end up at that function.

Bugs

In Internet Explorer 11 the filler stroke of the second circle has a noticable dent. This seems to be caused by the stroke width. An easy fix is to set the stroke width like so:

stroke-width: $diameter / 2 - 1;

Possible improvements

In numberFixer assigns a value to result, then returns result. The formula seems not to be too complicated. The outer parentheses are not needed either, and you do not seem to use that verbose style anywhere else. I would recommend reducing it to return value * total / 100;.

You have two functions, scoped under makePieChart, that use a variable that is defined in makePieChart. It is unclear to me why you have chosen this construction. Why not make these two functions take an extra variable? numberFixer seems unnecessary. It is used once, and it seems to have very little value outside makePieChart. I would recommend removing that function and just do the calculation in place where you are currently calling that function. setPieChart seems reasonable to have scoped higher to re-use at some point. I would recommend moving it one scope higher.

\$\endgroup\$
  • \$\begingroup\$ Thank you, I modified my codepen codepen.io/toplefty/pen/pgyjzr according to your comments. (I'll validate answer later as I'd like some input on the scss if I can get it) \$\endgroup\$ – Antonin Cezard Jan 6 '16 at 10:44
  • \$\begingroup\$ Hey, I'm gonna accept your answer. Anyway I've worked on the pie and changed a good amount of stuff, if by chance you have some time to check it out it would be greatly appreciated (same url as before). \$\endgroup\$ – Antonin Cezard Jan 16 '16 at 13:00
  • \$\begingroup\$ Quickly glancing over it, I notice you have one place where you execute code before defining all variables. See Variable Hoisting for more information on that. Further it does not fill it up completely on 100%. If you want a full review on your improved code, feel free to make a new question. See this meta question for more details how to do that. \$\endgroup\$ – Sumurai8 Jan 16 '16 at 16:20
  • \$\begingroup\$ Thanks I will do that. Noticed the 100% bug, its because I add 0.5 on the second circle radius because there was an ugly bug on non-webkit browsers \$\endgroup\$ – Antonin Cezard Jan 16 '16 at 16:29
2
\$\begingroup\$

Why are you doing this in easeOutCubic?

return c * ((t = t / d - 1) * t * t + 1) + b;

that's way harder to read than something like

t = t / d - 1;
return c * (t * t * t + 1) + b;
// or
return c * (Math.pow(t, 3) + 1) + b;

Sorry I don't know a ton about jQuery or SVG or I'd try to help more, but that was really bothering me.

\$\endgroup\$
  • \$\begingroup\$ basically I just copied the function from jquery-UI, as is. I didn't want to include all the library just for an easing parameter \$\endgroup\$ – Antonin Cezard Jan 5 '16 at 14:54
2
\$\begingroup\$

Surely $(elem).width() / 4 is already defined as a <circle> attribute so why not read it from the DOM in the same way as percentage?

Alternatively, simplify the HTML down to the <figure></figure> wrapper and generate its <svg>...</svg> content dynamically.

The legend doesn't appear in either Opera or Chrome.

\$\endgroup\$
  • \$\begingroup\$ Yes I tried generating the html from JS (circle dimensions etc). But I didn't like the fact that the pie was then popping up after the dom load, it looked a bit goofy. But it would be far better than having to write the html with the correct radiuses and coordinates \$\endgroup\$ – Antonin Cezard Jan 7 '16 at 7:51
  • \$\begingroup\$ You might try pre-rendering in an off-screen canvas then rendering the off-screen canvas onto the visible one. The technique is described here. \$\endgroup\$ – Roamer-1888 Jan 7 '16 at 10:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.