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

GH-538 Adds support for range of data in the Column Chart #543

Merged
merged 1 commit into from Jul 8, 2015

Conversation

Projects
None yet
2 participants
@anmoljagetia
Contributor

anmoljagetia commented Jul 1, 2015

#538.

@GordonSmith @mlzummo Please Review.

Signed-off-by: Anmol Jagetia anmoljagetia@gmail.com

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:columnRange branch from b96d056 to 6870641 Jul 1, 2015

.style("fill", function (d, idx) { return context._palette(context._columns[idx + 1]); })
;
}
title
.text(function (d, idx) { return dataRow[0] + " (" + d + "," + " " + context._columns[idx + 1] + ")"; })
.text(function (d, idx) { return dataRow[0] + " (" + d[1] + "," + " " + context._columns[idx + 1] + ")"; })

This comment has been minimized.

@GordonSmith

GordonSmith Jul 1, 2015

Member

Should use d instanceof Array

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:columnRange branch from 19a2300 to 7887b1c Jul 2, 2015

title
.text(function (d, idx) { return dataRow[0] + " (" + d + "," + " " + context._columns[idx + 1] + ")"; })
columnRect.select("title")
.text(function (d, idx) { return dataRow[0] + " (" + d + "," + " " + context._columns[idx] + ")"; })

This comment has been minimized.

@GordonSmith

GordonSmith Jul 2, 2015

Member

Should this be + 1?

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:columnRange branch from 7887b1c to 395eb1d Jul 2, 2015

@@ -310,10 +327,10 @@
break;
}
var min = d3.min(this.formattedData(), function (data) {
return d3.min(data.filter(function (cell, i) { return i > 0 && context._columns[i] && context._columns[i].indexOf("__") !== 0; }), function (d) { return +d; });
return d3.min(data.filter(function (cell, i) { return i > 0 && context._columns[i] && context._columns[i].indexOf("__") !== 0; }), function (d) { return d instanceof Array ? d[0] : d; });

This comment has been minimized.

@GordonSmith

GordonSmith Jul 6, 2015

Member

Add back the "+" operator as it is converting strings to numbers.

GH-538 Adds support for range of data in the Column Chart
Fixes GH-538

Signed-off-by: Anmol Jagetia <anmoljagetia@gmail.com>

@anmoljagetia anmoljagetia force-pushed the anmoljagetia:columnRange branch from 395eb1d to 7eb20a1 Jul 6, 2015

@GordonSmith

This comment has been minimized.

Member

GordonSmith commented Jul 8, 2015

Looks good.

GordonSmith added a commit that referenced this pull request Jul 8, 2015

Merge pull request #543 from anmoljagetia/columnRange
GH-538 Adds support for range of data in the Column Chart

@GordonSmith GordonSmith merged commit 2d7957c into hpcc-systems:master Jul 8, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment