Skip to content

Commit b1e538d

Browse files
committed
Check for numeric - text name collisions within frames of a single addFrames call too
1 parent a666251 commit b1e538d

File tree

2 files changed

+28
-8
lines changed

2 files changed

+28
-8
lines changed

src/plot_api/plot_api.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2631,16 +2631,21 @@ Plotly.addFrames = function(gd, frameList, indices) {
26312631
var bigIndex = _frames.length + frameList.length * 2;
26322632

26332633
var insertions = [];
2634+
var _frameHashLocal = {};
26342635
for(i = frameList.length - 1; i >= 0; i--) {
26352636
if(!Lib.isPlainObject(frameList[i])) continue;
26362637

2637-
var name = (_frameHash[frameList[i].name] || {}).name;
2638+
// The entire logic for checking for this type of name collision can be removed once we migrate to ES6 and
2639+
// use a Map instead of an Object instance, as Map keys aren't converted to strings.
2640+
var lookupName = frameList[i].name;
2641+
var name = (_frameHash[lookupName] || _frameHashLocal[lookupName] || {}).name;
26382642
var newName = frameList[i].name;
2643+
var collisionPresent = _frameHash[name] || _frameHashLocal[name];
26392644

2640-
if(name && newName && typeof newName === 'number' && _frameHash[name] && numericNameWarningCount < numericNameWarningCountLimit) {
2645+
if(name && newName && typeof newName === 'number' && collisionPresent && numericNameWarningCount < numericNameWarningCountLimit) {
26412646
numericNameWarningCount++;
26422647

2643-
Lib.warn('addFrames: overwriting frame "' + _frameHash[name].name +
2648+
Lib.warn('addFrames: overwriting frame "' + (_frameHash[name] || _frameHashLocal[name]).name +
26442649
'" with a frame whose name of type "number" also equates to "' +
26452650
name + '". This is valid but may potentially lead to unexpected ' +
26462651
'behavior since all plotly.js frame names are stored internally ' +
@@ -2653,6 +2658,8 @@ Plotly.addFrames = function(gd, frameList, indices) {
26532658
}
26542659
}
26552660

2661+
_frameHashLocal[lookupName] = {name: lookupName};
2662+
26562663
insertions.push({
26572664
frame: Plots.supplyFrameDefaults(frameList[i]),
26582665
index: (indices && indices[i] !== undefined && indices[i] !== null) ? indices[i] : bigIndex + i

test/jasmine/tests/animate_test.js

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -684,13 +684,26 @@ describe('Animate API details', function() {
684684

685685
it('emits warning if strings are not used and this creates ambiguity', function(done) {
686686
spyOn(Lib, 'warn');
687-
Plotly.addFrames(gd, [{name: '8', data: [{x: [8, 7, 6]}]}])
688-
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
689-
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
690-
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
691-
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
687+
688+
// Test with both multiframe additions and repeated `addFrames` calls - both should count toward the warn limit
689+
Plotly.addFrames(gd, [
690+
{name: 8, data: [{x: [8, 7, 6]}]},
691+
{name: 8888, data: [{x: [8, 7, 6]}]},
692+
{name: 8, data: [{x: [8, 7, 6]}]},
693+
{name: '8', data: [{x: [8, 7, 6]}]}
694+
])
695+
.then(function() {
696+
// so far, two warnings
697+
expect(Lib.warn.calls.count()).toEqual(2);
698+
return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);
699+
})
692700
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
693701
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
702+
.then(function() {
703+
// so far, 5 + 1 warnings
704+
expect(Lib.warn.calls.count()).toEqual(5 + 1);
705+
return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);
706+
})
694707
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
695708
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})
696709
.then(function() {return Plotly.addFrames(gd, [{name: 8, data: [{x: [3, 2, 1]}]}]);})

0 commit comments

Comments
 (0)