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

hideAxis is recently broken #57

Closed
joshhjacobson opened this issue Dec 29, 2018 · 3 comments
Closed

hideAxis is recently broken #57

joshhjacobson opened this issue Dec 29, 2018 · 3 comments

Comments

@joshhjacobson
Copy link
Contributor

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
Copy link
Contributor

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

timelyportfolio added a commit to timelyportfolio/parcoords-es that referenced this issue Jan 1, 2019
@joshhjacobson
Copy link
Contributor Author

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

@BigFatDog
Copy link
Owner

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.

joshhjacobson added a commit to joshhjacobson/parcoords-es that referenced this issue Jan 18, 2019
BigFatDog added a commit that referenced this issue Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants