v2.29 multibrush and hideAxis bug

rvberloo commented 5 years ago

When using brushmode "1D-axes-multi" and performing a hideAxis call an error occurs in brus\1d-multi\brushreset.js

It seems properties are read in a forEach of an object (the axis that was just hidden?) that does not exist.

I modified the slickgrid example file to create a ready to use example, this example adds a button that removes three axes. This works OK for brushmode "1D-axes" but fails for "1D-axes-multi"

eg below code save as 'demo/slickgrid2.html'

<!doctype html>
<title>Linking to SlickGrid</title>

<!-- SlickGrid -->
<link rel="stylesheet" href="lib/slickgrid/slick.grid.css" type="text/css"/>
<link rel="stylesheet" href="lib/slickgrid/jquery-ui-1.8.16.custom.css" type="text/css"/>
<link rel="stylesheet" href="lib/slickgrid/examples.css" type="text/css"/>
<link rel="stylesheet" href="lib/slickgrid/slick.pager.css" type="text/css"/>
<script src="lib/slickgrid/jquery-1.7.min.js"></script>
<script src="lib/slickgrid/jquery.event.drag-2.0.min.js"></script>
<script src="lib/slickgrid/slick.core.js"></script>
<script src="lib/slickgrid/slick.grid.js"></script>
<script src="lib/slickgrid/slick.pager.js"></script>
<script src="lib/slickgrid/slick.dataview.js"></script>
<!-- End SlickGrid -->

<link rel="stylesheet" type="text/css" href="./parcoords.css">
<link rel="stylesheet" type="text/css" href="style.css">
<style>
body, html {
  margin: 0;
  height: 100%;
  width: 100%;
  overflow: hidden;
  font-size: 12px;
}
#grid, #pager {
  position: fixed;
  width: 100%;
}
#grid {
  bottom: 0;
  height: 300px;
}
#pager {
  bottom: 306px;
  height: 20px;
}
.slick-row:hover {
  font-weight: bold;
  color: #069;
}
</style>
<script src="lib/d3.v5.min.js"></script>
<script src="./parcoords.standalone.js"></script>
<script src="lib/divgrid.js"></script>
<button id="show_ext">remove axes</button>
<div id="example" class="parcoords" style="height:240px;"></div>
<div id="grid"></div>
<div id="pager"></div>
<script id="brushing">

var parcoords = ParCoords()("#example")
    .alpha(0.4)
    .mode("queue") // progressive rendering
    .height(d3.max([document.body.clientHeight-326, 220]))
    .margin({
      top: 36,
      left: 0,
      right: 0,
      bottom: 16
    });

    d3.select('#show_ext').on('click', function() {
      let hiddencolumns =['cylinders', 'power (hp)', 'year'];
      parcoords.hideAxis(hiddencolumns);
      parcoords.render().updateAxes(0);
    });

// load csv file and create the chart
d3.csv('data/cars.csv').then(function(data) {
  // slickgrid needs each data element to have an id
  data.forEach(function(d,i) { d.id = d.id || i; });

  parcoords
    .data(data)
    .hideAxis(["name"])
    .render()
    .reorderable()
    .brushMode("1D-axes-multi");

  // setting up grid
  var column_keys = d3.keys(data[0]);
  var columns = column_keys.map(function(key,i) {
    return {
      id: key,
      name: key,
      field: key,
      sortable: true
    }
  });

  var options = {
    enableCellNavigation: true,
    enableColumnReorder: false,
    multiColumnSort: false
  };

  var dataView = new Slick.Data.DataView();
  var grid = new Slick.Grid("#grid", dataView, columns, options);
  var pager = new Slick.Controls.Pager(dataView, grid, $("#pager"));

  // wire up model events to drive the grid
  dataView.onRowCountChanged.subscribe(function (e, args) {
    grid.updateRowCount();
    grid.render();
  });

  dataView.onRowsChanged.subscribe(function (e, args) {
    grid.invalidateRows(args.rows);
    grid.render();
  });

  // column sorting
  var sortcol = column_keys[0];
  var sortdir = 1;

  function comparer(a, b) {
    var x = a[sortcol], y = b[sortcol];
    return (x == y ? 0 : (x > y ? 1 : -1));
  }

  // click header to sort grid column
  grid.onSort.subscribe(function (e, args) {
    sortdir = args.sortAsc ? 1 : -1;
    sortcol = args.sortCol.field;

    if ($.browser.msie && $.browser.version <= 8) {
      dataView.fastSort(sortcol, args.sortAsc);
    } else {
      dataView.sort(comparer, args.sortAsc);
    }
  });

  // highlight row in chart
  grid.onMouseEnter.subscribe(function(e,args) {
    // Get row number from grid
    var grid_row = grid.getCellFromEvent(e).row;

    // Get the id of the item referenced in grid_row
    var item_id = grid.getDataItem(grid_row).id;
    var d = parcoords.brushed() || data;

    // Get the element position of the id in the data object
    elementPos = d.map(function(x) {return x.id; }).indexOf(item_id);

    // Highlight that element in the parallel coordinates graph
    parcoords.highlight([d[elementPos]]);
  });

  grid.onMouseLeave.subscribe(function(e,args) {
    parcoords.unhighlight();
  });

  // fill grid with data
  gridUpdate(data);

  // update grid on brush
  parcoords.on("brush", function(d) {
    gridUpdate(d);
  });

  function gridUpdate(data) {
    dataView.beginUpdate();
    dataView.setItems(data);
    dataView.endUpdate();
  };

});
</script>

BigFatDog commented 5 years ago

v2.2.10 has been released to address this issue.

I've tested with example you provided:

Set brushes, click 'hide axis' and then set brushes again.
click 'hide axis' and set brushes.
click 'hide axis' and set brushes in turns.

rvberloo commented 5 years ago

Works nicely, thanks!

BigFatDog commented 5 years ago

You're welcome!

joshhjacobson commented 5 years ago

@BigFatDog doesn't look like v2.2.10 is available through npm. Can you publish it there?

BigFatDog commented 5 years ago

Done. My mistake.

joshhjacobson commented 5 years ago

Thanks!

BigFatDog commented 5 years ago

You're welcome.

BigFatDog / parcoords-es

v2.29 multibrush and hideAxis bug #69