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

Adding support for dc.lineChart.curve. Deprecating interpolate/tension. #1381

Closed
wants to merge 2 commits into from
Closed

Conversation

kum-deepak
Copy link
Collaborator

Resolves #1376

Updated documentation and code examples. Looking forward to your feedback.

}

if (_tension && typeof curve.tension === 'function') {
curve = curve.tension(_tension);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a warning here if _tension was specified but the curve doesn't support it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

While fixing also realized that 0 is a valid value for _tension, which would have gotten discarded.

@gordonwoodhull
Copy link
Contributor

This is really solid, careful work. Just one more backward-compatibility warning would help, I think.

Thank you for all your efforts.

@kum-deepak
Copy link
Collaborator Author

Thanks @gordonwoodhull, will add the warning soon. Currently deep into the zoom behavior.

gordonwoodhull added a commit that referenced this pull request Mar 28, 2018
@gordonwoodhull
Copy link
Contributor

Thanks @kum-deepak! Made some tweaks and merged.

@kum-deepak
Copy link
Collaborator Author

Thanks @gordonwoodhull, all your changes make sense!

Any suggestions what I should pick up next?

@kum-deepak kum-deepak deleted the curve branch March 28, 2018 15:22
@gordonwoodhull
Copy link
Contributor

Thanks @kum-deepak, please review your comments and Upgrade Notes from #1363 and see if there is anything else you want to address.

After that, please start looking at the 4 PRs I listed there, which I believe are ready to go, but need to be ported to d3v4. I have not thoroughly reviewed them, so if you see anything that needs improvement, go for it. I think they are ready because they have passing automated tests and most of them have also been tested by people beside the author.

I will look at the transition issues and d3.stack. Apart from an issue or two in the examples, I think these are the only blockers for releasing a beta.

Congratulations, and thank you!

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.

2 participants