Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make Chart.Animation/animations/Tooltip importable #5382

Merged
merged 1 commit into from
Apr 2, 2018

Conversation

simonbrunel
Copy link
Member

Relates to #4478

@simonbrunel simonbrunel added this to the Version 2.8 milestone Apr 1, 2018
@simonbrunel simonbrunel requested a review from etimberg April 1, 2018 15:04
@@ -1,5 +1,6 @@
{
"indent-style": "tabs",
"line-end-style": false,
Copy link
Member Author

Choose a reason for hiding this comment

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

@loicbourgois I had to disable this rule, else we can't run gulp lint on Windows with Git configured to AutoCrlf.

/**
* @namespace Chart.Tooltip.positioners
*/
exports.positioners = positioners;
Copy link
Member Author

Choose a reason for hiding this comment

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

@etimberg I'm not sure why we need to expose positioners (Chart.Tooltip.positioners)

Copy link
Member

Choose a reason for hiding this comment

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

It must be a backwards compatibility thing. Not sure when it first got introduced

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I think it was public to allow users to implement their own logic:

Chart.Tooltip.positioners.foobar = function(elements) {
  // ...
};

options: {
  tooltips: {
    position: 'foobar'
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

@simonbrunel simonbrunel mentioned this pull request Apr 1, 2018
9 tasks
@simonbrunel simonbrunel merged commit d284686 into chartjs:master Apr 2, 2018
@simonbrunel simonbrunel deleted the core-modules branch April 2, 2018 08:55
}
// If the user provided a sorting function, use it to modify the tooltip items
if (opts.itemSort) {
tooltipItems = tooltipItems.sort(function(a, b) {

Choose a reason for hiding this comment

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

tooltipItems.sort(function(a, b) {
    return opts.itemSort(a, b, data);
});

remove self-assignment

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants