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

Make `Chart.Animation/animations/Tooltip` importable #5382

Merged
merged 1 commit into from Apr 2, 2018

Conversation

@simonbrunel
Copy link
Member

simonbrunel commented Apr 1, 2018

Relates to #4478

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

This comment has been minimized.

Copy link
@simonbrunel

simonbrunel Apr 1, 2018

Author Member

@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;

This comment has been minimized.

Copy link
@simonbrunel

simonbrunel Apr 1, 2018

Author Member

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

This comment has been minimized.

Copy link
@etimberg

etimberg Apr 1, 2018

Member

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

This comment has been minimized.

Copy link
@simonbrunel

simonbrunel Apr 1, 2018

Author Member

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

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

options: {
  tooltips: {
    position: 'foobar'
  }
}
@simonbrunel simonbrunel mentioned this pull request Apr 1, 2018
8 of 9 tasks complete
@simonbrunel simonbrunel merged commit d284686 into chartjs:master Apr 2, 2018
3 checks passed
3 checks passed
codeclimate Approved by Simon Brunel.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 95.329%
Details
@simonbrunel simonbrunel deleted the simonbrunel:core-modules branch Apr 2, 2018
}
// If the user provided a sorting function, use it to modify the tooltip items
if (opts.itemSort) {
tooltipItems = tooltipItems.sort(function(a, b) {

This comment has been minimized.

Copy link
@A1vinSmith

A1vinSmith Jul 31, 2019

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

remove self-assignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.