Master / Detail dynamic isRowMaster feature

[avallete]

I'm submitting a ... (check one with "x")

[X] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/ceolter/ag-grid-enterprise/blob/master/CONTRIBUTING.md#question

Current behavior

When using ag-grid-enterprise MasterDetail feature with the 'editable: true' feature of ag-grid. We get some inconsistency with the Dynamically Specify Master Nodes feature of the MasterDetail.

The isRowMaster seem to be called only when the number of rows into the data changes. Not when the row count are the same but data inside row change. That lead to "dynamic" isRowMaster no being immediately evaluated.

Expected behavior

When isRowMaster is a function and the row contain a column marked as 'editable', we know that the api.setRowData will not be the only way that our data change. So, in that case, we should evaluate the 'isRowMaster' on every data changes (editing).

Minimal reproduction of the problem with instructions Here a reproduction of the bug based on the MasterDetail: Dynamically Specify Master Nodes documentation example. In that example, the isRowMaster depend of the value inside the 'calls' columns which can be edited.

https://stackblitz.com/edit/ag-grid-master-detail-bug

What is the motivation / use case for changing the behavior?

If you have some logic of form/subform inside an editable ag-grid, you may want to display the expand icon only if it make sense.

Ex: In the case of the phone calls, if that data aren't automatically fetched from server but provided by user input, you may want to display the subgrid only if the 'calls' number are greater than 0.

Please tell us about your environment:

Irrelevant for this bug.

• ag-Grid-Enterprise version: 20.2.0

• Browser: [all]

• Language: [all]

[matchdav]

I had the same issue, found the following hack worked

<Grid
  //...
  isFullWidthCell={(rowNode) => {
    // hack because isRowMaster isn't supported
    // @see https://github.com/ag-grid/ag-grid-enterprise/issues/221

    if (!this.isRowMaster(rowNode)) {  // assuming you have this function in your class
      rowNode.master = false;
    }
    return false;
  }}
/>

[avallete]

I had the same issue, found the following hack worked

<Grid
  //...
  isFullWidthCell={(rowNode) => {
    // hack because isRowMaster isn't supported
    // @see https://github.com/ag-grid/ag-grid-enterprise/issues/221

    if (!this.isRowMaster(rowNode)) {  // assuming you have this function in your class
      rowNode.master = false;
    }
    return false;
  }}
/>

Nice, could you please integrate this hack into the minimal reproduction code here: https://stackblitz.com/edit/ag-grid-master-detail-bug

And share the link ? Because I've not been able to use your method to fix my issue (even with isFullWidthCell, when a change occurs, the master/detail does not seem to update).

To me, the only way I've been able to really have a dynamic master detail with editing mode is by putting the master/detail check into the grid column definition with something like:

    {
      field: 'calls',
      editable: true,
      onCellValueChanged: function (params) { // Here we listen cell change and redraw line each time to display master/detail
        if (!isRowMaster(params.data)) {
          params.node.master = false;
          params.node.setExpanded(false); // Close the previously opened detail grid if it has been open to avoid 'remaining child grid display' bug
        } else {
          params.node.master = true;
        }
        params.api.redrawRows(params);
      }
    },

The solution can be tested here. Only issue is that it force the grid to re-render all rows at every column change (which can totally blow the performances for huge grid).

[fidian]

I have the same issue. I'm using a server-side row model. Actions outside the grid can change the data that's being rendered, so I can loop through the node data and set a property. My isRowMaster() function is not called again when I use api.redrawRows(). By putting the same condition into isFullWidthCell() it works great for me. Terrible workaround, but I'm quite thankful it exists!

gridOptions.isFullWidthCell = (rowNode) => {
    rowNode.master = this.isMaster(rowNode.data);
    return false;
};
gridOptions.isRowMaster = (data) => this.isMaster(data);

My data is not editable within Ag-Grid. It appears that the problem is that the isRowMaster() result is cached and only called once for the life of the row node.