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

Pie chart feature #3

Merged
65 commits merged into from
Jul 22, 2016
Merged

Pie chart feature #3

65 commits merged into from
Jul 22, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jun 20, 2016

No description provided.

(d3-attr 'y' (pipe outputAccessor yScale))
(d3-attr 'height' (pipe outputAccessor yScale (bar-height height)))
)
enter=(pipe
Copy link
Author

Choose a reason for hiding this comment

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

can you please revert the changes to this file? they shouldn't be in this PR

Choose a reason for hiding this comment

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

yep, will resolve in the next commit

@ghost
Copy link
Author

ghost commented Jun 28, 2016

addresses #13


export function emberSparklesArc([ radius ], textLabel=false) {
if (textLabel === true){
return arc().outerRadius(radius - 40).innerRadius(radius - 40);
Copy link
Author

Choose a reason for hiding this comment

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

we will want to pass outerRadius and innerRadius arguments to this helper instead of hardcoding their inputs

Copy link
Author

Choose a reason for hiding this comment

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

this means we will be able to remove the if (textLabel...) part (since the arguments will be different depending on whether these are text labels or not)

@ghost
Copy link
Author

ghost commented Jul 15, 2016

@taras can you review this PR please? The pie chart is fully functional now


this.setProperties({ data, domain });

assert.equal(this.$('path').length, 4, 'There are the correct number of arcs after the data is updated');
Copy link
Author

Choose a reason for hiding this comment

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

I know that checking the data bound to each arc is very hard to do - instead, can you also check the arc percentages as a proxy for this?

@taras
Copy link
Contributor

taras commented Jul 18, 2016

I get this error when I click around the pie chart.

screen shot 2016-07-18 at 3 19 33 am

I did some googling. It looks like you need a custom tween for this. http://stackoverflow.com/a/21287079/172894

@ghost
Copy link
Author

ghost commented Jul 22, 2016

Looks great!

@ghost ghost merged commit 0c582d4 into master Jul 22, 2016
@ghost
Copy link
Author

ghost commented Jul 22, 2016

closes #3

@ghost ghost mentioned this pull request Jul 22, 2016
ghost pushed a commit that referenced this pull request Jul 22, 2016
…h-pages

* 'master' of github.com:LocusEnergy/ember-sparkles: (88 commits)
  Released v0.4.7
  Pie chart feature (#3)
  Removed use of map and d3-get (#19)
  Reactive helpers (#18)
  Released v0.4.6
  removes unused code
  Released v0.4.5
  sets up demo site (#17)
  updates node version
  fixes CI config
  upgrades ember-cli, adds ember-cli-eslint
  Released v0.4.4
  adds ember-cli-release config
  adds ember-try
  Released v0.4.3
  uses v0.9 of ember-cli-d3-shape and v0.5.1 of ember-d3-helpers (#15)
  0.4.2
  Refines axis filter, makes vertical axis label responsive (#16)
  0.4.1
  filters ticks based on integer input (#14)
  ...
@guozhaonan guozhaonan deleted the pie-chart-feature branch July 26, 2016 21:56
@guozhaonan guozhaonan restored the pie-chart-feature branch July 27, 2016 20:07
@guozhaonan guozhaonan deleted the pie-chart-feature branch July 27, 2016 20:07
This pull request was closed.
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