BigFatDog / parcoords-es

ES6 module of Syntagmatic's Parallel Coordinates
MIT License
62 stars 32 forks source link

hideAxis is recently broken #57

Closed joshhjacobson closed 5 years ago

joshhjacobson commented 5 years ago

In version 2.2.0, the bug fix in the brushFor function had the side effect of partially breaking hideAxis. The function works when data is first added to the chart, but if called individually there is an error. The following, taken from the brushing.html demo, demonstrates the issue:

d3.csv('data/cars.csv').then(function(data) {
            parcoords
                .data(data)
                .hideAxis(["name"])
                .composite("darker")
                .render()
                .shadows()
                .reorderable()
                .brushMode("1D-axes");  // enable brushing

            // next line is the problem (added for demonstration)
            parcoords.hideAxis(['year']).render().updateAxes(0);

        });

This gives:

Uncaught (in promise) TypeError: Cannot read property 'yscale' of undefined
    at convertBrushArguments (brushFor.js:29)
    at SVGGElement.<anonymous> (brushFor.js:63)
    at Dispatch.apply (dispatch.js:61)
    at customEvent (on.js:103)
    at Emitter.emit (brush.js:286)
    at Emitter.brush (brush.js:278)
    at SVGGElement.<anonymous> (brush.js:222)
    at Selection.selection_each [as each] (each.js:5)
    at brush.move (brush.js:212)
    at Selection.selection_call [as call] (call.js:4)

In versions prior to 2.2.0, this was not an issue. Now, it seems that the axis being referenced in brushFor.js here

const convertBrushArguments = args => {
    const args_array = Array.prototype.slice.call(args);
    const axis = args_array[0];
    // ordinal scales do not have invert
    const yscale = config.dimensions[axis].yscale;

has already been removed from config.dimensions in sideEffects.js here

.on('hideAxis', d => {
      pc.dimensions(pc.applyDimensionDefaults());
      pc.dimensions(without(config.dimensions, d.value));  // this line
    })

thus, config.dimensions[axis] is undefined.

I would attempt to fix this myself, but I'm not really sure why it's happening. @timelyportfolio do you have any thoughts on this? Maybe we could delay execution of pc.dimensions(without(config.dimensions, d.value)) until after brushFor has been executed?

timelyportfolio commented 5 years ago

@joshhjacobson thanks so much for the report. I have a fix for this, but I'm going to look quickly at #26 to see if we can resolve both.

joshhjacobson commented 5 years ago

@timelyportfolio thank you for the fast response, so glad you were able to figure this out so quickly!

BigFatDog commented 5 years ago

Thanks @joshhjacobson for discovering this potential bug and @timelyportfolio 's quick fix on his weekend!

v2.2.5 has been released to deliver to fix.