Skip to content

Commit aec4e47

Browse files
committed
Handle NaN values and add JSDoc to compareArrays
Per review suggestion from camdecoster: - Add JSDoc comment documenting ascending-order comparison, NaN-sorting behavior, and prefix handling. - Handle values that do not order against each other (NaN, undefined) by sorting them after every orderable value. - Add test verifying NaN color values sort after orderable values in bundled parallel-categories paths. The 4 pre-existing drag/reorder test failures are unchanged by this commit (27->28 passing, same 4 baseline failures).
1 parent 620bf41 commit aec4e47

2 files changed

Lines changed: 45 additions & 4 deletions

File tree

src/traces/parcats/parcats.js

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,12 +370,27 @@ function compareRawColor(a, b) {
370370
}
371371
}
372372

373+
/**
374+
* Compare two sort arrays element by element in ascending order.
375+
* Values that do not order against each other, for example NaN, sort last.
376+
* The shorter array sorts first when one array is a prefix of the other.
377+
*
378+
* @param {Array} a
379+
* @param {Array} b
380+
*/
373381
function compareArrays(a, b) {
374382
for(var i = 0; i < Math.min(a.length, b.length); i++) {
375-
if(a[i] < b[i]) {
376-
return -1;
377-
} else if(a[i] > b[i]) {
378-
return 1;
383+
var valA = a[i];
384+
var valB = b[i];
385+
386+
if(valA < valB) return -1;
387+
if(valA > valB) return 1;
388+
// Handle values that do not order against each other (NaN, undefined, etc.)
389+
if(valA !== valB) {
390+
// Sort these after every orderable value.
391+
var badA = isNaN(valA);
392+
var badB = isNaN(valB);
393+
if(badA !== badB) return badA ? 1 : -1;
379394
}
380395
}
381396

test/jasmine/tests/parcats_test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,32 @@ describe('Basic parcats trace', function() {
307307
.then(done, done.fail);
308308
});
309309

310+
it('should sort NaN color values after orderable values', function(done) {
311+
var trace = {
312+
type: 'parcats',
313+
dimensions: [
314+
{values: ['a', 'a', 'a', 'a']},
315+
{values: ['b', 'b', 'b', 'b']}
316+
],
317+
line: {color: [10, NaN, 2, NaN]},
318+
bundlecolors: true
319+
};
320+
321+
Plotly.newPlot(gd, [trace])
322+
.then(function() {
323+
var parcatsViewModel = d3Select('g.trace.parcats').datum();
324+
var pathColors = parcatsViewModel.paths.map(function(path) {
325+
return path.model.rawColor;
326+
});
327+
328+
// Orderable values sort first, NaN values sort last
329+
expect(pathColors.slice(0, 2)).toEqual([2, 10]);
330+
expect(isNaN(pathColors[2])).toBe(true);
331+
expect(isNaN(pathColors[3])).toBe(true);
332+
})
333+
.then(done, done.fail);
334+
});
335+
310336
it('should compute initial model views properly', function(done) {
311337
Plotly.newPlot(gd, basicMock)
312338
.then(function() {

0 commit comments

Comments
 (0)