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.

Sign up for GitHub

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

Jump to bottom

Fix tooltip animation when target changes while animating #5005

Merged
merged 7 commits into from Dec 12, 2017
Merged

Fix tooltip animation when target changes while animating #5005

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
33a51ed
Fix issue #4989
jcopperfield Nov 29, 2017
ce679dc
Change: different approach for smooth tooltip animation in 'index'
jcopperfield Nov 30, 2017
982f0b4
Fix: jslint error
jcopperfield Nov 30, 2017
482e59f
Fix: remove spyOn pivot from test
jcopperfield Nov 30, 2017
38574c3
Add: setAnimating-flag in transition used to set on tooltip.transition
jcopperfield Dec 3, 2017
f7d4d54
Decrease code complexity
jcopperfield Dec 3, 2017
badc4d9
Revert transition and complexity changes
jcopperfield Dec 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/core/core.controller.js
View file
Open in desktop
Expand Up @@ -555,7 +555,7 @@ module.exports = function(Chart) {
}
}

me.tooltip.transition(easingValue);
me.tooltip.transition(easingValue, true);
},

/**
Expand Down Expand Up @@ -821,7 +821,13 @@ module.exports = function(Chart) {
me._bufferedRequest = null;

var changed = me.handleEvent(e);
changed |= tooltip && tooltip.handleEvent(e);
// for smooth tooltip animations issue #4989
// the tooltip should be the source of change
if (tooltip) {
changed = tooltip.animating
? tooltip.handleEvent(e)
: changed | tooltip.handleEvent(e);
}

plugins.notify(me, 'afterEvent', [e]);

Expand Down
17 changes: 10 additions & 7 deletions src/core/core.element.js
View file
Open in desktop
Expand Up @@ -35,12 +35,10 @@ function interpolate(start, view, model, ease) {
if (type === typeof origin) {
if (type === 'string') {
c0 = color(origin);
if (c0.valid) {
c1 = color(target);
if (c1.valid) {
view[key] = c1.mix(c0, ease).rgbString();
continue;
}
c1 = color(target);
Copy link
Member

Choose a reason for hiding this comment

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

Why changing that part of the code? Your changes actually decrease performances because c0 and c1 are now always evaluated (color()), which is a very costly operation while transitioning values.

if (c0.valid && c1.valid) {
view[key] = c1.mix(c0, ease).rgbString();
continue;
}
} else if (type === 'number' && isFinite(origin) && isFinite(target)) {
view[key] = origin + (target - origin) * ease;
Expand Down Expand Up @@ -72,7 +70,7 @@ helpers.extend(Element.prototype, {
return me;
},

transition: function(ease) {
transition: function(ease, setAnimating) {
var me = this;
var model = me._model;
var start = me._start;
Expand All @@ -82,6 +80,7 @@ helpers.extend(Element.prototype, {
if (!model || ease === 1) {
me._view = model;
me._start = null;
delete me.animating;
return me;
}

Expand All @@ -93,6 +92,10 @@ helpers.extend(Element.prototype, {
start = me._start = {};
}

if (setAnimating && !me.animating) {
me.animating = true;
}

interpolate(start, view, model, ease);

return me;
Expand Down
32 changes: 13 additions & 19 deletions src/core/core.tooltip.js
View file
Open in desktop
Expand Up @@ -852,25 +852,19 @@ module.exports = function(Chart) {
// Remember Last Actives
changed = !helpers.arrayEquals(me._active, me._lastActive);

// If tooltip didn't change, do not handle the target event
if (!changed) {
return false;
}

me._lastActive = me._active;

if (options.enabled || options.custom) {
me._eventPosition = {
x: e.x,
y: e.y
};

var model = me._model;
me.update(true);
me.pivot();

// See if our tooltip position changed
changed |= (model.x !== me._model.x) || (model.y !== me._model.y);
// Only handle target event on tooltip change
if (changed) {
me._lastActive = me._active;

if (options.enabled || options.custom) {
me._eventPosition = {
x: e.x,
y: e.y
};

me.update(true);
me.pivot();
}
}

return changed;
Expand Down