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

angles option for polar chart #6472

Closed
wants to merge 5 commits into from
Closed

Conversation

mp3000mp
Copy link
Contributor

Hello,

This should fix #4751.

Demo here : https://jsfiddle.net/wanvfqbr/

@@ -96,6 +96,7 @@ These are the customisation options specific to Polar Area charts. These options
| `startAngle` | `number` | `-0.5 * Math.PI` | Starting angle to draw arcs for the first item in a dataset.
| `animation.animateRotate` | `boolean` | `true` | If true, the chart will animate in with a rotation animation. This property is in the `options.animation` object.
| `animation.animateScale` | `boolean` | `true` | If true, will animate scaling the chart from the center outwards.
| `angles` | `array` | `(2*Math.PI)/serieCount` | Computed angles to draw each item arc.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think serie is a word. It also doesn't match the usual terminology. We have datasets which contain data points

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oups sorry, you're right, this is actualy the French word.

I suggest to put [] as default value and explain the behavior more in details in the description column with the following :

  • If this option is not set, polar circumference = 2 * PI and each arc will be (2 * PI) / (visible datasets count).
  • Otherwise, polar circumference = sum(options.angles).
    • If options.angles.length < (visible datasets count)
      • Undefined angles will be set to (2 * PI) / (visible datasets count).
      • Defined angles will be recomputed to fill proportionaly the remaining polar circumference ((polar circumference) - sum(undefined angles)).
    • If options.angles.length > (visite datasets count)
      • Angles will be recomputed proportionaly to fill the entire polar circumference.

What do you think ?

Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

Would be good to have image based tests for these changes as well

var angle, ratio;
var initialCircumference = 0;
var convertedAngles = [];
var circumference = angles.length ? angles.reduce(function(a, b) {
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the sum of the angles is > 2*pi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. If the sum of the angles is > 2pi, the draw is more than one complete circle (you can easily try it here : https://jsfiddle.net/wanvfqbr/ ).
    This does not chock me as it is the same behavior if sum of the angles is < 2
    pi (draw is less than one complete circle) and people may find this usefull.
    Tell me if you would expect another behavior.

  2. How much test do you think we need ? I'd suggest four cases :

  • angles option is not set
  • options.angles.length = (visible datasets count)
  • options.angles.length < (visible datasets count)
  • options.angles.length > (visible datasets count)

@mp3000mp
Copy link
Contributor Author

It seems this feature is already implemented and tested by angle-array.json.

It's working and it's good enough for me (demo here : https://jsfiddle.net/h1e78vbq/).

We can close this PR and just update the documentation (see #6491)

@mp3000mp mp3000mp closed this Aug 31, 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.

[FEATURE] Custom angles for polarArea graph
3 participants